Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-17 Thread Kees Cook via Gcc-patches
On Thu, Aug 17, 2023 at 01:44:42PM +, Qing Zhao wrote: > Thanks for the testing case. > Yes, I noticed this issue too, and already fixed it in my private branch. > > With the latest patch, the compilation has no issue: > [opc@qinzhao-ol8u3-x86 108896]$ sh t >

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-17 Thread Qing Zhao via Gcc-patches
Hi, Kees, Thanks for the testing case. Yes, I noticed this issue too, and already fixed it in my private branch. With the latest patch, the compilation has no issue: [opc@qinzhao-ol8u3-x86 108896]$ sh t /home/opc/Install/latest-d/bin/gcc -O2 -c -o /dev/null bug.c [opc@qinzhao-ol8u3-x86

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-17 Thread Kees Cook via Gcc-patches
On Wed, Aug 16, 2023 at 10:31:30PM -0700, Kees Cook wrote: > On Fri, Aug 04, 2023 at 07:44:28PM +, Qing Zhao wrote: > > This is the 2nd version of the patch, per our discussion based on the > > review comments for the 1st version, the major changes in this version > > I've been using

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-16 Thread Kees Cook via Gcc-patches
On Fri, Aug 04, 2023 at 07:44:28PM +, Qing Zhao wrote: > This is the 2nd version of the patch, per our discussion based on the > review comments for the 1st version, the major changes in this version I've been using Coccinelle to find and annotate[1] structures (193 so far...), and I've

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-16 Thread Qing Zhao via Gcc-patches
Hi, After some more studying and consideration, the following is my thoughts: For a structure with FMA annotated with counted_by attribute: (the following small example) struct annotated { size_t foo; char b; char array[] __attribute__((counted_by (foo))); };

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Qing Zhao via Gcc-patches
> On Aug 10, 2023, at 12:39 PM, Jakub Jelinek wrote: > > On Thu, Aug 10, 2023 at 12:30:06PM -0400, Siddhesh Poyarekar wrote: >> The definition of __bos/__bdos allows us the freedom to *estimate* rather >> than be precise, so I'd go for sizeof(x) + N * sizeof(*x.a) since it's bound >> to give

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Siddhesh Poyarekar
On 2023-08-10 12:39, Jakub Jelinek wrote: On Thu, Aug 10, 2023 at 12:30:06PM -0400, Siddhesh Poyarekar wrote: The definition of __bos/__bdos allows us the freedom to *estimate* rather than be precise, so I'd go for sizeof(x) + N * sizeof(*x.a) since it's bound to give the more conservative

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Jakub Jelinek via Gcc-patches
On Thu, Aug 10, 2023 at 12:30:06PM -0400, Siddhesh Poyarekar wrote: > The definition of __bos/__bdos allows us the freedom to *estimate* rather > than be precise, so I'd go for sizeof(x) + N * sizeof(*x.a) since it's bound > to give the more conservative answer of the two. To be precise, we have

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Siddhesh Poyarekar
On 2023-08-10 11:18, Martin Uecker wrote: Am Donnerstag, dem 10.08.2023 um 10:58 -0400 schrieb Siddhesh Poyarekar: On 2023-08-10 10:47, Martin Uecker wrote: Am Donnerstag, dem 10.08.2023 um 16:42 +0200 schrieb Jakub Jelinek: On Thu, Aug 10, 2023 at 04:38:21PM +0200, Martin Uecker wrote: Am

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Qing Zhao via Gcc-patches
Hi, Martin, > On Aug 10, 2023, at 11:18 AM, Martin Uecker wrote: > > Am Donnerstag, dem 10.08.2023 um 10:58 -0400 schrieb Siddhesh Poyarekar: >> On 2023-08-10 10:47, Martin Uecker wrote: >>> Am Donnerstag, dem 10.08.2023 um 16:42 +0200 schrieb Jakub Jelinek: On Thu, Aug 10, 2023 at

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Martin Uecker
Am Donnerstag, dem 10.08.2023 um 10:58 -0400 schrieb Siddhesh Poyarekar: > On 2023-08-10 10:47, Martin Uecker wrote: > > Am Donnerstag, dem 10.08.2023 um 16:42 +0200 schrieb Jakub Jelinek: > > > On Thu, Aug 10, 2023 at 04:38:21PM +0200, Martin Uecker wrote: > > > > Am Donnerstag, dem 10.08.2023 um

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Siddhesh Poyarekar
On 2023-08-10 10:47, Martin Uecker wrote: Am Donnerstag, dem 10.08.2023 um 16:42 +0200 schrieb Jakub Jelinek: On Thu, Aug 10, 2023 at 04:38:21PM +0200, Martin Uecker wrote: Am Donnerstag, dem 10.08.2023 um 13:59 + schrieb Qing Zhao: On Aug 10, 2023, at 2:58 AM, Martin Uecker wrote: Am

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Martin Uecker
Am Donnerstag, dem 10.08.2023 um 16:42 +0200 schrieb Jakub Jelinek: > On Thu, Aug 10, 2023 at 04:38:21PM +0200, Martin Uecker wrote: > > Am Donnerstag, dem 10.08.2023 um 13:59 + schrieb Qing Zhao: > > > > > > > On Aug 10, 2023, at 2:58 AM, Martin Uecker wrote: > > > > > > > > Am Mittwoch,

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Jakub Jelinek via Gcc-patches
On Thu, Aug 10, 2023 at 04:38:21PM +0200, Martin Uecker wrote: > Am Donnerstag, dem 10.08.2023 um 13:59 + schrieb Qing Zhao: > > > > > On Aug 10, 2023, at 2:58 AM, Martin Uecker wrote: > > > > > > Am Mittwoch, dem 09.08.2023 um 20:10 + schrieb Qing Zhao: > > > > > > > > > On Aug 9,

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Martin Uecker
Am Donnerstag, dem 10.08.2023 um 13:59 + schrieb Qing Zhao: > > > On Aug 10, 2023, at 2:58 AM, Martin Uecker wrote: > > > > Am Mittwoch, dem 09.08.2023 um 20:10 + schrieb Qing Zhao: > > > > > > > On Aug 9, 2023, at 12:21 PM, Michael Matz wrote: > > > > I am not sure for the reason

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Michael Matz via Gcc-patches
Hey, On Thu, 10 Aug 2023, Martin Uecker wrote: > > offset(struct foo_flex, t[0]) + N * sizeof(foo->t); > > > > With GCC, offset(struct foo_flex,t[0]) == 6, which is also correct. > > This formula might be considered incorrect / dangerous because > it might allocate less storage than

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Qing Zhao via Gcc-patches
> On Aug 10, 2023, at 2:58 AM, Martin Uecker wrote: > > Am Mittwoch, dem 09.08.2023 um 20:10 + schrieb Qing Zhao: >> >>> On Aug 9, 2023, at 12:21 PM, Michael Matz wrote: > > ... >> >> By definition, the sizeof() of a struct with FAM might not be the same as >> the non-FAM one. >>

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Michael Matz via Gcc-patches
Hello, On Wed, 9 Aug 2023, Qing Zhao wrote: > > So, should the equivalent FAM struct also have this sizeof()? If no: > > there should be a good argument why it shouldn't be similar to the non-FAM > > one. > > The sizeof() of a structure with FAM is defined as: (after I searched online, > I

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-10 Thread Martin Uecker
Am Mittwoch, dem 09.08.2023 um 20:10 + schrieb Qing Zhao: > > > On Aug 9, 2023, at 12:21 PM, Michael Matz wrote: ... > > By definition, the sizeof() of a struct with FAM might not be the same as the > non-FAM one. > i.e, for the following two structures, one with FAM, the other with

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-09 Thread Qing Zhao via Gcc-patches
> On Aug 8, 2023, at 10:54 AM, Martin Uecker wrote: > > > > I am sure this has been discussed before, but seeing that you > test for a specific formula, let me point out the following: > > There at least three different size expression which could > make sense. Consider > > short foo { int

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-09 Thread Qing Zhao via Gcc-patches
> On Aug 9, 2023, at 12:21 PM, Michael Matz wrote: > > Hello, > > On Wed, 9 Aug 2023, Qing Zhao wrote: > >> Although this is an old FAM related issue that does not relate to my current >> patch >> (and might need to be resolved in a separate patch). I think that it’s >> necessary to have

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-09 Thread Kees Cook via Gcc-patches
On Mon, Aug 07, 2023 at 04:33:13PM +, Qing Zhao wrote: > What’s the testing case for the one that failed? > If it’s > > __builtin_dynamic_object_size(p->array, 0/2) without the allocation > information in the routine, > then with the current algorithm, gcc cannot deduce the size for the

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-09 Thread Michael Matz via Gcc-patches
Hello, On Wed, 9 Aug 2023, Qing Zhao wrote: > Although this is an old FAM related issue that does not relate to my current > patch > (and might need to be resolved in a separate patch). I think that it’s > necessary to have > more discussion on this old issue and resolve it. > > The first

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-09 Thread Qing Zhao via Gcc-patches
Hi, Martin, Thanks for raising this issue. Although this is an old FAM related issue that does not relate to my current patch (and might need to be resolved in a separate patch). I think that it’s necessary to have more discussion on this old issue and resolve it. The first thing that I’d

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-08 Thread Kees Cook via Gcc-patches
On Tue, Aug 08, 2023 at 04:54:46PM +0200, Martin Uecker wrote: > > > I am sure this has been discussed before, but seeing that you > test for a specific formula, let me point out the following: > > There at least three different size expression which could > make sense. Consider > > short foo

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-08 Thread Michael Matz via Gcc-patches
Hello, On Tue, 8 Aug 2023, Martin Uecker via Gcc-patches wrote: > There at least three different size expression which could > make sense. Consider > > short foo { int a; short b; char t[]; };  > > Most people seem to use > > sizeof(struct foo) + N * sizeof(foo->t); > > which for N == 3

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-08 Thread Martin Uecker via Gcc-patches
I am sure this has been discussed before, but seeing that you test for a specific formula, let me point out the following: There at least three different size expression which could make sense. Consider short foo { int a; short b; char t[]; };  Most people seem to use sizeof(struct foo) + N

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-07 Thread Qing Zhao via Gcc-patches
> On Aug 7, 2023, at 12:16 PM, Kees Cook wrote: > > On Fri, Aug 04, 2023 at 07:44:28PM +, Qing Zhao wrote: >> This is the 2nd version of the patch, per our discussion based on the >> review comments for the 1st version, the major changes in this version >> are: > > Thanks for the update!

Re: [V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-07 Thread Kees Cook via Gcc-patches
On Fri, Aug 04, 2023 at 07:44:28PM +, Qing Zhao wrote: > This is the 2nd version of the patch, per our discussion based on the > review comments for the 1st version, the major changes in this version > are: Thanks for the update! > > 1. change the name "element_count" to "counted_by"; > 2.

[V2][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896)

2023-08-04 Thread Qing Zhao via Gcc-patches
Hi, This is the 2nd version of the patch, per our discussion based on the review comments for the 1st version, the major changes in this version are: 1. change the name "element_count" to "counted_by"; 2. change the parameter for the attribute from a STRING to an Identifier; 3. Add logic and