Re: Fix inconsistent section flags

[Jeff Law <law@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On 07/17/2017 08:35 AM, Joerg Sonnenberger wrote:
> Hello,
> attached patch fixes inconsistent handling of section flags when using
> the section attribute, i.e.:
> 
> __attribute__((section("writable1"))) int foo1;
> __attribute__((section("readonly1"))) const int foo1c;
> __attribute__((section("writable2"))) int foo2 = 42;
> __attribute__((section("readonly2"))) const int foo2c = 42;
> 
> should give section attributes of "aw", "a", "aw", "a" in that order.
> Currently, "foo1c" is classified as BSS though and therefore put into a
> writable section.
ISTM the test we need here is whether or not the underlying DECL is
readonly.  If it READONLY, then it shouldn't go into .bss, but should
instead to into a readable section.

Testing based on names seems wrong.

Does the attached (untested) patch work for you?

JEff

[-- Attachment #2: Q --]
[-- Type: text/plain, Size: 2100 bytes --]

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 91680d6..37438c1 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -976,16 +976,16 @@ decode_reg_name (const char *name)
 bool
 bss_initializer_p (const_tree decl)
 {
-  return (DECL_INITIAL (decl) == NULL
-	  /* In LTO we have no errors in program; error_mark_node is used
-	     to mark offlined constructors.  */
-	  || (DECL_INITIAL (decl) == error_mark_node
-	      && !in_lto_p)
-	  || (flag_zero_initialized_in_bss
-	      /* Leave constant zeroes in .rodata so they
-		 can be shared.  */
-	      && !TREE_READONLY (decl)
-	      && initializer_zerop (DECL_INITIAL (decl))));
+  /* Do not put constants into the .bss section, they belong in a readonly
+     section.  */
+  return (!TREE_READONLY (decl)
+	  && (DECL_INITIAL (decl) == NULL
+	      /* In LTO we have no errors in program; error_mark_node is used
+	         to mark offlined constructors.  */
+	      || (DECL_INITIAL (decl) == error_mark_node
+	          && !in_lto_p)
+	      || (flag_zero_initialized_in_bss
+	          && initializer_zerop (DECL_INITIAL (decl)))));
 }
 
 /* Compute the alignment of variable specified by DECL.
@@ -6508,7 +6508,8 @@ categorize_decl_for_section (const_tree decl, int reloc)
 	ret = SECCAT_BSS;
       else if (! TREE_READONLY (decl)
 	       || TREE_SIDE_EFFECTS (decl)
-	       || ! TREE_CONSTANT (DECL_INITIAL (decl)))
+	       || (DECL_INITIAL (decl)
+		   && ! TREE_CONSTANT (DECL_INITIAL (decl))))
 	{
 	  /* Here the reloc_rw_mask is not testing whether the section should
 	     be read-only or not, but whether the dynamic link will have to
@@ -6528,7 +6529,8 @@ categorize_decl_for_section (const_tree decl, int reloc)
 	   location.  -fmerge-all-constants allows even that (at the
 	   expense of not conforming).  */
 	ret = SECCAT_RODATA;
-      else if (TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+      else if (DECL_INITIAL (decl)
+	       && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
 	ret = SECCAT_RODATA_MERGE_STR_INIT;
       else
 	ret = SECCAT_RODATA_MERGE_CONST;