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 <p...@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-f...@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 -apKUFreeBSD FBSDFSSD 12.0-CURRENT FreeBSD 12.0-CURRENT r327485M amd64 amd64 1200054 1200054The system-clang 5.0.1 result was: # clang -O2 alloc_size_attr_test.cThe 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, 0my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 14, 14, 0The 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, 0my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 7, 14, 14my_calloc_alt1 __builtin_object_size 0,1,2,3: 14, 14, 14, 0 (system clang)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, 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.
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);
_______________________________________________ freebsd-toolchain@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-toolchain To unsubscribe, send any mail to "freebsd-toolchain-unsubscr...@freebsd.org"
