From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48100 invoked by alias); 11 Dec 2018 20:37:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 48085 invoked by uid 89); 11 Dec 2018 20:37:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.1 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=POS, integral, nonexistent X-HELO: mail-qk1-f196.google.com Received: from mail-qk1-f196.google.com (HELO mail-qk1-f196.google.com) (209.85.222.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Dec 2018 20:37:02 +0000 Received: by mail-qk1-f196.google.com with SMTP id r71so9433638qkr.10 for ; Tue, 11 Dec 2018 12:37:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=Axy8vVXQAM0Md66RgUimLfTZFoBHC8cKneyIDY/A24k=; b=LqnN1UE+9dGoHgNX31w2vs1g94jXBygfzqVZg0IZ6E6Lg7utZT0PfDCa9c4BNhxjzh m3UBgo9MIy2xDz+ZArj8EF/pjZYBWUd0JmNqis7hGq5rbubLNqsDXvlGL3VAY5BNQXwc aB1UIat+SY1HrpW4nbLNbfbCQ89RSO8LPULRMJgpksN8p54zFTTVia1pA08spvVHfm7y cIKqy2lkKusJoK2EHspKLr0kpcbXeNTwVpKW3Ad2Xz2gm/XFHwG5znaqw/gB/OsdMYdz 1Dd9D3o5/373LWnVlfAspGoTUgx9tpkt2G6QzhRr2WE8qHuwRPTcOG+NnieALirNLlOJ 6szA== Return-Path: Received: from localhost.localdomain (97-118-99-160.hlrn.qwest.net. [97.118.99.160]) by smtp.gmail.com with ESMTPSA id c11sm8518507qtn.95.2018.12.11.12.36.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 11 Dec 2018 12:37:00 -0800 (PST) Subject: Re: [PATCH] accept all C integer types in function parameters referenced by alloc_align (PR 88363) To: Jakub Jelinek , "Joseph S. Myers" , Marek Polacek , Jason Merrill , Nathan Sidwell Cc: Gcc Patch List References: <0f3f1395-adac-8b5f-82e4-e656bf1207fb@gmail.com> <20181211071726.GI12380@tucnak> From: Martin Sebor Message-ID: Date: Tue, 11 Dec 2018 20:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20181211071726.GI12380@tucnak> Content-Type: multipart/mixed; boundary="------------45ED00EDEB25C6B43DC0B287" X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00740.txt.bz2 This is a multi-part message in MIME format. --------------45ED00EDEB25C6B43DC0B287 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 968 On 12/11/18 12:17 AM, Jakub Jelinek wrote: > On Mon, Dec 10, 2018 at 04:30:11PM -0700, Martin Sebor wrote: >> Some of my testing exposed a minor problem in GCC 9's validation >> of the type of function parameters referred to by attribute >> positional arguments. Whereas GCC 8 accepts all C integer types, >> including enumerated types, such as: >> >> enum AllocAlign { Align16 = 16, Align32 = 32 }; >> >> __attribute__ ((alloc_align (1))) void* >> alloc (size_t, enum AllocAlign) >> >> GCC 9 only accepts signed and unsigned integer types. This change >> (introduced by myself) was unintentional, and a fix for it is in >> the attached trivial patch. I plan to commit it without approval >> in the next day or so unless any concerns or suggestions come up. Attached is an updated version of the patch that restores the original behavior for the positional argument validation (i.e., prior to r266195) for integral types except bool as discussed. Martin --------------45ED00EDEB25C6B43DC0B287 Content-Type: text/x-patch; name="gcc-88363.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gcc-88363.diff" Content-length: 6090 Index: gcc/c-family/c-attribs.c =================================================================== --- gcc/c-family/c-attribs.c (revision 266966) +++ gcc/c-family/c-attribs.c (working copy) @@ -498,10 +498,11 @@ attribute_takes_identifier_p (const_tree attr_id) /* Verify that argument value POS at position ARGNO to attribute NAME applied to function TYPE refers to a function parameter at position - POS and the expected type CODE. If so, return POS after default - conversions, if any. Otherwise, issue appropriate warnings and - return null. A non-zero 1-based ARGNO should be passed ib by - callers only for attributes with more than one argument. */ + POS and the expected type CODE. Treat CODE == INTEGER_TYPE as + matching all C integral types except bool. If successful, return + POS after default conversions, if any. Otherwise, issue appropriate + warnings and return null. A non-zero 1-based ARGNO should be passed + in by callers only for attributes with more than one argument. */ tree positional_argument (const_tree fntype, const_tree atname, tree pos, @@ -630,11 +631,11 @@ positional_argument (const_tree fntype, const_tree return NULL_TREE; } - /* Where the expected code is STRING_CST accept any pointer - to a narrow character type, qualified or otherwise. */ bool type_match; if (code == STRING_CST && POINTER_TYPE_P (argtype)) { + /* Where the expected code is STRING_CST accept any pointer + to a narrow character type, qualified or otherwise. */ tree type = TREE_TYPE (argtype); type = TYPE_MAIN_VARIANT (type); type_match = (type == char_type_node @@ -641,6 +642,11 @@ positional_argument (const_tree fntype, const_tree || type == signed_char_type_node || type == unsigned_char_type_node); } + else if (code == INTEGER_TYPE) + /* For integers, accept enums, wide characters and other types + that match INTEGRAL_TYPE_P except for bool. */ + type_match = (INTEGRAL_TYPE_P (argtype) + && TREE_CODE (argtype) != BOOLEAN_TYPE); else type_match = TREE_CODE (argtype) == code; Index: gcc/doc/extend.texi =================================================================== --- gcc/doc/extend.texi (revision 266966) +++ gcc/doc/extend.texi (working copy) @@ -2471,7 +2471,9 @@ The @code{aligned} attribute can also be used for @item alloc_align (@var{position}) @cindex @code{alloc_align} function attribute The @code{alloc_align} attribute may be applied to a function that -returns a pointer and takes at least one argument of an integer type. +returns a pointer and takes at least one argument of an integer or +enumerated type, but not @code{bool}. Arguments of other types are +diagnosed. It indicates that the returned pointer is aligned on a boundary given by the function argument at @var{position}. Meaningful alignments are powers of 2 greater than one. GCC uses this information to improve @@ -2495,7 +2497,9 @@ given by parameter 1. @itemx alloc_size (@var{position-1}, @var{position-2}) @cindex @code{alloc_size} function attribute The @code{alloc_size} attribute may be applied to a function that -returns a pointer and takes at least one argument of an integer type. +returns a pointer and takes at least one argument of an integer or +enumerated type, but not @code{bool}. Arguments of other types are +diagnosed. It indicates that the returned pointer points to memory whose size is given by the function argument at @var{position-1}, or by the product of the arguments at @var{position-1} and @var{position-2}. Meaningful Index: gcc/testsuite/c-c++-common/attributes-4.c =================================================================== --- gcc/testsuite/c-c++-common/attributes-4.c (nonexistent) +++ gcc/testsuite/c-c++-common/attributes-4.c (working copy) @@ -0,0 +1,47 @@ +/* PR c/88363 - alloc_align attribute doesn't accept enumerated arguments + Verify that attribute positional arguments can refer to all C integer + types in both C and C++. + { dg-do compile } + { dg-options "-Wall" } + { dg-options "-Wall -Wno-c++-compat" { target c } } */ + +#define ATTR(...) __attribute__ ((__VA_ARGS__)) + +#if __cplusplus == 199711L +typedef __CHAR16_TYPE__ char16_t; +typedef __CHAR32_TYPE__ char32_t; +#elif !__cplusplus +typedef _Bool bool; +typedef __CHAR16_TYPE__ char16_t; +typedef __CHAR32_TYPE__ char32_t; +typedef __WCHAR_TYPE__ wchar_t; +#endif + +enum A { A0 }; + +ATTR (alloc_align (1)) void* falloc_align_char (char); +ATTR (alloc_align (1)) void* falloc_align_char16 (char16_t); +ATTR (alloc_align (1)) void* falloc_align_char32 (char32_t); +ATTR (alloc_align (1)) void* falloc_align_wchar (wchar_t); +/* Using an enum might make sense in an API that limits the alignments + it accepts to just the set of the defined enumerators. */ +ATTR (alloc_align (1)) void* falloc_align_enum (enum A); +ATTR (alloc_align (1)) void* falloc_align_int128 (__int128_t); + + +ATTR (alloc_align (1)) void* falloc_size_char (char); +ATTR (alloc_size (1)) void* falloc_size_char16 (char16_t); +ATTR (alloc_size (1)) void* falloc_size_char32 (char32_t); +ATTR (alloc_size (1)) void* falloc_size_wchar (wchar_t); +ATTR (alloc_size (1)) void* falloc_size_enum (enum A); +ATTR (alloc_align (1)) void* falloc_size_int128 (__int128_t); + + +typedef struct { int i; } S; + +/* Using bool is most likely a bug and so diagnosed even though + it could be accepted. None of the other types makes sense. */ +ATTR (alloc_align (1)) void* falloc_align_bool (bool); /* { dg-warning "attribute argument value .1. refers to parameter type .\(_Bool|bool\)" } */ +ATTR (alloc_align (1)) void* falloc_align_float (float); /* { dg-warning "attribute argument value .1. refers to parameter type .float" } */ +ATTR (alloc_align (1)) void* falloc_align_voidp (void*); /* { dg-warning "attribute argument value .1. refers to parameter type .void ?\\\*" } */ +ATTR (alloc_align (1)) void* falloc_align_struct (S); /* { dg-warning "attribute argument value .1. refers to parameter type .S" } */ --------------45ED00EDEB25C6B43DC0B287--