Attribute alloc__size use and clang 5.0.1 vs. gcc7 (e.g.): __builtin_object_size(p,1) and __builtin_object_size(p,3) disagreements result

[Pedro Giffuni]

On 01/21/18 12:59, Pedro Giffuni wrote:
> Hi;
>
>
> On 01/21/18 11:56, Mark Millard wrote:
>> [May be an __alloc_size2(n,s) should be added and used?]
>>
>> On 2018-Jan-20, at 5:05 PM, Pedro Giffuni <pfg at FreeBSD.org> wrote:
>>
>>> Very interesting , thanks for running such tests ...
>>>
>>>
>>> On 01/20/18 18:59, Mark Millard wrote:
>>>> [Noting a typo in the program source, and
>>>> so in the output text: the 2nd occurance of: "my_calloc_alt0
>>>> should have been: "my_calloc_alt1
>>>> . Hand edited corrections below for clarity.]
>>>>
>>>> On 2018-Jan-20, at 3:27 PM, Mark Millard <marklmi26-fbsd at yahoo.com> 
>>>> wrote:
>>>>
>>>>> [Bugzilla 225197 indirectly lead to this.
>>>>> Avoiding continuing there.]
>>>>>
>>>>> I decided to compare some alternate uses of
>>>>> __attribute__((alloc_size(. . .))) compiled
>>>>> and run under clang 5.0.1 and gcc7. I did not
>>>>> get what I expected based on prior discussion
>>>>> material.
>>>>>
>>>>> This is an FYI since I do not know how important
>>>>> the distinctions that I found are.
>>>>>
>>>>> Here is the quick program:
>>>>>
>>>>> # more alloc_size_attr_test.c
>>>>> #include <stdlib.h>
>>>>> #include <stdio.h>
>>>>>
>>>>> __attribute__((alloc_size(1,2)))
>>>>> void* my_calloc_alt0(size_t n, size_t s)
>>>>> {
>>>>>    void* p = calloc(n,s);
>>>>>    printf("calloc __builtin_object_size 0,1,2,3: %ld, %ld, %ld, 
>>>>> %ld\n"
>>>>>          ,(long) __builtin_object_size(p, 0)
>>>>>          ,(long) __builtin_object_size(p, 1)
>>>>>          ,(long) __builtin_object_size(p, 2)
>>>>>          ,(long) __builtin_object_size(p, 3)
>>>>>          );
>>>>>    return p;
>>>>> }
>>>>>
>>>>> __attribute__((alloc_size(1))) __attribute__((alloc_size(2)))
>>>>> void* my_calloc_alt1(size_t n, size_t s)
>>>>> {
>>>>>    void* p = calloc(n,s);
>>>>>    printf("calloc __builtin_object_size 0,1,2,3: %ld, %ld, %ld, 
>>>>> %ld\n"
>>>>>          ,(long) __builtin_object_size(p, 0)
>>>>>          ,(long) __builtin_object_size(p, 1)
>>>>>          ,(long) __builtin_object_size(p, 2)
>>>>>          ,(long) __builtin_object_size(p, 3)
>>>>>          );
>>>>>    return p;
>>>>> }
>>>>>
>>>>> int main()
>>>>> {
>>>>>    void* p = my_calloc_alt0(2,7);
>>>>>    printf("my_calloc_alt0 __builtin_object_size 0,1,2,3: %ld, %ld, 
>>>>> %ld, %ld\n"
>>>>>          ,(long) __builtin_object_size(p, 0)
>>>>>          ,(long) __builtin_object_size(p, 1)
>>>>>          ,(long) __builtin_object_size(p, 2)
>>>>>          ,(long) __builtin_object_size(p, 3)
>>>>>          );
>>>>>    void* q = my_calloc_alt1(2,7);
>>>>>    printf("my_calloc_alt0 __builtin_object_size 0,1,2,3: %ld, %ld, 
>>>>> %ld, %ld\n"
>>>> The above line should have been:
>>>>
>>>> printf("my_calloc_alt1 __builtin_object_size 0,1,2,3: %ld, %ld, 
>>>> %ld, %ld\n"
>>>>
>>>>>          ,(long) __builtin_object_size(q, 0)
>>>>>          ,(long) __builtin_object_size(q, 1)
>>>>>          ,(long) __builtin_object_size(q, 2)
>>>>>          ,(long) __builtin_object_size(q, 3)
>>>>>          );
>>>>> }
>>>>>
>>>>> # uname -apKU
>>>>> FreeBSD FBSDFSSD 12.0-CURRENT FreeBSD 12.0-CURRENT r327485M  amd64 
>>>>> amd64 1200054 1200054
>>>>>
>>>>> The system-clang 5.0.1 result was:
>>>>>
>>>>> # clang -O2 alloc_size_attr_test.c
>>>> The later outputs are edited for clarity:
>>>>
>>>>> # ./a.out
>>>>> calloc __builtin_object_size 0,1,2,3: 14, 14, 14, 0
>>>>> my_calloc_alt0 __builtin_object_size 0,1,2,3: 14, 14, 14, 0
>>>>> calloc __builtin_object_size 0,1,2,3: 14, 14, 14, 0
>>>> my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 14, 14, 0
>>>>> The lang/gcc7 result was:
>>>>>
>>>>> # gcc7 -O2 alloc_size_attr_test.c
>>>>>
>>>>> # ./a.out
>>>>> calloc __builtin_object_size 0,1,2,3: -1, -1, 0, 0
>>>>> my_calloc_alt0 __builtin_object_size 0,1,2,3: 14, 14, 14, 14
>>>>> calloc __builtin_object_size 0,1,2,3: -1, -1, 0, 0
>>>> my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 7, 14, 14
>>>>> I'll ignore that gcc does not provide actual sizes
>>>>> via __builtin_object_size for calloc use.
>>>>>
>>>>> Pairing the other lines for easy comparison, with
>>>>> some notes mixed in:
>>>>>
>>>>> __attribute__((alloc_size(1,2))) style:
>>>>> my_calloc_alt0 __builtin_object_size 0,1,2,3: 14, 14, 14, 0  
>>>>> (system clang)
>>>>> my_calloc_alt0 __builtin_object_size 0,1,2,3: 14, 14, 14, 14 (gcc7)
>>>>>
>>>>> __attribute__((alloc_size(1))) __attribute__((alloc_size(2))) style:
>>>> my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 14, 14, 0 (system 
>>>> clang)
>>>> my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 7, 14, 14 (gcc7)
>>> So on GCC7 it appears
>>>   __attribute__((alloc_size(1,2))) != __attribute__((alloc_size(1))) 
>>> __attribute__((alloc_size(2)))
>>>
>>> This is not good as it was the base for r280801 .. related to the 
>>> old discussion about deprecating old compilers that don't accept 
>>> VA_ARGS.
>>>
>>> I am unsure if its a regression but it appears that for clang it is 
>>> the same thing though.
>> May be there should be a __alloc_size2(n,s) that translates to
>> __attribute__((alloc_size(n,s))) when it is not a no-op. This
>> avoids the VA_ARGS issue and gcc's documentation makes no mention
>> of a more than 2 argument variant of
>> __attribute__((alloc_size(. . .))) .
>>
>> Looking up glib it has:
>>
>> G_GNUC_ALLOC_SIZE(x)
>> G_GNUC_ALLOC_SIZE2(x,y)
>>
>> but none for any other count of arguments.
>
>
> Yes, that would work fine for alloc_array. It would work for the 
> nonnull attribute but that is deprecated on FreeBSD :).
>
>
> Concept patch attached.
And of course there was a bug.

Pedro.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alloc_size2.diff
Type: text/x-patch
Size: 2126 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20180121/81813228/attachment.bin>