This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)
- From: Jeff Law <law at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>, Joseph Myers <joseph at codesourcery dot com>
- Cc: Marek Polacek <polacek at redhat dot com>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>
- Date: Mon, 2 Oct 2017 16:23:54 -0600
- Subject: Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=law at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9ECC37E425
- References: <f48353a5-a1b9-6a2d-98c9-831c5d9d44a0@gmail.com> <20170809115341.GM17069@redhat.com> <52dbf0e4-09b3-f313-d6b1-92f0e5ac6c16@gmail.com> <alpine.DEB.2.20.1708091711010.18502@digraph.polyomino.org.uk> <c6150c61-3774-bfc7-b909-05efda68cfc4@gmail.com> <alpine.DEB.2.20.1708091949180.9262@digraph.polyomino.org.uk> <0fc9f24b-f474-3ab5-e660-b206486cc644@gmail.com> <alpine.DEB.2.20.1708102145580.8101@digraph.polyomino.org.uk> <61464927-ba0d-860d-d08f-5d36d473e3c2@gmail.com> <c6c50436-d6f8-69ee-3550-432cd3e32bc7@gmail.com> <alpine.DEB.2.20.1709062317320.29155@digraph.polyomino.org.uk> <cb5c4c9f-a4ee-6488-4a15-8bbd2be46da6@gmail.com> <alpine.DEB.2.20.1709192056310.21037@digraph.polyomino.org.uk> <b2c7fd33-7f73-9df3-6d0e-dad8650ae99c@gmail.com>
On 09/20/2017 12:04 PM, Martin Sebor wrote:
> On 09/19/2017 03:00 PM, Joseph Myers wrote:
>> On Tue, 19 Sep 2017, Martin Sebor wrote:
>>
>>>> In general, the data structures where you need to ensure manually that if
>>>>
>>>> attribute A is listed in EXCL for B, then attribute B is also listed in
>>>> EXCL for A, seem concerning. I'd expect either data structures that make
>>>>
>>>> such asymmetry impossible, or a self-test that verifies that the tables in
>>>>
>>>> use are in fact symmetric (unless there is some reason the symmetry is not
>>>>
>>>> in fact required and symmetric diagnostics still result from asymmetric
>>>> tables - in which case the various combinations and orderings of
>>>> gnu_inline and noinline definitely need tests to show that the diagnostics
>>>>
>>>> work).
>>>
>>> If I understand correctly what you're concerned about then I don't
>>> think there are any such cases in the updated version of the patch.
>>
>> I don't see how you ensure that it's not possible to have such asymmetry.
>> My point wasn't so much "there was a bug in the previous patch version" as
>>
>> "the choice of data structures for defining such exclusions is prone to
>> such bugs". Which can be addressed either by using different data
>> structures (e.g. listing incompatible pairs in a single array) or by a
>> self-test to verify symmetry so a compiler with asymmetry doesn't build.
>
> Okay, that's a useful thing to add. It exposed a couple of missing
> attribute exclusions that I had overlooked. Thanks for the suggestion!
> Attached is an incremental diff with just these changes to make review
> easier and an updated patch.
>
> As an aside, there are a number of other possible logic errors in
> the attribute specifications that could be detected by self-tests.
> The one I ran into is misspelled attribute names. The added test
> detects misspelled names in exclusions, but not in the main specs.
>
> Martin
>
> gcc-81544-1-inc.diff
>
>
> gcc-81544-1.diff
>
>
> PR c/81544 - attribute noreturn and warn_unused_result on the same function accepted
>
> gcc/c/ChangeLog:
>
> PR c/81544
> * c-decl.c (c_decl_attributes): Look up existing declaration and
> pass it to decl_attributes.
>
> gcc/c-family/ChangeLog:
>
> PR c/81544
> * c-attribs.c (attr_aligned_exclusions): New array.
> (attr_alloc_exclusions, attr_cold_hot_exclusions): Same.
> (attr_common_exclusions, attr_const_pure_exclusions): Same.
> (attr_gnu_inline_exclusions, attr_inline_exclusions): Same.
> (attr_noreturn_exclusions, attr_returns_twice_exclusions): Same.
> (attr_warn_unused_result_exclusions): Same.
> (handle_hot_attribute, handle_cold_attribute): Simplify.
> (handle_const_attribute): Warn on function returning void.
> (handle_pure_attribute): Same.
> * c-warn.c (diagnose_mismatched_attributes): Simplify.
>
> gcc/ChangeLog:
>
> PR c/81544
> * attribs.c (empty_attribute_table): Initialize new member of
> struct attribute_spec.
> (decl_attributes): Add argument. Handle mutually exclusive
> combinations of attributes.
> * attribs.h (decl_attributes): Add default argument.
> * selftest.h (attribute_c_tests): Declare.
> * selftest-run-tests.c (selftest::run_tests): Call attribute_c_tests.
> * tree-core.h (attribute_spec::exclusions, exclude): New type and
> member.
> * doc/extend.texi (Common Function Attributes): Update const and pure.
>
> gcc/testsuite/ChangeLog:
>
> PR c/81544
> * c-c++-common/Wattributes-2.c: New test.
> * c-c++-common/Wattributes.c: New test.
> * c-c++-common/attributes-3.c: Adjust.
> * gcc.dg/attr-noinline.c: Adjust.
> * gcc.dg/pr44964.c: Same.
> * gcc.dg/torture/pr42363.c: Same.
> * gcc.dg/tree-ssa/ssa-ccp-2.c: Same.
OK.
jeff