Bug 90132 - make bootstrap fails with -O3 (gcc9 snapshot 20190414)
Summary: make bootstrap fails with -O3 (gcc9 snapshot 20190414)
Status: RESOLVED WONTFIX
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2019-04-17 17:30 UTC by Jason Mancini
Modified: 2020-04-22 22:02 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-04-17 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Mancini 2019-04-17 17:30:42 UTC
(I don't know what the correct 'component' is, please adjust as necessary.)

On x86_64 with gcc9 snapshot 20190414 while building c,c++ targets, I noticed that -O2 works, but -O3 fails with:

checking for vsprintf... checking whether byte ordering is bigendian...
../../gcc-9-20190414/libdecnumber/decNumber.c: In function 'decNumberPower':
cc1: error: '__builtin_memcpy' reading 2 or more bytes from a region of size 0
[-Werror=stringop-overflow=]
yes
Comment 1 Martin Sebor 2019-04-17 20:46:27 UTC
I can reproduce the warning without -fPIC:

$ (cd /build/gcc-svn-self/libdecnumber  && /build/gcc-svn/gcc/xgcc -B /build/gcc-svn/gcc  -I/src/gcc/svn/libdecnumber -I.  -O3 -g3 -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wmissing-format-attribute -Wcast-qual -pedantic -Wno-long-long  -fno-lto -I/src/gcc/svn/libdecnumber -I. -S /src/gcc/svn/libdecnumber/decNumber.c)
/src/gcc/svn/libdecnumber/decNumber.c: In function ‘decNumberPower’:
cc1: warning: ‘__builtin_memcpy’ reading 2 or more bytes from a region of size 0 [-Wstringop-overflow=]
$ 

The variable being read is dnOne:

  <bb 94> [local count: 3614127]:
  # iftmp.231_443 = PHI <iftmp.231_438(92), iftmp.231_442(93)>
  smsup_444 = &dnOne.lsu + iftmp.231_443;
  if (&MEM[(void *)&dnOne + 12B] < smsup_444)
    goto <bb 95>; [89.00%]
  else
    goto <bb 97>; [11.00%]

  <bb 95> [local count: 3216573]:
  _110 = (unsigned long) smsup_444;
  _550 = (unsigned long) &MEM[(void *)&dnOne + 12B];
  _568 = ~_550;
  _525 = _110 + _568;
  _524 = _525 >> 1;
  _520 = _524 + 1;
  _518 = _520 * 2;
  __builtin_memcpy (d_434, &MEM[(void *)&dnOne + 12B], _518);

The offset 12 in the MEM_REF corresponds to sizeof (decnumber).  compute_builtin_object_size() returns zero for &MEM[(void *)&dnOne + 12B] and _518's range is ~[1, 1], so either zero or 2 or more.  The warning doesn't consider the unlikely case of zero and treats the anti-range as [2, SIZE_MAX].

The missing location information suggests the memcpy call is the result of some transformation.  The reference to smsup implies it comes from this loop in decNumberCopy() (the only function in the translation unit that defines the smsup variable):

  if (src->digits>3) {
    const uint16_t *smsup, *s;
    uint16_t *d;


    d=dest->lsu+1;
    smsup=src->lsu+((src->digits)<=49?d2utable[src->digits]:((src->digits)+3 -1)/3);
    for (s=src->lsu+1; s<smsup; s++, d++) *d=*s;
    }
  return dest;
  }

The trailing one-element decnumber::lsu array is being used as a flexible-array member with decnumber::digits corresponding to the number of elements.  The value src->lsu+1 is past the end of the dnOne object but smsup has one of two values: src->lsu + d2utable[src->digits] or src->lsu + 1, and the first one doesn't look determinate because dnOne is also used as some sort of a temporary by decNumberPower and its digits member (initially set to 1) is overwritten.  So the loop gets transformed into memcpy that reads some number other than 1 from an object of size zero.

Adding an assertion like in the otherwise untested patch below should let GCC see that dnOne.digits is unchanged.  I don't know enough about the library to tell if that's actually correct or what the correct macro or value to use here might be.

===================================================================
--- libdecnumber/decNumber.c	(revision 270418)
+++ libdecnumber/decNumber.c	(working copy)
@@ -2188,6 +2188,8 @@ decNumber * decNumberPower(decNumber *res, const d
 	    }
 	  /* [inv now points to big-enough buffer or allocated storage] */
 	  decNumberCopy(inv, dac);	/* copy the 1/lhs */
+	  if (dnOne.digits > 1)
+	    __builtin_unreachable ();
 	  decNumberCopy(dac, &dnOne);	/* restore acc=1 */
 	  lhs=inv;			/* .. and go forward with new lhs */
 	#if DECSUBSET
Comment 2 Martin Sebor 2019-04-17 20:58:41 UTC
I think the following test case reproduces what's going on in decNumber.c.  Both GCC 8 and 9 issue the warning (IIRC, the warning was added in GCC 7 for writes but enhanced to reads in GCC 8).

$ cat a.c && gcc -O3 -S -Wall -fdump-tree-optimized=/dev/stdout a.c
struct S
{
  int n, a[1];
};

static inline void f (int *d, const struct S *p)
{
  const int *e = p->n > 1 ? p->a + p->n : p->a + 1;

  for (const int *s = p->a + 1; s < e; ++s, ++d)
    *d = *s;
}

void g (struct S*);

void h (int *d)
{
  struct S s = { 0 };
  g (&s);
  f (d, &s);
}

;; Function h (h, funcdef_no=1, decl_uid=1920, cgraph_uid=2, symbol_order=1)

Removing basic block 6
Removing basic block 7
h (int * d)
{
  struct S s;
  int _6;
  long unsigned int _7;
  long unsigned int _8;
  const int * iftmp.0_9;
  unsigned long _15;
  unsigned long _16;
  sizetype _20;
  unsigned long _23;
  sizetype _24;
  unsigned long _25;
  unsigned long _26;

  <bb 2> [local count: 118111600]:
  s = {};
  g (&s);
  _6 = s.n;
  if (_6 > 1)
    goto <bb 3>; [59.00%]
  else
    goto <bb 5>; [41.00%]

  <bb 3> [local count: 69685844]:
  _7 = (long unsigned int) _6;
  _8 = _7 * 4;
  iftmp.0_9 = &s.a + _8;
  if (&MEM[(void *)&s + 8B] < iftmp.0_9)
    goto <bb 4>; [93.20%]
  else
    goto <bb 5>; [6.80%]

  <bb 4> [local count: 64949629]:
  _26 = (unsigned long) iftmp.0_9;
  _16 = (unsigned long) &MEM[(void *)&s + 8B];
  _15 = ~_16;
  _23 = _15 + _26;
  _25 = _23 >> 2;
  _24 = _25 + 1;
  _20 = _24 * 4;
  __builtin_memcpy (d_4(D), &MEM[(void *)&s + 8B], _20); [tail call]

  <bb 5> [local count: 118111601]:
  s ={v} {CLOBBER};
  return;

}


a.c: In function ‘h’:
cc1: warning: ‘__builtin_memcpy’ reading 4 or more bytes from a region of size 0 [-Wstringop-overflow=]
Comment 3 Richard Biener 2019-04-18 08:45:43 UTC
I guess the basic issue again that we warn for unreachable code.  Note libdecnumber is barely maintained and quite a big mess...
Comment 4 Jakub Jelinek 2019-04-18 10:44:50 UTC
Plus when using non-standard compilation flags during bootstrap --disable-werror should be used, we only support no warnings for the default set of flags and normal and profiledbootstrap (not sure about lto bootstraps).
Comment 5 Jason Mancini 2019-04-18 14:09:59 UTC
But bootstrap-O3 is a documented target, which is equivalent to BOOT_CFLAGS='-g -O3', per https://gcc.gnu.org/install/build.html
Comment 6 Jakub Jelinek 2019-04-18 14:27:39 UTC
Even in that case, several times in the past we've just decided to recommend --disable-werror in those cases instead of adding too ugly workarounds for some warnings (while for the default we always add workarounds or disable warnings or whatever is needed to make them build cleanly).
The thing is, with too many different optimization options vs. different targets etc. the support matrix for warning-free builds gets too large.
Comment 7 Jason Mancini 2019-04-18 17:20:41 UTC
Okay! The patch in Comment #1 worked for me. Someone else can fix or reject as it's not an important bug then.
Comment 8 Martin Sebor 2020-04-22 22:02:10 UTC
I guess won't fix then per comment #6 and comment #7.