Bug 55035 - reload1.c:3766:41: error: ‘orig_dup[0]’ may be used uninitialized in this function (for fr30, microblaze, moxie, rl78)
Summary: reload1.c:3766:41: error: ‘orig_dup[0]’ may be used uninitialized in this fun...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: build, diagnostic
: 67475 (view as bug list)
Depends on:
Blocks: 44756
  Show dependency treegraph
 
Reported: 2012-10-23 14:23 UTC by Jorn Wolfgang Rennecke
Modified: 2015-12-07 08:50 UTC (History)
9 users (show)

See Also:
Host:
Target: cr16-elf,fr30-elf,lm32-elf,lm32-rtems,lm32-uclinux,microblaze-elf,microblaze-linux,moxie-elf,moxie-rtems,moxie-uclinux,nios2-elf,nios2-linux-gnu,nios2-rtems,rl78-elf,sparcv9-sun-solaris2*
Build:
Known to work:
Known to fail: 4.8.0, 4.9.1, 5.0, 6.0
Last reconfirmed: 2015-05-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jorn Wolfgang Rennecke 2012-10-23 14:23:00 UTC
g++ -c   -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    ../../../gcc/gcc/reload1.c -o reload1.o
../../../gcc/gcc/reload1.c: In function ‘void calculate_elim_costs_all_insns()’:
../../../gcc/gcc/reload1.c:3766:41: error: ‘orig_dup[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     *recog_data.dup_loc[i] = orig_dup[i];
                                         ^
../../../gcc/gcc/reload1.c:3642:7: note: ‘orig_dup[0]’ was declared here
   rtx orig_dup[MAX_RECOG_OPERANDS];
       ^
cc1plus: all warnings being treated as errors
make[2]: *** [reload1.o] Error 1
Comment 1 Hans-Peter Nilsson 2014-07-27 22:29:25 UTC
Also seen for gcc-4.9.1 ppc64-linux host (gcc110) same targets this far (considering the PR author probably same script, config-list.mk; except with ada removed).
Comment 2 Hans-Peter Nilsson 2014-07-27 22:31:51 UTC
(In reply to Hans-Peter Nilsson from comment #1)
> Also seen for gcc-4.9.1 ppc64-linux host (gcc110)

I.e. host gcc-4.9.1 on gcc110 compiling trunk r212879.
Comment 3 Hans-Peter Nilsson 2014-07-29 11:05:15 UTC
(In reply to Hans-Peter Nilsson from comment #2)
> I.e. host gcc-4.9.1 on gcc110 compiling trunk r212879.

Also seen with trunk r212879 compiling trunk r212879, so these poor targets would have broken native builds.  Thankfully apparently none have self-hosted systems at this time.
Comment 4 Bernhard Reutner-Fischer 2015-04-09 19:16:38 UTC
Reconfirmed.

Nowadays (trunk@221914) also breaks all-gcc for nios2-linux-gnu.
Reminds me of bug #36550

Smallish testcase:

$ cat reload1.i ; echo EOF
/* PR target/55035 */
/* { dg-do compile } */
/* { dg-options "-O2 -W -Wall -Werror" } */
struct rtx_def;
typedef struct rtx_def *rtx;
enum rtx_code {
  UNKNOWN,
  INSN, 
  ASM_INPUT,
  CLOBBER
};
struct rtx_def {
  enum rtx_code code: 4;
};
class rtx_def;
class rtx_insn : public rtx_def {};
struct recog_data_d
{
  rtx operand[30];
  rtx *operand_loc[30];
  rtx *dup_loc[1];
  char dup_num[1];
  char n_operands;
  char n_dups;
};
extern struct recog_data_d recog_data;
extern int get_int(void);
void
elimination_costs_in_insn (rtx_insn *insn)
{
  int icode = get_int ();
  int i;
  rtx orig_operand[30];
  rtx orig_dup[30];
  if (icode < 0)
    {
      if (((enum rtx_code) insn->code) == INSN)
        __builtin_abort();
      return;
    }
  for (i = 0; i < recog_data.n_dups; i++)
    orig_dup[i] = *recog_data.dup_loc[i];
  for (i = 0; i < recog_data.n_operands; i++)
    {
      orig_operand[i] = recog_data.operand[i];
      if (orig_operand[i]->code == CLOBBER)
        *recog_data.operand_loc[i] = 0;
    }
  for (i = 0; i < recog_data.n_dups; i++)
    *recog_data.dup_loc[i]
      = *recog_data.operand_loc[(int) recog_data.dup_num[i]];
  for (i = 0; i < recog_data.n_dups; i++)
    *recog_data.dup_loc[i] = orig_dup[i];
}
EOF

$ g++ -O2 -W -Wall -Werror -c reload1.i -o reload1.o
reload1.i: In function ‘void elimination_costs_in_insn(rtx_insn*)’:
reload1.i:53:41: error: ‘orig_dup[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     *recog_data.dup_loc[i] = orig_dup[i];
                                         ^
cc1plus: all warnings being treated as errors
Comment 5 Mikhail Maltsev 2015-05-20 01:06:41 UTC
Reduced testcase:

$ cat ./warn2.cc
int arr[1];
int N;

void bar();

void foo()
{
  int temp[2];
  for (int i = 0; i < N; i++)
    temp[i] = arr[i];
  bar();
  for (int i = 0; i < N; i++)
    arr[i] = temp[i];
}


$ /opt/gcc-6.0.0/bin/g++ -c -Wall -O2 -fdump-tree-all ./warn2.cc
./warn2.cc: In function 'void foo()':
./warn2.cc:13:21: warning: 'temp[0]' may be used uninitialized in this function [-Wmaybe-uninitialized]
     arr[i] = temp[i];

$ /opt/gcc-6.0.0/bin/g++ -c -Wall -O2 -fno-tree-sra ./warn2.cc
(no warning)

GCC performs scalar replacement of "temp" and unrolls both loops. N is global and can be modified by bar(). The final GIMPLE looks like this:

$ cat ./warn2.cc.190t.optimized

void foo() ()
{
  int temp$0;
  int N.0_18;
  int N.0_24;

  <bb 2>:
  N.0_24 = N;
  if (N.0_24 <= 0)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 3>:
  temp$0_25 = arr[0];

  <bb 4>:
  # temp$0_26 = PHI <temp$0_25(3), temp$0_27(D)(2)>
  bar ();
  N.0_18 = N;
  if (N.0_18 <= 0)
    goto <bb 6>;
  else
    goto <bb 5>;

  <bb 5>:
  arr[0] = temp$0_26;

  <bb 6>:
  return;
}

Here temp$0_27(D)(2) is the "uninitialized" case. In the original testcase recog_data.n_dups is considered escaped and orig_dup[0] is treated the same way as temp[0].
Comment 6 Jeffrey A. Law 2015-07-24 18:58:24 UTC
Per this discussion:

https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01996.html

It's believed the reduced testcase in c#5 is an invalid reduction.

A cursory review of the code in c#4 indicates that testcase is a valid reduction.  We ought to be able to determine that the writes inside the two inner loops do not clobber recog_data.n_dups and thus the first and last loops iterate over the same space and every read of orig_dup[] in the last loop will have had a value set in the first loop.

There's almost certainly a hideously complex missed jump threading opportunity in here.  Conceptually it'd be exposed by duplicating the two inner loops, one duplicate would be reached when n_dups is zero the other when n_dups is nonzero after the first loop.   The first duplicate will bypass the last loop the second duplicate would execute the final loop.  The net result of all that copying and cfg transformations should in theory make the false positive warning go away.
Comment 7 Mikhail Maltsev 2015-07-26 08:56:50 UTC
(quote from the mentioned discussion):
> We then have the call to bar() which could change the value of N to 1. We then hit the second loop and read temp[0] which is uninitialized. The warning for the reduced testcase is correct.

Oops, I thought that GCC is more conservative here. Then, indeed, the reduction is invalid. Here is another attempt (tested against trunk revision r226223 and this time it does not contain any calls, but I'm still not sure, whether it's correct):

typedef struct rtx_def *rtx;
struct recog_data_d
{
  rtx operand;
  char n_dups;
};

rtx *operand_loc;
rtx dup_loc;

struct recog_data_d recog_data;

void elimination_costs_in_insn ()
{
  rtx orig_dup;
  if (recog_data.n_dups)
      orig_dup = dup_loc;
  *operand_loc = 0;
  if (recog_data.n_dups)
      dup_loc = orig_dup;
}

A warning is issued even at -O1. If I remove "rtx operand" field from struct recog_data_d (or if I change the type of n_dups to int), then it disappears at -O2 (but still present at -O1).
Comment 8 Jeffrey A. Law 2015-07-27 15:43:47 UTC
There are cases where the compiler can look into the implementation of bar() and make better decisions about how memory objects may be effected.  This isn't one of them ;-)
Comment 9 rsandifo@gcc.gnu.org 2015-08-13 20:23:06 UTC
Author: rsandifo
Date: Thu Aug 13 20:22:34 2015
New Revision: 226874

URL: https://gcc.gnu.org/viewcvs?rev=226874&root=gcc&view=rev
Log:
gcc/
	PR bootstrap/55035
	* reload1.c (elimination_costs_in_insn): Make it obvious to the
	compiler that the n_dups and n_operands loop bounds are invariant.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload1.c
Comment 10 rsandifo@gcc.gnu.org 2015-08-13 20:29:14 UTC
Closing as fixed per Jeff's proposal in:

    https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00634.html

Jeff said he'd open a second BZ issue for the testcase in c#4
which (unlike the reload1.c code) shouldn't trigger a warning.
Comment 11 Rainer Orth 2015-08-26 07:28:40 UTC
As already mentioned on gcc-patches, the patch broke sparcv9-sun-solaris2.*
bootstrap (unlike sparc-sun-solaris2.*, which is still fine):

/vol/gcc/src/hg/trunk/local/gcc/reload1.c: In function 'void elimination_costs_in_insn(rtx_insn*)':
/vol/gcc/src/hg/trunk/local/gcc/reload1.c:3772:41: error: 'orig_dup[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     *recog_data.dup_loc[i] = orig_dup[i];
                                         ^
/vol/gcc/src/hg/trunk/local/gcc/reload1.c:3772:41: error: 'orig_dup[0]' may be used uninitialized in this function [-Werror=maybe-uninitialized]

  Rainer
Comment 12 Hans-Peter Nilsson 2015-09-09 00:36:41 UTC
*** Bug 67475 has been marked as a duplicate of this bug. ***
Comment 13 Eric Botcazou 2015-12-07 08:50:28 UTC
SPARC 64-bit bootstrap works for me.