> On Nov 20, 2024, at 14:23, Martin Uecker <uec...@tugraz.at> wrote:
>
> Am Mittwoch, dem 20.11.2024 um 17:37 +0000 schrieb Qing Zhao:
>> 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?
>>
>
> There is no specification yet and nothing formally proposed, so
> it is entirely unclear at this point.
>
> My idea would be to give 'x->buf' the type '(char(*)[x->n])'
> where 'x->n' is loaded at the same time 'x->buf' is accessed
> and then the value is frozen (like in the simpler versions
> of 'counted_by' you had implemented first). Of course, one
> would then have to set x->n before setting the buffer (or
> at the same time). This could be ensured by making the
> member 'n' const, so that it can only be changed by
> overwriting the whole struct. But I am still thinking
> about this.
Okay, so the key here is:
x->n, x->p can only be changed by changing the whole structure at the same
time.
Otherwise, x->n might not be consistent with x->p.
We might need to add such limitation for the counted-by attribute of pointer
field in the documentation.
>
> In any case, I think for "counted_by" this is not an option
> because it would be confusing if it works differently
> than for the FAM case.
But we need to clearly let the user know the limitation of using counted-by
attribute for pointer field.
Qing
>
> Martin
>
>
>> 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
>>>>>
>>>>
>>>
>>
>
