This is the mail archive of the gcc-patches@gcc.gnu.org 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 1/3] improve detection of attribute conflicts (PR 81544)


On 04/04/2018 10:51 AM, Andreas Krebbel wrote:
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?

It looks fine to me.  I can't approve patches but this one looks
trivially safe so I think you can commit it without a formal
approval.

Thanks for fixing the failure!

Martin


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