* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
@ 2011-09-20 10:49 ` jakub at gcc dot gnu.org
2011-09-20 13:47 ` hjl.tools at gmail dot com
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-09-20 10:49 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
Target Milestone|--- |4.7.0
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-09-20 09:59:49 UTC ---
Seems to be caused by PR48571, we throw away the important info (that the
access was through a.buf1 rather than &a), which is essential for
-D_FORTIFY_SOURCE=2.
The change happens already during gimplification:
- strcpy (&a.buf1[4], D.2732);
+ strcpy (&MEM[(void *)&a + 4B], D.2732);
while in *.original it was
strcpy ((char *) &a.buf1 + 4, str1 + 5);
Not reconstrucing the array ref is fine, but before *.objsz pass we really
shouldn't throw away the buf1 from it, so it should be tmp = &a.buf1 + 4;
-D_FORTIFY_SOURCE=2 cares whether the user wrote
strcpy ((char *) &a + 4, ...); (in which case it allows to overwrite the
whole object) or strcpy ((char *) &a.buf1 + 4, ...); (in which case it is
allowed to overwrite just the buf1 field).
Richard, can you please have a look at this?
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
2011-09-20 10:49 ` [Bug middle-end/50460] " jakub at gcc dot gnu.org
@ 2011-09-20 13:47 ` hjl.tools at gmail dot com
2011-09-22 22:06 ` rguenth at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hjl.tools at gmail dot com @ 2011-09-20 13:47 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
H.J. Lu <hjl.tools at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2011-09-20 13:34:15 UTC ---
It is caused by revision 178312:
http://gcc.gnu.org/ml/gcc-cvs/2011-08/msg01330.html
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
2011-09-20 10:49 ` [Bug middle-end/50460] " jakub at gcc dot gnu.org
2011-09-20 13:47 ` hjl.tools at gmail dot com
@ 2011-09-22 22:06 ` rguenth at gcc dot gnu.org
2011-09-23 8:34 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-22 22:06 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2011-09-22
AssignedTo|unassigned at gcc dot |rguenth at gcc dot gnu.org
|gnu.org |
Ever Confirmed|0 |1
--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-22 21:39:11 UTC ---
(In reply to comment #1)
> Seems to be caused by PR48571, we throw away the important info (that the
> access was through a.buf1 rather than &a), which is essential for
> -D_FORTIFY_SOURCE=2.
> The change happens already during gimplification:
> - strcpy (&a.buf1[4], D.2732);
> + strcpy (&MEM[(void *)&a + 4B], D.2732);
> while in *.original it was
> strcpy ((char *) &a.buf1 + 4, str1 + 5);
> Not reconstrucing the array ref is fine, but before *.objsz pass we really
> shouldn't throw away the buf1 from it, so it should be tmp = &a.buf1 + 4;
> -D_FORTIFY_SOURCE=2 cares whether the user wrote
> strcpy ((char *) &a + 4, ...); (in which case it allows to overwrite the
> whole object) or strcpy ((char *) &a.buf1 + 4, ...); (in which case it is
> allowed to overwrite just the buf1 field).
>
> Richard, can you please have a look at this?
Sure. Avoiding this during gimplification should be possible - but
preserving &a.buf1 + 4 in some form until objsz will be impossible
I fear. It looks like objsz relies on something that GIMPLE does not
provide.
I'm sure we don't guarantee that any obfuscation of a.buf1 + 4 doesn't
trip on fortify=2, do we?
I suppose the error is in &MEM[(void *)&a + 4B] objsz handling (again).
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (2 preceding siblings ...)
2011-09-22 22:06 ` rguenth at gcc dot gnu.org
@ 2011-09-23 8:34 ` jakub at gcc dot gnu.org
2011-09-25 11:24 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-09-23 8:34 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-09-23 07:26:10 UTC ---
Looking at:
const char *str1 = "JIHGFEDCBA";
#define strcpy(x,y) __builtin___strcpy_chk (x, y, __builtin_object_size (x, 1))
int
f1 (void)
{
struct A { char buf1[9]; char buf2[1]; } a;
strcpy (a.buf1 + (0 + 4), str1 + 5);
return 0;
}
int
f2 (void)
{
struct A { char buf1[9]; char buf2[1]; } a;
strcpy ((char *) &a + (0 + 4), str1 + 5);
return 0;
}
int
f3 (void)
{
struct A { char buf1[9]; char buf2[1]; } a;
char *p = (char *) &a;
strcpy (p + (0 + 4), str1 + 5);
return 0;
}
int
f4 (void)
{
struct A { char buf0; char buf1[9]; char buf2[1]; } a;
char *p = (char *) &a;
strcpy (p + (0 + 5), str1 + 5);
return 0;
}
int
f5 (void)
{
struct A { char buf0; char buf1[9]; char buf2[1]; } a;
strcpy ((char *) &a + (0 + 5), str1 + 5);
return 0;
}
with GCC 4.4, seems we have always reconstructed it into &a.buf1[4].
So likely we want to reconstruct it from the MEM_REF in the *.objsz pass then.
If there is union involved, we probably want to reconstruct it to the
alternative with the largest possible __builtin_object_size (X, 1) resp.
smallest possible __builtin_object_size (X, 3).
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (3 preceding siblings ...)
2011-09-23 8:34 ` jakub at gcc dot gnu.org
@ 2011-09-25 11:24 ` rguenth at gcc dot gnu.org
2011-09-26 9:14 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-25 11:24 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-25 11:11:21 UTC ---
(In reply to comment #4)
> Looking at:
> const char *str1 = "JIHGFEDCBA";
> #define strcpy(x,y) __builtin___strcpy_chk (x, y, __builtin_object_size (x, 1))
>
> int
> f1 (void)
> {
> struct A { char buf1[9]; char buf2[1]; } a;
> strcpy (a.buf1 + (0 + 4), str1 + 5);
> return 0;
> }
>
> int
> f2 (void)
> {
> struct A { char buf1[9]; char buf2[1]; } a;
> strcpy ((char *) &a + (0 + 4), str1 + 5);
> return 0;
> }
>
> int
> f3 (void)
> {
> struct A { char buf1[9]; char buf2[1]; } a;
> char *p = (char *) &a;
> strcpy (p + (0 + 4), str1 + 5);
> return 0;
> }
>
> int
> f4 (void)
> {
> struct A { char buf0; char buf1[9]; char buf2[1]; } a;
> char *p = (char *) &a;
> strcpy (p + (0 + 5), str1 + 5);
> return 0;
> }
>
> int
> f5 (void)
> {
> struct A { char buf0; char buf1[9]; char buf2[1]; } a;
> strcpy ((char *) &a + (0 + 5), str1 + 5);
> return 0;
> }
>
> with GCC 4.4, seems we have always reconstructed it into &a.buf1[4].
> So likely we want to reconstruct it from the MEM_REF in the *.objsz pass then.
> If there is union involved, we probably want to reconstruct it to the
> alternative with the largest possible __builtin_object_size (X, 1) resp.
> smallest possible __builtin_object_size (X, 3).
I'm not sure. What's the C / fortify difference of a.buf1 + 9 vs. a.buf2?
Both would be MEM[&a, 9]. I suppose we didn't re-construct array-refs in
4.4 from
void *p = a.buf1;
char *q = p + 4;
so, did we fail with 4.4 here, too?
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (4 preceding siblings ...)
2011-09-25 11:24 ` rguenth at gcc dot gnu.org
@ 2011-09-26 9:14 ` jakub at gcc dot gnu.org
2011-09-28 12:29 ` rguenth at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-09-26 9:14 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-09-26 09:08:36 UTC ---
#define strcpy(x,y) __builtin___strcpy_chk (x, y, __builtin_object_size (x, 1))
int
f1 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy (a.buf1 + 9, "a");
return 0;
}
int
f2 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy (a.buf2 + 0, "a");
return 0;
}
int
f3 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy (a.buf1 + 10, "a");
return 0;
}
int
f4 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy (a.buf2 - 1, "a");
return 0;
}
int
f5 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy ((char *) &a + 10, "a");
return 0;
}
int
f6 (void)
{
struct A { char buf1[9]; char buf2[4]; } a;
strcpy ((char *) a.buf2 - 1, "a");
return 0;
}
used to warn in f{1,3,4,6} (and fail at runtime) and not in f{2,5} in
4.{1,2,3,4,6} (haven't checked 4.5), doesn't warn nor fail on the trunk.
So yes, 4.0+ clearly did some reconstruction, but only in limited cases (e.g.
when the &a is offsetted). Some field + offset remained COMPONENT_REF +
offset.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (5 preceding siblings ...)
2011-09-26 9:14 ` jakub at gcc dot gnu.org
@ 2011-09-28 12:29 ` rguenth at gcc dot gnu.org
2011-09-28 13:52 ` rguenth at gcc dot gnu.org
2011-09-28 14:01 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-28 12:29 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |diagnostic
--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-28 12:23:15 UTC ---
Btw, this is equivalent to a missing diagnostic, it's correctly not trapping
similar as to if it didn't know anything about the object that is refered to.
Indeed when I disable the folding during gimplification CCP comes along
and does
<bb 2>:
str1.0_1 = str1;
D.2732_2 = str1.0_1 + 5;
- D.2733_3 = &a.buf1 + 4;
- __dest_7 = (char * restrict) D.2733_3;
__src_8 = (const char * restrict) D.2732_2;
- D.2747_9 = __builtin_object_size (__dest_7, 1);
- D.2746_10 = __builtin___strcpy_chk (__dest_7, __src_8, D.2747_9);
- D.2746_12 = D.2746_10;
- D.2734_4 = 0;
- return D.2734_4;
+ D.2746_10 = __builtin___strcpy_chk (&MEM[(void *)&a + 4B], __src_8, 6);
+ return 0;
which is good, as the address is invariant.
So, short of moving the objsize pass way earlier (which I'm sure we don't
want to do), I don't see a good way to recover this diagnostic.
One possibility is to make sure try_move_mult_to_index handles the case
of &a.buf1 + 4, instead of just &a.buf1[0] + 4. I have a patch for that.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (6 preceding siblings ...)
2011-09-28 12:29 ` rguenth at gcc dot gnu.org
@ 2011-09-28 13:52 ` rguenth at gcc dot gnu.org
2011-09-28 14:01 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-28 13:52 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-28 13:47:16 UTC ---
Author: rguenth
Date: Wed Sep 28 13:47:12 2011
New Revision: 179313
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=179313
Log:
2011-09-28 Richard Guenther <rguenther@suse.de>
PR middle-end/50460
* fold-const.c (try_move_mult_to_index): Handle &a.array the
same as &a.array[0].
Modified:
trunk/gcc/ChangeLog
trunk/gcc/fold-const.c
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/50460] [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work
2011-09-20 8:16 [Bug middle-end/50460] New: [4.7 Regression] __builtin___strcpy_chk/__builtin_object_size don't work hjl.tools at gmail dot com
` (7 preceding siblings ...)
2011-09-28 13:52 ` rguenth at gcc dot gnu.org
@ 2011-09-28 14:01 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-28 14:01 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50460
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-28 13:47:57 UTC ---
Fixed^WWorked around.
^ permalink raw reply [flat|nested] 10+ messages in thread