This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] handle expressions in __builtin_has_attribute (PR 88383)

Jeff, did this and the rest of the discussion answer your question
and if so, is the patch okay to commit?


On 12/13/18 12:48 PM, Martin Sebor wrote:
On 12/13/18 12:20 PM, Martin Sebor wrote:
On 12/13/18 11:59 AM, Jeff Law wrote:
On 12/5/18 8:55 PM, Martin Sebor wrote:
The __builtin_has_attribute function fails with an ICE when
its first argument is an expression such as INDIRECT_REF, or
many others.  The code simply assumes it's either a type or
a decl.  The attached patch corrects this oversight.

While testing the fix I also noticed the C++ front end expects
the first operand to be a unary expression, which causes most
other kinds of expressions to be rejected.  The patch fixes
that as well.

Finally, while testing the fix even more I realized that
the built-in considers the underlying array itself in ARRAY_REF
expressions rather than its type, which leads to inconsistent
results for overaligned arrays (it's the array itself that's
overaligned, not its elements).  So I fixed that too and
adjusted the one test designed to verify this.

Tested on x86_64-linux.



PR c/88383 - ICE calling __builtin_has_attribute on a reference


    PR c/88383
    * c-attribs.c (validate_attribute): Handle expressions.
    (has_attribute): Handle types referenced by expressions.
    Avoid considering array attributes in ARRAY_REF expressions .


    PR c/88383
    * parser.c (cp_parser_has_attribute_expression): Handle assignment


    PR c/88383
    * c-c++-common/builtin-has-attribute-4.c: Adjust expectations.
    * c-c++-common/builtin-has-attribute-6.c: New test.
Well, the high level question here is do we want to support this builtin
on expressions at all.  Generally attributes apply to types and decls,
not expressions.

Clearly we shouldn't fault, but my first inclination is that the
attribute applies to types or decls, not expressions.  In that case we
should just be issuing an error.

I could be convinced otherwise, so if you think we should support
passing expressions to this builtin, make your case.

The support is necessary in order to determine the attributes
in expressions such as:

   struct S { __attribute__ ((packed)) int a[32]; };

   extern struct S s;

   _Static_assert (__builtin_has_attribute (s.a, packed));

An example involving types might be a better one:

   typedef __attribute__ ((may_alias)) int* BadInt;

   void f (BadInt *p)
     _Static_assert (__builtin_has_attribute (*p, may_alias));


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]