Hi, Martin,
Thanks a lot for pointing this out.
This does look like a problem we need avoid for the pointer arrays.
Does the same problem exist in the language extension too if the n is allowed
to be changed after initialization?
If so, for the future language extension, is there any proposed solution to
this problem?
Qing
> On Nov 20, 2024, at 10:52, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Mittwoch, dem 20.11.2024 um 15:27 +0000 schrieb Qing Zhao:
>>
>>> On Nov 19, 2024, at 10:47, Marek Polacek <pola...@redhat.com> wrote:
>>>
>>> On Mon, Nov 18, 2024 at 07:10:35PM +0100, Martin Uecker wrote:
>>>> Am Montag, dem 18.11.2024 um 17:55 +0000 schrieb Qing Zhao:
>>>>> Hi,
>>
> ..
>
> Hi Qing,
>
>> Per our discussion so far, if treating the following
>>
>> struct foo {
>> int n;
>> char *p __attribute__ ((counted_by (n)));
>> };
>>
>> as an array with upper-bounds being “n” is reasonable.
>
> There is one issue I want to point out, which I just realized during
> a discussion in WG14. For "counted_by" we defined the semantics
> in a way that later store to 'n' will be taken into account.
> We did this to support the use case where 'n' is set after
> the access to 'p'.
>
> struct foo *x = ;
>
> char *q = x->p;
> x->n = 100; // this should apply
>
>
> For FAMs this is fine, because it is a fixed part
> of the struct. But for the new pointer case, I think this is
> problematic. Consider this example:
>
> struct foo *x = allocate_buffer(100);
>
> where x->n is set to the right value in the allocation function.
>
> Now let's continue with
>
> char *q = x->p;
>
> x = allocate_buffer(50);
> // x->n is set to 50.
>
> Now q refers to the old buffer, but x->n to the size of the new
> buffer. That does not seem right and scares me a little bit.
>
>
>
> Martin
>
>
>>
>> Then, it’s reasonable to extend -fsanitize=bounds to instrument the
>> corresponding reference for the pointer with
>> Counted-by attribute.
>>
>> What do you think?
>>
>> Qing
>>
>>>
>>>> I think the question is what -fsanitize=bounds is meant to be.
>>>>
>>>> I am a bit frustrated about the sanitizer. On the
>>>> one hand, it is not doing enough to get spatial memory
>>>> safety even where this would be easily possible, on the
>>>> other hand, is pedantic about things which are technically
>>>> UB but not problematic and then one is prevented from
>>>> using it
>>>>
>>>> When used in default mode, where execution continues, it
>>>> also does not mix well with many warning, creates more code,
>>>> and pulls in a libary dependency (and the library also depends
>>>> on upstream choices / progress which seems a limitation for
>>>> extensions).
>>>>
>>>> What IMHO would be ideal is a protection mode for spatial
>>>> memory safety that simply adds traps (which then requires
>>>> no library, has no issues with other warnings, and could
>>>> evolve independently from clang)
>>>>
>>>> So shouldn't we just add a -fboundscheck (which would
>>>> be like -fsanitize=bounds -fsanitize-trap=bounds just with
>>>> more checking) and make it really good? I think many people
>>>> would be very happy about this.
>>>
>>> That's a separate concern. We already have the -fbounds-check option,
>>> currently only used in Fortran (and D?), so perhaps we could make
>>> that option a shorthand for -fsanitize=bounds -fsanitize-trap=bounds.
>>>
>>> Marek
>>>
>>
>
