Bug 66868 - [5 Regression] wrong code generated with -O3 (dead code removal?)
Summary: [5 Regression] wrong code generated with -O3 (dead code removal?)
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 5.1.1
: P3 normal
Target Milestone: 5.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2015-07-14 12:05 UTC by Matthias Klose
Modified: 2016-08-13 10:17 UTC (History)
5 users (show)

See Also:
Host:
Target: powerpc64le-linux-gnu, x86_64-linux-gnu, aarch64-linux-gnu
Build:
Known to work: 4.9.3, 6.0
Known to fail: 5.1.0, 5.2.0
Last reconfirmed: 2015-11-18 00:00:00


Attachments
preprocessed source (135.96 KB, application/gzip)
2015-07-14 12:05 UTC, Matthias Klose
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Klose 2015-07-14 12:05:54 UTC
Created attachment 35975 [details]
preprocessed source

[forwarded from https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1473674]

seen in the apt package manager, when built with -O3. Works with -O2 or -O3 -fno-inline-functions -finline-small-functions, on at least x86_64 and powerpc64le. The test CDROMTest.FindPackages from the apt testsuite then fails.

built with g++ -c -g -O3 -fPIE -fstack-protector-strong -fPIC 

upstream writes:

There is a Cdrom wrapper:
"""
class Cdrom : public pkgCdrom {
   public:
      bool FindPackages(std::string const &CD,
     std::vector<std::string> &List,
     std::vector<std::string> &SList,
     std::vector<std::string> &SigList,
     std::vector<std::string> &TransList,
     std::string &InfoDir) {
  std::string const startdir = SafeGetCWD();
  EXPECT_FALSE(startdir.empty());
  bool const result = pkgCdrom::FindPackages(CD, List, SList, SigList, TransList, InfoDir, NULL, 0);
...
}
"""

and a unittest that calls it:
"""
TEST(CDROMTest,FindPackages)
{
   Cdrom cd;
  std::string InfoDir;
  EXPECT_TRUE(cd.FindPackages(path, Packages, Sources, Signatur, Translation, InfoDir));
...
  EXPECT_EQ(path + "/.disk/", InfoDir);
}
"""
The actual code for this is apt-pkg/cdrom.cc:
"""
bool pkgCdrom::FindPackages(string CD,
       vector<string> &List,
       vector<string> &SList,
       vector<string> &SigList,
       vector<string> &TransList,
       string &InfoDir, pkgCdromStatus *log,
       unsigned int Depth)
{
...
   if (DirectoryExists(".disk") == true)
   {
      if (InfoDir.empty() == true)
  InfoDir = CD + ".disk/";
   }
...
"""

So I suspect that the optimizer gets confused that InfoDir is a reference or it gets confused because InfoDir is not used in FindPackages anymore and it assumes its dead code.

I tried to create a simplified testcase but failed so far. Whats interessting is that if I add a std::cerr << "debug" line into cdrom.cc lines (or even a "CD = CD"):
"""
   if (DirectoryExists(".disk") == true)
   {
      if (InfoDir.empty() == true) {
std::cerr << "debug" << std::endl;
  InfoDir = CD + ".disk/";
}
   }
"""
it works (which indicates dead-code elimination to me).
Comment 1 Markus Trippelsdorf 2015-07-14 12:25:42 UTC
Have you tried to build with -fsanitize=undefined?
Comment 2 Matthias Klose 2015-07-14 13:02:47 UTC
the testsuite passes with -fsanitize=undefined (no additional output) and both -fsanitize=undefined -fsanitize-undefined-trap-on-error doesn't abort either.
Comment 3 Richard Biener 2015-07-16 09:12:36 UTC
GCC 5.2 is being released, adjusting target milestone to 5.3.
Comment 4 Bill Schmidt 2015-07-21 22:10:17 UTC
Hm.  I compiled it as stated and I see a bunch of code that appears to be storing the ".disk/" string.  So it doesn't look like dead code elimination.  Perhaps a branch is short circuiting this, or the address of where to store it has been corrupted.

I don't have any way to confirm that I'm reproducing the problem when I compile, from what's given here.  It might be helpful to provide the full test case without ellipses, and the failure message given by the test.
Comment 5 Bill Schmidt 2015-07-22 13:54:47 UTC
Using current trunk, I compared the 193t.optimized dumps for the original cdrom.ii attachment and for the same attachment modified to add the debug output statement from the end of comment 2.  Other than the added statements for the debug output, there don't appear to be any changes.  This bug is marked as 5.1/6 regression -- are you certain that it regresses on trunk?  Will check 5.1 the same way.
Comment 6 Bill Schmidt 2015-07-22 13:55:28 UTC
Sorry, not from comment 2 on this bugzilla; comment 2 from the launchpad bug.
Comment 7 Bill Schmidt 2015-07-22 14:07:46 UTC
I tried the same thing with a snapshot of the 5 branch I had lying around:  r225783 from 2015-07-14.  I also don't see any differences in the output from the middle end, as I would expect since this bug has shown up on multiple targets.  So maybe this has been fixed recently?  Let me see if I can find an older snapshot.
Comment 8 Bill Schmidt 2015-07-22 14:28:34 UTC
Same experiment with r222218 from 2015-04-18.  Same results.

At this point I can't reproduce anything from the information given.  Do you have any local modifications that could be causing this?
Comment 9 Matthias Klose 2015-07-24 19:58:48 UTC
this works ok with trunk r226068, still fails on the branch.

gcc is configured with --enable-languages=c,ada,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-libquadmath --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-ppc64el/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-ppc64el --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-ppc64el --with-arch-directory=ppc64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-secureplt --with-cpu=power7 --with-tune=power8 --disable-multilib --enable-multiarch --disable-werror --with-long-double-128 --enable-checking=release --build=powerpc64le-linux-gnu --host=powerpc64le-linux-gnu --target=powerpc64le-linux-gnu
Comment 10 Matthias Klose 2015-07-24 23:34:11 UTC
rechecked with a 5.2.0 release tarball, fails with -O3
Comment 11 Matthias Klose 2015-07-27 17:05:45 UTC
rechecked with a 5.1.0 release tarball, fails with -O3
Comment 12 Richard Biener 2015-11-18 11:15:12 UTC
Matthias, can you bisect what makes it work on trunk?
Comment 13 Jakub Jelinek 2015-11-18 12:15:32 UTC
I don't see significant difference when adding the CD = CD; on the gcc 5 branch either.  So, IMNSHO, if you want anything to move forward on this issue, you need to create a (small) self-contained testcase out of it.  Add noinline attributes to the functions/methods that are called in FindPackages and do not need to be inlined to reproduce it, add noinline to FindPackages itself, if it still reproduces, stub the noinline functions/methods it calls and stub a caller (new main) that calls FindPackages that just sets up everything that is needed to reproduce.
Comment 14 Richard Biener 2015-12-04 10:45:51 UTC
GCC 5.3 is being released, adjusting target milestone.
Comment 15 Richard Biener 2016-06-03 10:05:40 UTC
GCC 5.4 is being released, adjusting target milestone.
Comment 16 Matthias Klose 2016-08-13 10:17:30 UTC
this turned out as a code issue in apt:
https://anonscm.debian.org/cgit/apt/apt.git/commit/?id=9445fa62386c80c9822e77484d30b2109aa0f2dc