On Fri, Aug 4, 2017 at 9:42 AM, Martin Liška <mli...@suse.cz> wrote:
> On 08/02/2017 01:25 PM, Joseph Myers wrote:
>> On Thu, 13 Jul 2017, Martin Liška wrote:
>>
>>> +/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
>>> + so that we have a canonical form of attribute names. */
>>> +
>>> +static inline tree
>>> +canonicalize_attr_name (tree attr_name)
>>> +{
>>> + const size_t l = IDENTIFIER_LENGTH (attr_name);
>>> + const char *s = IDENTIFIER_POINTER (attr_name);
>>> +
>>> + if (l > 4 && s[0] == '_')
>>> + {
>>> + gcc_checking_assert (s[l - 2] == '_');
>>> + return get_identifier_with_length (s + 2, l - 4);
>>> + }
>>> +
>>> + return attr_name;
>>
>> For this to (a) be correct, (b) not trigger the assertion, there must be a
>> precondition that attr_name either starts and ends with __, or does not
>> start with _, or has length 4 or less. I don't see anything in the
>> callers to ensure this precondition holds, so that, for example,
>> __attribute__ ((_foobar)) does not trigger the assertion, and
>> __attribute__ ((_xformat__)) is not wrongly interpreted as a format
>> attribute (and similarly for attribute arguments when those are
>> canonicalized).
>
> Thanks for the review. I've updated to to canonicalize just __.+__.
> I added test-case for that: gcc/testsuite/gcc.dg/Wattributes-5.c.
>
>>
>>> +/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
>>> + ATTR2_LEN. */
>>> +
>>> +static inline bool
>>> +cmp_attribs (const char *attr1, size_t attr1_len,
>>> + const char *attr2, size_t attr2_len)
>>> +{
>>> + gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
>>> + gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');
>
> And I removed these asserts.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
OK.
Jason
