Hi,
with the attached patch the read-side of the out of bounds accesses are fixed.
There is a single new test case pr57748-3.c that is derived from Martin's test
case.
The test case does only test the read access and does not depend on part 1 of
the
patch.
This patch was boot-strapped and regre
Hello,
this is a minor update to my previous version of this patch, (using a boolean
expand_reference,
instead of adding a new expand_modifier enum value):
I forgot to pass down the expand_reference value at the second expand_expr call
inside the
case VIEW_CONVERT_EXPR. Sorry for the inconveni
On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
wrote:
> Hi,
>
> with the attached patch the read-side of the out of bounds accesses are fixed.
> There is a single new test case pr57748-3.c that is derived from Martin's
> test case.
> The test case does only test the read access and does not depe
> Index: gcc/expr.c
> ===
> --- gcc/expr.c (revision 202778)
> +++ gcc/expr.c (working copy)
> @@ -9878,7 +9878,7 @@
> && modifier != EXPAND_STACK_PARM
> ? target : NULL_RTX),
>
Hi,
On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
> wrote:
> > Hi,
> >
> > with the attached patch the read-side of the out of bounds accesses are
> > fixed.
> > There is a single new test case pr57748-3.c that is derived from M
On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
> Hi,
>
> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
>> wrote:
>> > Hi,
>> >
>> > with the attached patch the read-side of the out of bounds accesses are
>> > fixed.
>> >
Hmm.,
On Tue, 24 Sep 2013 20:06:51, Martin Jambor wrote:
> Hi,
>
> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
>> wrote:
>>> Hi,
>>>
>>> with the attached patch the read-side of the out of bounds accesses are
>>> fixed.
>
Hi,
On Wed, Sep 25, 2013 at 11:05:21AM +0200, Richard Biener wrote:
> On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
> > Hi,
> >
> > On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
> >> On Tue, Sep 24, 2013 at 4:52 AM, Bernd Edlinger
> >> wrote:
> >> > Hi,
> >> >
> >> > wi
Hi,
On Wed, 25 Sep 2013 16:01:02, Martin Jambor wrote:
> Hi,
>
> On Wed, Sep 25, 2013 at 11:05:21AM +0200, Richard Biener wrote:
>> On Tue, Sep 24, 2013 at 8:06 PM, Martin Jambor wrote:
>>> Hi,
>>>
>>> On Tue, Sep 24, 2013 at 12:02:17PM +0200, Richard Biener wrote:
On Tue, Sep 24, 2013 at 4:
> So I still think my patch does the right thing.
>
> The rationale is:
>
> = expand_expr (tem,
> (TREE_CODE (TREE_TYPE (tem)) == UNION_TYPE
> && COMPLETE_TYPE_P (TREE_TYPE (tem))
> && (TREE_CODE (TYPE_SIZE (TR
Hi,
On Thu, 26 Sep 2013 11:34:02, Eric Botcazou wrote:
>
>> So I still think my patch does the right thing.
>>
>> The rationale is:
>>
>> = expand_expr (tem,
>> (TREE_CODE (TREE_TYPE (tem)) == UNION_TYPE
>> && COMPLETE_TYPE_P (TREE_TYPE (tem))
>> && (TREE_CODE (TYPE_SIZE (TREE_TYPE (tem)))
>> != I
> Sure, but the modifier is not meant to force something into memory,
> especially when it is already in an register. Remember, we are only
> talking of structures here, and we only want to access one member.
>
> It is more the other way round:
> It says: "You do not have to load the value in a re
Hi Eric,
On Fri, 27 Sep 2013 10:36:57, Eric Botcazou wrote:
>
>> Sure, but the modifier is not meant to force something into memory,
>> especially when it is already in an register. Remember, we are only
>> talking of structures here, and we only want to access one member.
>>
>> It is more the ot
> OK, what do you think of it now?
My take on this is that the Proper Fix(tm) has been posted by Martin:
http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00082.html
IMO it's a no-brainer, modulo the ABI concern. Everything else is more or
less clever stuff to paper over this original compute_recor
Hi,
On Tue, 8 Oct 2013 10:01:37, Eric Botcazou wrote:
>
>> OK, what do you think of it now?
>
> My take on this is that the Proper Fix(tm) has been posted by Martin:
> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00082.html
> IMO it's a no-brainer, modulo the ABI concern. Everything else is more o
> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
> should be considered a bug.
Fine, then let's apply Martin's patch, on mainline at least.
> And again, this is not only a problem of structures with zero-sized
> arrays at the end. Remember my previous example code:
> O
Hi,
On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>
>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
>> should be considered a bug.
>
> Fine, then let's apply Martin's patch, on mainline at least.
>
That would definitely be a good move. Maybe someone should approv
> That would definitely be a good move. Maybe someone should approve it?
Yes, but I agree that we ought to restrict it to trailing zero-sized arrays.
That would help to allay Jakub's concerns about possible ABI fallouts.
--
Eric Botcazou
Hi,
>> That would definitely be a good move. Maybe someone should approve it?
>
> Yes, but I agree that we ought to restrict it to trailing zero-sized arrays.
> That would help to allay Jakub's concerns about possible ABI fallouts.
>
> --
> Eric Botcazou
Other uses of zero-sized arrays in structu
> Would you agree that this "error: flexible array member"
> should also be emitted for a zero-sized array member,
> maybe as "error: zero-sized array member not at end of struct"?
I would have answered yes when zero-sized arrays where introduced, but it's
far less clear a couple of decades later
Hi Eric,
>> Would you agree that this "error: flexible array member"
>> should also be emitted for a zero-sized array member,
>> maybe as "error: zero-sized array member not at end of struct"?
>
> I would have answered yes when zero-sized arrays where introduced, but it's
> far less clear a couple
> But if zero-sized arrays everywhere in a structure is valid C,
> then the attached test case is a valid test case.
Not necessarily, you can write the declaration but you cannot index the array,
i.e. this is undefined behavior. And there is nothing new, distinct fields
have been disambiguated
Hi,
> On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>>
>>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
>>> should be considered a bug.
>>
>> Fine, then let's apply Martin's patch, on mainline at least.
>>
>
> That would definitely be a good move. Maybe someone sh
On Tue, Oct 22, 2013 at 12:25 PM, Bernd Edlinger
wrote:
> Hi,
>
>> On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>>>
I agree, that assigning a non-BLKmode to structures with zero-sized arrays
should be considered a bug.
>>>
>>> Fine, then let's apply Martin's patch, on mainline at le
Hi,
On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>
...
> ? And why should the same issue not exist for
>
> struct { V a[1]; }
>
indeed, it does and it's simple to trigger (on x86_64):
/* { dg-do run } */
#include
typedef long lo
> That looks backwards. Why should
>
> struct { V i; V j[0]; }
>
> have a different mode than
>
> struct { V j[0]; V i; }
>
> ?
Because we support out-of-bounds access for the array in the former case and
not in the latter case.
> And why should the same issue not exist for
>
> struct { V
> While I was willing to discount the zero sized array as an
> insufficiently specified oddity, this seems to be much more serious
> and potentially common. It seems we really need to be able to detect
> these out-of-bounds situations and tell lower levels of expander that
> "whatever mode you thi
Hi Martin,
On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>
>
> ...
>
>> ? And why should the same issue not exist for
>>
>> struct { V a[1]; }
>>
>
> indeed, it does and it's simple to trigger (on x86_64):
>
>
> I think that is common practice to write array[1]; at the end of the
> structure, and use it as a flexible array. The compute_record_mode has no
> way to decide if that is going to be a flexible array or not.
>
> Sorry Eric, but now I have no other choice than to go back to my previous
> version
>> I think that is common practice to write array[1]; at the end of the
>> structure, and use it as a flexible array. The compute_record_mode has no
>> way to decide if that is going to be a flexible array or not.
>>
>> Sorry Eric, but now I have no other choice than to go back to my previous
>> ve
> Because of the ABI-change?
While concerns about accidental ABI changes are real, they shouldn't be
overstated either. We have been saying for longer than a decade that the
back-ends must not use the type mode to implement calling conventions and
other external interfaces.
--
Eric Botcazou
On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou wrote:
>> I think that is common practice to write array[1]; at the end of the
>> structure, and use it as a flexible array. The compute_record_mode has no
>> way to decide if that is going to be a flexible array or not.
>>
>> Sorry Eric, but now I h
On Thu, Oct 24, 2013 at 9:15 AM, Bernd Edlinger
wrote:
> Hi Martin,
>
> On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
>> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>>
>>
>> ...
>>
>>> ? And why should the same issue not exist for
>>>
>>> struct { V a[1]; }
>>>
>>
>> inde
On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>
> On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou wrote:
>>> I think that is common practice to write array[1]; at the end of the
>>> structure, and use it as a flexible array. The compute_record_mode has no
>>> way to decide if that is going t
On Thu, Oct 24, 2013 at 12:22 PM, Richard Biener
wrote:
> On Thu, Oct 24, 2013 at 9:15 AM, Bernd Edlinger
> wrote:
>> Hi Martin,
>>
>> On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
>>> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>>
>>> ...
>>>
? And why should
On Thu, Oct 24, 2013 at 2:55 PM, Bernd Edlinger
wrote:
> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>>
>> On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
>> wrote:
I think that is common practice to write array[1]; at the end of the
structure, and use it as a flexible array. T
On Thu, Oct 24, 2013 at 02:55:59PM +0200, Bernd Edlinger wrote:
> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
> >
> > On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
> > wrote:
> >>> I think that is common practice to write array[1]; at the end of the
> >>> structure, and use it as a flex
>> Did you just propose:
>>
>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>> @@ -471,27 +471,7 @@
>> static enum machine_mode
>> mode_for_array (tree elem_type, tree size)
>> {
>> - tree elem_size;
>> - unsigned HOST_WIDE_INT
On Thu, Oct 24, 2013 at 3:13 PM, Jakub Jelinek wrote:
> On Thu, Oct 24, 2013 at 02:55:59PM +0200, Bernd Edlinger wrote:
>> On Thu, 24 Oct 2013 12:18:43, Richard Biener wrote:
>> >
>> > On Thu, Oct 24, 2013 at 10:48 AM, Eric Botcazou
>> > wrote:
>> >>> I think that is common practice to write arr
Hi Richard,
>> Did you just propose:
>>
>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>> @@ -471,27 +471,7 @@
>> static enum machine_mode
>> mode_for_array (tree elem_type, tree size)
>> {
>> - tree elem_size;
>> - unsigned
On Fri, Oct 25, 2013 at 8:29 AM, Bernd Edlinger
wrote:
> Hi Richard,
>
>
>>> Did you just propose:
>>>
>>> --- stor-layout.c.orig 2013-10-22 10:46:49.233261818 +0200
>>> +++ stor-layout.c 2013-10-24 14:54:00.425259062 +0200
>>> @@ -471,27 +471,7 @@
>>> static enum machine_mode
>>> mode_for_array (
Hi,
> Eh ... even
>
> register struct { int i; int a[0]; } asm ("ebx");
>
> works. Also with int a[1] but not with a[2]. So just handling trailing
> arrays makes this case regress as well.
>
> Now I'd call this use questionable ... but we've likely supported that
> for decades and cannot change th
On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
wrote:
> Hi,
>
>> Eh ... even
>>
>> register struct { int i; int a[0]; } asm ("ebx");
>>
>> works. Also with int a[1] but not with a[2]. So just handling trailing
>> arrays makes this case regress as well.
>>
>> Now I'd call this use questionable ..
Hi,
On Fri, Oct 25, 2013 at 12:51:13PM +0200, Richard Biener wrote:
> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
> wrote:
> > Hi,
> >
> >> Eh ... even
> >>
> >> register struct { int i; int a[0]; } asm ("ebx");
> >>
> >> works. Also with int a[1] but not with a[2]. So just handling trailing
Hi,
On Fri, 25 Oct 2013 12:51:13, Richard Biener wrote:
>
> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
> wrote:
>> Hi,
>>
>>> Eh ... even
>>>
>>> register struct { int i; int a[0]; } asm ("ebx");
>>>
>>> works. Also with int a[1] but not with a[2]. So just handling trailing
>>> arrays makes
oops - this time with attachments...
> Hi,
>
> On Fri, 25 Oct 2013 12:51:13, Richard Biener wrote:
>>
>> On Fri, Oct 25, 2013 at 12:02 PM, Bernd Edlinger
>> wrote:
>>> Hi,
>>>
Eh ... even
register struct { int i; int a[0]; } asm ("ebx");
works. Also with int a[1] but not
Hi,
ping...
this patch still open: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html
Note: it does, as it is, _not_ depend on the keep_aligning patch.
And it would fix some really nasty wrong code generation issues.
Thanks
Bernd.
> Date: Tue, 19 Nov 2013 15:39:39 +0100
>
> Hello,
>
>
>
On Wed, Nov 27, 2013 at 1:29 PM, Bernd Edlinger
wrote:
> Hi,
>
> ping...
>
> this patch still open: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html
>
> Note: it does, as it is, _not_ depend on the keep_aligning patch.
> And it would fix some really nasty wrong code generation issues.
I'll
On 11/27/13 05:29, Bernd Edlinger wrote:
Hi,
ping...
this patch still open: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html
Note: it does, as it is, _not_ depend on the keep_aligning patch.
And it would fix some really nasty wrong code generation issues.
Is there a testcase for this p
Hi,
On Wed, 27 Nov 2013 12:07:16, Jeff Law wrote:
>
> On 11/27/13 05:29, Bernd Edlinger wrote:
>> Hi,
>>
>> ping...
>>
>> this patch still open:
>> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html
>>
>> Note: it does, as it is, _not_ depend on the keep_aligning patch.
>> And it would fix s
On 11/28/13 03:24, Bernd Edlinger wrote:
Hi,
On Wed, 27 Nov 2013 12:07:16, Jeff Law wrote:
On 11/27/13 05:29, Bernd Edlinger wrote:
Hi,
ping...
this patch still open: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html
Note: it does, as it is, _not_ depend on the keep_aligning patch.
A
51 matches
Mail list logo
