Bug 101278 - [12 regression] g++ miscompiles cmake-3.18.5
Summary: [12 regression] g++ miscompiles cmake-3.18.5
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.0
: P1 normal
Target Milestone: 12.0
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2021-07-01 08:06 UTC by Sergei Trofimovich
Modified: 2021-07-01 10:32 UTC (History)
3 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-07-01 00:00:00


Attachments
untested patch (1.21 KB, patch)
2021-07-01 09:19 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2021-07-01 08:06:30 UTC
Initially noticed the failure as a cmake-3.18.5 install failure when built with today's gcc.

Here is the extracted and slightly simplified example. It should concatenate "P", "S" into "PS". On -O0 it gets "PS", on -O1 it it gets "S":

//$ cat cmakemain.cxx
#include <algorithm>
#include <initializer_list>
#include <vector>

#include <cstdio>

static std::vector<char> cmCatViews_(std::initializer_list<const char *> views)
{
  // assume length=3 for each input to avoid strlen
  std::size_t each_size = 1;

  std::size_t total_size = 0;
  for (const char * p : views) {
    total_size += each_size;
  }

  std::vector<char> r(total_size + 1, '\0');
  std::vector<char>::iterator sit = r.begin();

  for (const char * p : views) {
    sit = std::copy_n(p, each_size, sit);
  }
  return r;
}

int main(int ac, char const* const* av)
{
  const char * v = "P";
  const char * s = "S";

  std::vector<char> r = cmCatViews_({v, s});

  fprintf(stderr, "R: %s\n", &r[0]);
  return 0;
}


$ /tmp/gcc-native-quick-installed/bin/x86_64-pc-linux-gnu-g++ cmakemain.cxx -o b0 -O0
$ ./b0

R: PS
$ /tmp/gcc-native-quick-installed/bin/x86_64-pc-linux-gnu-g++ cmakemain.cxx -o b1 -O1
$ ./b1
R: S

$ /tmp/gcc-native-quick-installed/bin/x86_64-pc-linux-gnu-g++ -v
Using built-in specs.
COLLECT_GCC=/tmp/gcc-native-quick-installed/bin/x86_64-pc-linux-gnu-g++
COLLECT_LTO_WRAPPER=/tmp/gcc-native-quick-installed/bin/../libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /home/slyfox/dev/git/gcc/configure --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu --enable-languages=c,c++ --disable-bootstrap --with-multilib-list=m64 --prefix=/tmp/gn/../gcc-native-quick-installed --disable-nls --without-isl --disable-libsanitizer --disable-libvtv --disable-libgomp --disable-libstdcxx-pch --disable-libunwind-exceptions CFLAGS='-O1 ' CXXFLAGS='-O1 '
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.0 20210701 (experimental) (GCC)
Comment 1 Jakub Jelinek 2021-07-01 08:17:23 UTC
Started with r12-656-ga564da506f52be66ade298b562417641e87b549f
Comment 2 Richard Biener 2021-07-01 09:02:31 UTC
There's a single deleted call LHS for the whole compilation:

 ;; Function cmCatViews_ (_ZL11cmCatViews_St16initializer_listIPKcE, funcdef_no=2127, decl_uid=467
17, cgraph_uid=266, symbol_order=284)
 
+  Deleted trivially dead stmt: p_35 = *__for_begin_3;
+
+  Deleted dead store in call LHS: sit = std::copy_n<const char*, long unsigned int, __gnu_cxx::__normal_iterator<char*, std::vector<char> > > (p_28, 1, sit);
+
 struct vector cmCatViews_ (struct initializer_list views)
 {
   char * SR.75;
@@ -1238,7 +1245,6 @@
   goto <bb 4>; [INV]
 
   <bb 3> :
-  p_35 = *__for_begin_3;
   total_size_36 = total_size_2 + 1;
   __for_begin_37 = __for_begin_3 + 8;
 
@@ -1264,7 +1270,7 @@
 
   <bb 7> :
   p_28 = *__for_begin_4;
-  sit = std::copy_n<const char*, long unsigned int, __gnu_cxx::__normal_iterator<char*, std::vector<char> > > (p_28, 1, sit);
+  std::copy_n<const char*, long unsigned int, __gnu_cxx::__normal_iterator<char*, std::vector<char> > > (p_28, 1, sit);
   __for_begin_30 = __for_begin_4 + 8;
 
   <bb 8> :
  # __for_begin_4 = PHI <views$_M_array_6(6), __for_begin_30(7)>
  if (__for_begin_4 != _19)
    goto <bb 7>; [INV]
  else
    goto <bb 9>; [INV]

so it seems we fail to consider the lhs used by the call itself.
Comment 3 Richard Biener 2021-07-01 09:06:43 UTC
struct X { int counter; };

struct X __attribute__((noipa)) foo (struct X x)
{
  x.counter++;
  if (x.counter == 5)
    __builtin_exit (0);
  return x;
}

int
main ()
{
  struct X x;
  x.counter = 0;
  for (int i = 0; i < 10; ++i)
    x = foo (x);
  __builtin_abort ();
}
Comment 4 Richard Biener 2021-07-01 09:19:08 UTC
Created attachment 51093 [details]
untested patch

I will test this.  Self-assignments x = x are handled explicitely already.
Comment 5 GCC Commits 2021-07-01 10:31:38 UTC
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:4a87605938428f6c4c62d5b92cfc183cd2b2554e

commit r12-1952-g4a87605938428f6c4c62d5b92cfc183cd2b2554e
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Jul 1 11:16:01 2021 +0200

    tree-optimization/101278 - handle self-use in DSE analysis
    
    DSE store classification short-cuts the to-be classified stmt itself
    from chaining but fails to first check whether the store uses itself
    which can be the case when it is a call with the LHS also passed by
    value as argument.
    
    2021-07-01  Richard Biener  <rguenther@suse.de>
    
            PR tree-optimization/101278
            * tree-ssa-dse.c (dse_classify_store): First check for
            uses, then ignore stmt for chaining purposes.
    
            * gcc.dg/torture/pr101278.c: New testcase.
Comment 6 Richard Biener 2021-07-01 10:32:14 UTC
Fixed.