On 04/04/2018 06:16 PM, Andreas Krebbel wrote:
> On 10/03/2017 12:23 AM, Jeff Law wrote:
>> 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
>>
>
> On targets enforcing a function alignment bigger than 4 bytes this triggers
> an error instead:
>
> +inline int ATTR ((aligned (4)))
> +finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned
> \\(4\\). because it
> conflicts with attribute .aligned \\(8\\)." } */
>
> gcc/gcc/testsuite/c-c++-common/Wattributes.c:404:1: error: alignment for
> 'finline_hot_noret_align'
> must be at least 8^M
>
diff --git a/gcc/testsuite/c-c++-common/Wattributes.c
b/gcc/testsuite/c-c++-common/Wattributes.c
index 902bcb61c30..a260d018dcf 100644
--- a/gcc/testsuite/c-c++-common/Wattributes.c
+++ b/gcc/testsuite/c-c++-common/Wattributes.c
@@ -401,7 +401,8 @@ inline int ATTR ((warn_unused_result))
finline_hot_noret_align (int); /* { dg-warning "ignoring attribute
.warn_unused_result. because it
conflicts with attribute .noreturn." } */
inline int ATTR ((aligned (4)))
-finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned
\\(4\\). because it
conflicts with attribute .aligned \\(8\\)." } */
+ finline_hot_noret_align (int); /* { dg-warning "ignoring attribute .aligned
\\(4\\). because it
conflicts with attribute .aligned \\(8\\)." "" { target { ! s390*-*-* } } } */
+/* { dg-error "alignment for 'finline_hot_noret_align' must be at least 8" ""
{ target s390*-*-* }
.-1 } */
inline int ATTR ((aligned (8)))
finline_hot_noret_align (int);
OK?
-Andreas-
