From owner-freebsd-hackers@freebsd.org Sun Jan 21 18:55:59 2018 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 48DF3EB44CE for ; Sun, 21 Jan 2018 18:55:59 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from sonic316-22.consmr.mail.gq1.yahoo.com (sonic316-22.consmr.mail.gq1.yahoo.com [98.137.69.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1F5EB773AA for ; Sun, 21 Jan 2018 18:55:58 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1516560957; bh=Oeb2XetJ+ct/IKmS4UKj4/rbGZKTJcjTmBbduFAdhd4=; h=Subject:From:To:Cc:References:Date:In-Reply-To:From:Subject; b=X4rV2GZuvdbafkp1woDqmvFOcs7Rhs3IgkSnEv9BXZr2GpseT/vUbg0fskw28StXxgirU2ldr9YBKuwBDRWgVrfyfiqbuqj1HKXulNz5vYjCIKFdSsLnXcyrdkg6BpKnedEwtaJUHojMnMlaZWj7PQzeNpICW1AJ8dXlSyOUsn/JIAJpT5ayX6bGKmvt7Zraabzi4EG+IeG60MjCMvhOyS7vgGHz7C54TFRBvBqrXpVwhVby9tZhvc+NNI4dmWvjjPS1C7wd0/uO3cROM9pv4QjiG0od0iQbGUYeBn4lngcHIoTo7uxm+iEPfUbV4gph31LvmUvpgU73MM9kvayXLw== X-YMail-OSG: ZzOTs3UVM1neXQb.Mfn_IpOr6MxQ2RDWt8rFG8EUhn5Q_hHUyZU66aGWihNNrfa 7eTPK36cVpEm0n3YAzaiWHdfyQB6boBPY.LZpsTNlMu75cprXmuWEudmFyYFkBMruHjDunXwU0aY BgXucOBmDzOvJqKfKJHahu8aMcBcwdtIIW3Ax2Lhggl14nw46jsxPgZyVeD6RJ.SOWxDenHwIHWG eoXVFNHr91hqs6xjyrpWXfdavFnXLNarsDCT_rFnwG_M7.5Sb1KFdvJ61WcBomyVHJypYwuK9.Jm tMkf4yq8ZQzF5ZUemFXI5SRiw8Y5MQX0EFZPuz8mzaAa1BLXfIFKbza5WTmoJLzuc7A1CR41.c7G jXe.dZv1x1z9_O.92fiWBdQGHT2B4ploBK5cSoAQIpl4ErMij9T56cKfbOgjkVYciTvItOy2vQaq r8zCuGvNVhQcYsCdxKlmuRkmwIO2T.CmUQA8RRkmFUT3tXzhmE2TSDja4.qQOqnt_5CyqqNmg.u6 OzKo6X1pg2Q-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic316.consmr.mail.gq1.yahoo.com with HTTP; Sun, 21 Jan 2018 18:55:57 +0000 Received: from smtp105.rhel.mail.gq1.yahoo.com (EHLO [192.168.0.8]) ([68.180.227.8]) by smtp412.mail.gq1.yahoo.com (JAMES SMTP Server ) with ESMTPA ID 521ad2623a0d6f915e996ea87fc2db5c; Sun, 21 Jan 2018 18:55:54 +0000 (UTC) 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 From: Pedro Giffuni To: Mark Millard Cc: FreeBSD Toolchain , FreeBSD Hackers References: <1AA0993D-81E4-4DC0-BBD9-CC42B26ADB1C@yahoo.com> <8c4c5985-885a-abf7-93df-0698c645d36e@FreeBSD.org> <9DE674C6-EAA3-4E8A-906F-446E74D82FC4@yahoo.com> <67a7b385-b554-1a5e-ef30-fb29a0c6cf08@FreeBSD.org> Message-ID: <9f07664c-6b39-8201-94ed-62c6b91cffbf@FreeBSD.org> Date: Sun, 21 Jan 2018 13:55:55 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <67a7b385-b554-1a5e-ef30-fb29a0c6cf08@FreeBSD.org> Content-Type: multipart/mixed; boundary="------------520FA10BA9E7194FE65018A2" Content-Language: en-US X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 Jan 2018 18:55:59 -0000 This is a multi-part message in MIME format. --------------520FA10BA9E7194FE65018A2 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit 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 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 >>>> 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 >>>>> #include >>>>> >>>>> __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. --------------520FA10BA9E7194FE65018A2 Content-Type: text/x-patch; name="alloc_size2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="alloc_size2.diff" 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); --------------520FA10BA9E7194FE65018A2--