public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-17] varasm: Two SECTION_RETAIN fixes [PR100130]
@ 2021-04-21  8:08 Richard Sandiford
  0 siblings, 0 replies; only message in thread
From: Richard Sandiford @ 2021-04-21  8:08 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:b67778b59932fdc64dc89a97550349e973ca0352

commit r12-17-gb67778b59932fdc64dc89a97550349e973ca0352
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Wed Apr 21 09:08:44 2021 +0100

    varasm: Two SECTION_RETAIN fixes [PR100130]
    
    switch_to_section warns if we try to output a retain decl in a
    section without a retain flag, or if we try to output a non-retain
    decl in a section with a retain flag.  However, the warning only
    applied if we were trying to “switch” to the current section.
    This works if all decls that use a section are generated consecutively,
    but not if there is an unrelated decl in between.
    
    This patch makes the check unconditional, but suppresses the warning
    if we're writing the section's named.decl (i.e. the decl from which
    the section name and original flags were derived).
    
    Also, the warning didn't fire for -fsection-anchors, for two reasons:
    we allowed retain and non-retain decls to be put into the same block,
    and we didn't pass a decl to switch_to_section.
    
    Although these are arguably separate bugs, it isn't easy to fix them
    independently without temporarily regressing -fsection-anchor targets.
    
    gcc/
            PR middle-end/100130
            * varasm.c (get_block_for_decl): Make sure that any use of the
            retain attribute matches the section's retain flag.
            (switch_to_section): Check for retain mismatches even when
            changing sections, but do not warn if the given decl is the
            section's named.decl.
            (output_object_block): Pass the first decl in the block (if any)
            to switch_to_section.
    
    gcc/testsuite/
            PR middle-end/100130
            * c-c++-common/attr-retain-10.c: New test.
            * c-c++-common/attr-retain-11.c: Likewise.

Diff:
---
 gcc/testsuite/c-c++-common/attr-retain-10.c | 11 ++++++
 gcc/testsuite/c-c++-common/attr-retain-11.c | 11 ++++++
 gcc/varasm.c                                | 60 +++++++++++++++--------------
 3 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/gcc/testsuite/c-c++-common/attr-retain-10.c b/gcc/testsuite/c-c++-common/attr-retain-10.c
new file mode 100644
index 00000000000..0bac947023a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-retain-10.c
@@ -0,0 +1,11 @@
+/* { dg-do compile { target R_flag_in_section } } */
+/* { dg-skip-if "non-ELF target" { *-*-darwin* powerpc*-*-aix* } } */
+/* { dg-options "-Wall -O2 -fno-toplevel-reorder" } */
+
+int __attribute__((used,retain,section(".data.foo"))) foo2 = 2;
+int between = 1;
+int __attribute__((section(".data.foo"))) foo1 = 1;
+/* { dg-warning "'.*' without 'retain' attribute and '.*' with 'retain' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */
+
+/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */
+/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */
diff --git a/gcc/testsuite/c-c++-common/attr-retain-11.c b/gcc/testsuite/c-c++-common/attr-retain-11.c
new file mode 100644
index 00000000000..d1d3d9e8c9f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-retain-11.c
@@ -0,0 +1,11 @@
+/* { dg-do compile { target R_flag_in_section } } */
+/* { dg-skip-if "non-ELF target" { *-*-darwin* powerpc*-*-aix* } } */
+/* { dg-options "-Wall -O2 -fno-toplevel-reorder" } */
+
+int __attribute__((section(".data.foo"))) foo1 = 1;
+/* { dg-warning "'.*' without 'retain' attribute and '.*' with 'retain' attribute are placed in a section with the same name" "" { target R_flag_in_section } .-1 } */
+int between = 1;
+int __attribute__((used,retain,section(".data.foo"))) foo2 = 2;
+
+/* { dg-final { scan-assembler ".data.foo,\"aw\"" { target R_flag_in_section } } } */
+/* { dg-final { scan-assembler ".data.foo,\"awR\"" { target R_flag_in_section } } } */
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 8bb921faa26..3ecf9e039bb 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1314,6 +1314,10 @@ get_block_for_decl (tree decl)
   if (SECTION_STYLE (sect) == SECTION_NOSWITCH)
     return NULL;
 
+  if (bool (lookup_attribute ("retain", DECL_ATTRIBUTES (decl)))
+      != bool (sect->common.flags & SECTION_RETAIN))
+    return NULL;
+
   return get_block_for_section (sect);
 }
 
@@ -7758,33 +7762,33 @@ output_section_asm_op (const void *directive)
 void
 switch_to_section (section *new_section, tree decl)
 {
-  if (in_section == new_section)
+  bool retain_p;
+  if ((new_section->common.flags & SECTION_NAMED)
+      && decl != nullptr
+      && DECL_P (decl)
+      && ((retain_p = !!lookup_attribute ("retain",
+					  DECL_ATTRIBUTES (decl)))
+	  != !!(new_section->common.flags & SECTION_RETAIN)))
     {
-      bool retain_p;
-      if ((new_section->common.flags & SECTION_NAMED)
-	  && decl != nullptr
-	  && DECL_P (decl)
-	  && ((retain_p = !!lookup_attribute ("retain",
-					      DECL_ATTRIBUTES (decl)))
-	      != !!(new_section->common.flags & SECTION_RETAIN)))
-	{
-	  /* If the SECTION_RETAIN bit doesn't match, switch to a new
-	     section.  */
-	  tree used_decl, no_used_decl;
+      /* If the SECTION_RETAIN bit doesn't match, switch to a new
+	 section.  */
+      tree used_decl, no_used_decl;
 
-	  if (retain_p)
-	    {
-	      new_section->common.flags |= SECTION_RETAIN;
-	      used_decl = decl;
-	      no_used_decl = new_section->named.decl;
-	    }
-	  else
-	    {
-	      new_section->common.flags &= ~(SECTION_RETAIN
-					     | SECTION_DECLARED);
-	      used_decl = new_section->named.decl;
-	      no_used_decl = decl;
-	    }
+      if (retain_p)
+	{
+	  new_section->common.flags |= SECTION_RETAIN;
+	  used_decl = decl;
+	  no_used_decl = new_section->named.decl;
+	}
+      else
+	{
+	  new_section->common.flags &= ~(SECTION_RETAIN
+					 | SECTION_DECLARED);
+	  used_decl = new_section->named.decl;
+	  no_used_decl = decl;
+	}
+      if (no_used_decl != used_decl)
+	{
 	  warning (OPT_Wattributes,
 		   "%+qD without %<retain%> attribute and %qD with "
 		   "%<retain%> attribute are placed in a section with "
@@ -7792,9 +7796,9 @@ switch_to_section (section *new_section, tree decl)
 	  inform (DECL_SOURCE_LOCATION (used_decl),
 		  "%qD was declared here", used_decl);
 	}
-      else
-	return;
     }
+  else if (in_section == new_section)
+    return;
 
   if (new_section->common.flags & SECTION_FORGET)
     in_section = NULL;
@@ -8007,7 +8011,7 @@ output_object_block (struct object_block *block)
       && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0))
     handle_vtv_comdat_section (block->sect, block->sect->named.decl);
   else
-    switch_to_section (block->sect);
+    switch_to_section (block->sect, SYMBOL_REF_DECL ((*block->objects)[0]));
 
   gcc_checking_assert (!(block->sect->common.flags & SECTION_MERGE));
   assemble_align (block->alignment);


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-04-21  8:08 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21  8:08 [gcc r12-17] varasm: Two SECTION_RETAIN fixes [PR100130] Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).