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 -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.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, 14I'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. Pedro.
Thus. . . For __attribute__((alloc_size(1))) __attribute__((alloc_size(2))): __builtin_object_size(p,1) is not equivalent (clang vs. gcc7) For both of the alloc_size usage styles: __builtin_object_size(p,3) is not equivalent (clang vs. gcc7) This means that the two style of alloc_size use are not equivalent across some major compilers/toolchains.This is actually not a surprise: GCC and clang implementation of __alloc_size__ has differences due to limitations on the LLVM IR (or the fact there is one). The alloc_size attribute is basically only used for the so-called FORTIFY_SOURCE feature that depends on GCC with some support from the C-library: last time I looked clang didn't support the compile-time checks very well. The attributes are mostly unused in FreeBSD at this time but, GCC7 -Walloc-size-larger-than=size depends on them (I have never tested that though). FWIW, we had an unfinished GSoC that attempted to implement FORTIFY_SOURCE but we got stuck on the lack of clang support and other issues. Lately Google has been spending some effort on it but it is more limited and doesn't match the GCC behavior.But I do not know if either of the differences is a problem or not. Note: without a sufficient -O<?> all the figures can be the mix of -1's and 0's.=== Mark Millard marklmi at yahoo.com ( markmi at dsl-only.net is going away in 2018-Feb, late)
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_size(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"
