User account creation filtered due to spam.

Bug 43987 - [4.5 Regression] type-punning causes broken binaries unless -O0 is used
Summary: [4.5 Regression] type-punning causes broken binaries unless -O0 is used
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.5.0
: P2 normal
Target Milestone: 4.5.1
Assignee: Richard Biener
URL:
Keywords: alias, wrong-code
: 44060 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-05-04 22:46 UTC by Thomas Bächler
Modified: 2010-05-21 08:16 UTC (History)
5 users (show)

See Also:
Host: x86_64-unknown-linux-gnu / i686-pc-linux-gnu
Target: x86_64-unknown-linux-gnu / i686-pc-linux-gnu
Build: x86_64-unknown-linux-gnu / i686-pc-linux-gnu
Known to work: 4.4.3 4.6.0
Known to fail: 4.5.0
Last reconfirmed: 2010-05-05 13:16:40


Attachments
Preprocessor output of sed.c (40.41 KB, application/octet-stream)
2010-05-05 10:53 UTC, Thomas Bächler
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Bächler 2010-05-04 22:46:15 UTC
I noticed this when building busybox 1.16.1, but I also saw this in other code:

When building with -O2 I get warnings like:
'warning: dereferencing type-punned pointer will break strict-aliasing rules'

The produced binary is broken, in the case of busybox, the 'sed' applet segfaults.

When adding -fno-strict-aliasing, the warnings disappear, but the resulting binary is still broken (with no warning or error indicating that something is broken). The same also happens with -O1 or -Os, only a -O0 binary will work.

This used to work fine in gcc 4.4. I don't know how a test case would look like, as I don't know the code that breaks that closely.

In my opinion, one of the following should happen:
a) gcc 4.5 should produce code that works
b) gcc 4.5 should throw a warning or error indicating that the code is incorrect
Comment 1 Andrew Pinski 2010-05-04 22:48:27 UTC
Can you try also -fwrapv?  There could be other issues with the code besides just aliasing issues.  Maybe there is a buffer overflow that shows up now with the new optimizers.
Comment 2 Andrew Pinski 2010-05-04 22:48:49 UTC
Also without a testcase it is hard to figure out what is going on.
Comment 3 Thomas Bächler 2010-05-04 23:06:04 UTC
What is -fwrapv supposed to do? I don't see any changes in behaviour or compiler output.


As I said, I don't know exactly what to put into a test case (I didn't write any of the code that breaks), but I can try my best to find one.


I first encountered this problem in gap (http://www.gap-system.org), and it was fixed by this patch:
http://www.math.leidenuniv.nl/~wpalenst/sage/gap_gcc45_strict_aliasing.patch
(look at the sysinfo.c part, that was what caused the problems apparently).

I now encountered it again in busybox's 'sed' applet. I will try to take that code apart until I find out what's going wrong and hopefully produce a test case.
Comment 4 Andrew Pinski 2010-05-04 23:08:33 UTC
>What is -fwrapv supposed to do? I

Changes the behavior of signed integer overflow to be defined as wrapping rather than being undefined.
Comment 5 Thomas Bächler 2010-05-04 23:49:49 UTC
I am still not able to produce a short test case, but maybe this will help:
http://git.busybox.net/busybox/tree/editors/sed.c?h=1_16_stable&id=1_16_1#n738

In the function add_input_file(), the 'file' argument is not a NULL pointer, but at the end of the function, G.input_file_list[G.input_file_count-1]==NULL (and !=file in particular), which ultimately leads to the segfault.
Comment 6 Andrew Pinski 2010-05-04 23:53:16 UTC
How is xrealloc_vector defined?
Comment 7 Thomas Bächler 2010-05-05 07:23:32 UTC
It has its own file: http://git.busybox.net/busybox/tree/libbb/xrealloc_vector.c?h=1_16_stable&id=1_16_1
Comment 8 Richard Biener 2010-05-05 10:15:33 UTC
Waiting for a testcase (which could be just preprocessed source of
sed.c).

Can you reproduce the issue when you add -fno-inline?

Can you reproduce the issue with the current 4.5 branch?

As you can reproduce with -O1 I doubt this is an issue with type-punning.
Comment 9 Thomas Bächler 2010-05-05 10:53:43 UTC
Created attachment 20561 [details]
Preprocessor output of sed.c
Comment 10 Thomas Bächler 2010-05-05 10:58:29 UTC
(In reply to comment #8)
> Waiting for a testcase (which could be just preprocessed source of
> sed.c).

Attached. If I find a better (shorter) test case, I'll add that too.

> Can you reproduce the issue when you add -fno-inline?

Yes.

> Can you reproduce the issue with the current 4.5 branch?

We've scheduled to build the latest snapshot next week. Maybe I can do it before then, I'll let you know as soon as I get to try.

> As you can reproduce with -O1 I doubt this is an issue with type-punning.

That was my thought too, but it seems to happen only in places where type-punning occurs.
Comment 11 Thomas Bächler 2010-05-05 11:57:33 UTC
The problem persists with the 4.5-20100429 snapshot.
Comment 12 Richard Biener 2010-05-05 13:13:22 UTC
I see

extern char bb_common_bufsiz1[COMMON_BUFSIZE];

static void add_input_file(FILE *file)
{
 fprintf(stderr, "Adding input file %x.\n", file);
 (*(struct globals*)&bb_common_bufsiz1).input_file_list = xrealloc_vector_helper(((*(struct globals*)&bb_common_bufsiz1).input_file_list), (sizeof(((*(struct globals*)&bb_common_bufsiz1).input_file_list)[0]) << 8) + (2), ((*(struct globals*)&bb_common_bufsiz1).input_file_count));
 (*(struct globals*)&bb_common_bufsiz1).input_file_list[(*(struct globals*)&bb_common_bufsiz1).input_file_count++] = file;
 fprintf(stderr, "Added input file %x.\n", (*(struct globals*)&bb_common_bufsiz1).input_file_list[(*(struct globals*)&bb_common_bufsiz1).input_file_count-1]);
}

and the store to (*(struct globals*)&bb_common_bufsiz1).input_file_list[(*(struct globals*)&bb_common_bufsiz1).input_file_count++] is indeed DSEd
at -O1 or by DCE if the printfs are removed.

Testcase:

extern char B[256 * sizeof(void *)];
typedef void *FILE;
typedef struct globals {
    int c;
    FILE **l;
} T;
void* xrealloc(void *vector, unsigned sizeof_and_shift, int idx);
void add_input_file(FILE *file)
{
  (*(T*)&B).l = xrealloc(((*(T*)&B).l),
                         (sizeof(((*(T*)&B).l)[0]) << 8) + (2),
                         ((*(T*)&B).c));
  (*(T*)&B).l[(*(T*)&B).c++] = file;
}
Comment 13 Richard Biener 2010-05-05 13:16:40 UTC
Mine.
Comment 14 Richard Biener 2010-05-05 13:28:26 UTC
Bah.  Points-to analysis says:

  # PT = { B } (glob)
  B.0_8 = (struct T *) &B;
  # PT =
  D.2731_9 = B.0_8->l;

because B cannot have pointers.  Well.  That's obviously because points-to
uses types to determine that fact.  Something that keeps points-to sets
and the graph small - but at the same time it causes us to optimize things
we previously didn't with -fno-strict-aliasing.  The same will apply for
accesses with ref-all pointers :/  Which means we can't really do this
optimization at all.  Ugh.

And yes, the code violates C aliasing rules.
Comment 15 Richard Biener 2010-05-05 13:56:32 UTC
Testcase that does not violate strict aliasing rules (and thus should not
be miscompiled at -O2 or -Os either):

extern char B[256 * sizeof(void *)];
typedef void *FILE;
typedef struct globals {
    int c;
    FILE **l;
} __attribute__((may_alias)) T;
void* xrealloc(void *vector, unsigned sizeof_and_shift, int idx);
void add_input_file(FILE *file)
{
  (*(T*)&B).l = xrealloc(((*(T*)&B).l),
                         (sizeof(((*(T*)&B).l)[0]) << 8) + (2),
                         ((*(T*)&B).c));
  (*(T*)&B).l[(*(T*)&B).c++] = file;
}


I have a patch.  Workaround for 4.5.0: -fno-pta.
Comment 16 Richard Biener 2010-05-05 14:01:50 UTC
Runtime testcase:

char B[256 * sizeof(void *)];
typedef void *FILE;
typedef struct globals {
    int c;
    FILE *l;
} __attribute__((may_alias)) T;
void add_input_file(FILE *file)
{
  (*(T*)&B).l[0] = file;
}
extern void abort (void);
int main()
{
  FILE x;
  (*(T*)&B).l = &x;
  add_input_file ((void *)-1);
  if ((*(T*)&B).l[0] != (void *)-1)
    abort ();
  return 0;
}
Comment 17 Thomas Bächler 2010-05-05 14:10:46 UTC
> I have a patch.  Workaround for 4.5.0: -fno-pta.

There is no -fno-pta option in 4.5.0, but I can confirm that -fno-tree-pta fixes the problem in the testcase and in busybox.
Comment 18 rguenther@suse.de 2010-05-05 14:13:08 UTC
Subject: Re:  [4.5/4.6 Regression] type-punning causes
 broken binaries unless -O0 is used

On Wed, 5 May 2010, thomas at archlinux dot org wrote:

> ------- Comment #17 from thomas at archlinux dot org  2010-05-05 14:10 -------
> > I have a patch.  Workaround for 4.5.0: -fno-pta.
> 
> There is no -fno-pta option in 4.5.0, but I can confirm that -fno-tree-pta
> fixes the problem in the testcase and in busybox.

Indeed.  It's -fno-tree-pta.

Richard.
Comment 19 Richard Biener 2010-05-06 08:53:38 UTC
Subject: Bug 43987

Author: rguenth
Date: Thu May  6 08:53:19 2010
New Revision: 159098

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159098
Log:
2010-05-06  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43987
	* tree-ssa-structalias.c (could_have_pointers): For possibly
	address-taken variables force pointers to be recorded.
	(create_variable_info_for_1): Likewise.
	(push_fields_onto_fieldstack): Pass in wheter all fields
	must have pointers.
	(find_func_aliases): Query types instead of vars whether
	they contain pointers where appropriate.

	* gcc.c-torture/execute/pr43987.c: New testcase.
	* gcc.dg/torture/pta-escape-1.c: Adjust.
	* gcc.dg/tree-ssa/pta-escape-1.c: Likewise.
	* gcc.dg/tree-ssa/pta-escape-2.c: Likewise.
	* gcc.dg/tree-ssa/pta-escape-3.c: Likewise.
	* gcc.dg/ipa/ipa-pta-11.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr43987.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/gcc.dg/ipa/ipa-pta-11.c
    trunk/gcc/testsuite/gcc.dg/torture/pta-escape-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-2.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-3.c
    trunk/gcc/tree-ssa-structalias.c

Comment 20 Richard Biener 2010-05-06 08:54:02 UTC
Fixed on the trunk.  Queued for backporting to 4.5.1.
Comment 21 Bernhard Loos 2010-05-10 16:48:31 UTC
*** Bug 44060 has been marked as a duplicate of this bug. ***
Comment 22 Richard Biener 2010-05-19 14:48:46 UTC
Subject: Bug 43987

Author: rguenth
Date: Wed May 19 14:48:24 2010
New Revision: 159567

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=159567
Log:
2010-05-19  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43987
	* tree-ssa-structalias.c (could_have_pointers): For possibly
	address-taken variables force pointers to be recorded.
	(create_variable_info_for_1): Likewise.
	(push_fields_onto_fieldstack): Pass in wheter all fields
	must have pointers.
	(find_func_aliases): Query types instead of vars whether
	they contain pointers where appropriate.

	* gcc.c-torture/execute/pr43987.c: New testcase.
	* gcc.dg/torture/pta-escape-1.c: Adjust.
	* gcc.dg/tree-ssa/pta-escape-1.c: Likewise.
	* gcc.dg/tree-ssa/pta-escape-2.c: Likewise.
	* gcc.dg/tree-ssa/pta-escape-3.c: Likewise.
	* gcc.dg/torture/ipa-pta-1.c: Likewise.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.c-torture/execute/pr43987.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/torture/ipa-pta-1.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/torture/pta-escape-1.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-1.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-2.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/pta-escape-3.c
    branches/gcc-4_5-branch/gcc/tree-ssa-structalias.c

Comment 23 Richard Biener 2010-05-19 15:08:33 UTC
Fixed.
Comment 24 Thomas Bächler 2010-05-21 08:16:01 UTC
Preliminary tests with a recent gcc snapshot from the 4.5 branch shows that the problem is entirely solved by this patch (maybe in combination with other fixes).

Thanks for the quick responses and fixes.