Date: Sun, 21 Jan 2018 13:55:55 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: Mark Millard <marklmi26-fbsd@yahoo.com> Cc: FreeBSD Toolchain <freebsd-toolchain@freebsd.org>, FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: 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 Message-ID: <9f07664c-6b39-8201-94ed-62c6b91cffbf@FreeBSD.org> In-Reply-To: <67a7b385-b554-1a5e-ef30-fb29a0c6cf08@FreeBSD.org> References: <1AA0993D-81E4-4DC0-BBD9-CC42B26ADB1C@yahoo.com> <F227842D-6BE2-4680-82E7-07906AF61CD7@yahoo.com> <8c4c5985-885a-abf7-93df-0698c645d36e@FreeBSD.org> <9DE674C6-EAA3-4E8A-906F-446E74D82FC4@yahoo.com> <67a7b385-b554-1a5e-ef30-fb29a0c6cf08@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
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@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@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.
[-- Attachment #2 --]
Index: include/stdlib.h
===================================================================
--- include/stdlib.h (revision 328218)
+++ include/stdlib.h (working copy)
@@ -92,7 +92,7 @@
void *bsearch(const void *, const void *, size_t,
size_t, int (*)(const void * _Nonnull, const void *));
void *calloc(size_t, size_t) __malloc_like __result_use_check
- __alloc_size(1) __alloc_size(2);
+ __alloc_size2(1, 2);
div_t div(int, int) __pure2;
_Noreturn void exit(int);
void free(void *);
@@ -302,8 +302,8 @@
int (*)(void *, const void *, const void *));
int radixsort(const unsigned char **, int, const unsigned char *,
unsigned);
-void *reallocarray(void *, size_t, size_t) __result_use_check __alloc_size(2)
- __alloc_size(3);
+void *reallocarray(void *, size_t, size_t) __result_use_check
+ __alloc_size2(2, 3);
void *reallocf(void *, size_t) __result_use_check __alloc_size(2);
int rpmatch(const char *);
void setprogname(const char *);
Index: sys/sys/cdefs.h
===================================================================
--- sys/sys/cdefs.h (revision 328218)
+++ sys/sys/cdefs.h (working copy)
@@ -230,8 +230,10 @@
#endif
#if __GNUC_PREREQ__(4, 3) || __has_attribute(__alloc_size__)
#define __alloc_size(x) __attribute__((__alloc_size__(x)))
+#define __alloc_size2(n, x) __attribute__((__alloc_size__(n, x)))
#else
#define __alloc_size(x)
+#define __alloc_size2(n, x)
#endif
#if __GNUC_PREREQ__(4, 9) || __has_attribute(__alloc_align__)
#define __alloc_align(x) __attribute__((__alloc_align__(x)))
Index: sys/sys/malloc.h
===================================================================
--- sys/sys/malloc.h (revision 328218)
+++ sys/sys/malloc.h (working copy)
@@ -188,7 +188,7 @@
__malloc_like __result_use_check __alloc_size(1);
void *mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
int flags) __malloc_like __result_use_check
- __alloc_size(1) __alloc_size(2);
+ __alloc_size2(1, 2);
void malloc_init(void *);
int malloc_last_fail(void);
void malloc_type_allocated(struct malloc_type *type, unsigned long size);
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9f07664c-6b39-8201-94ed-62c6b91cffbf>