[gcc(refs/vendors/ARM/heads/morello)] Section attribute and RELRO handling

[gcc(refs/vendors/ARM/heads/morello)] Section attribute and RELRO handling

[Matthew Malcomson]

https://gcc.gnu.org/g:407e0b5441f9d544549753228bf0ba9aeb22ce53

commit 407e0b5441f9d544549753228bf0ba9aeb22ce53
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Fri Apr 8 17:20:35 2022 +0100

    Section attribute and RELRO handling
    
    If the user requests a specific section for a variable, GCC determines
    whether that section should be readonly, relro, or writeable based
    mainly on the properties of that decl (e.g. whether it is const or not).
    
    Glibc requests that some non-const variables are put in the .data.rel.ro
    section.  This is valid in the part of glibc that uses it, since glibc
    wants to assign to these variables before the `mprotect` call which
    turns that section to readonly.
    
    Due to the extra use of the .data.rel.ro section in Morello GCC when
    creating variables to be initialised as capabilities by the runtime, we
    end up using the .data.rel.ro through default_elf_select_rtx_section.
    In this case, default_elf_select_rtx_section explicitly requests the
    .data.rel.ro section is marked as SECTION_RELRO.
    
    This request for a SECTION_RELRO flag clashes with the earlier
    determination that .data.rel.ro is SECTION_WRITE due to the non-const
    decl we put in it.  This triggers an error.
    
    This patch adds an exception in `default_section_type_flags` so that if
    we're trying to determine the section flags that we would like for a
    DECL's section we account for if that section is named .data.rel.ro*.
    
    Throughout GCC a sections name is quite strongly tied to the properties
    that section will have.  This patch ensures that the user can not break
    that tie in ways that GCC can not handle.
    
    Alternatives considered:
    
    -- Override flags in `get_section` --
    get_section already contains a clause to allow overwriting of a sections
    flags from readonly to RELRO.  We could add another condition allowing
    the switch from WRITE to RELRO.
    
    This seems a bit too lax.  In general putting a non-const variable in a
    RELRO section is invalid, it's only in the special case inside glibc
    before the RELRO section has been made invalid that this is OK.
    
    -- Allow specifying RELRO as an argument in an attribute --
    This approach seems problematic since the compiler can not determine
    whether a section is RELRO or not.  That property is imparted to a
    section by the linker script.
    
    Allowing the user to mark a section as RELRO could therefore cause
    confusion where the user requests something and the resulting binary
    does not have that behaviour.
    
    -- Allow specifying all flags (not just RELRO) --
    This approach would raise questions about how to specify the flags
    (especially given that different targets have different avaialable
    flags).
    
    It would also break the strong tie between section name and section
    flags.  Until now the only section flags that we took from the DECL were
    to do with the readonly / RELRO / WRITE option.  Changing this seems
    like it would add unnecessary complexity for the motivating case of
    specifying a RELRO section.

Diff:
---
 .../gcc.target/aarch64/morello/relro-attribute-accepted.c      |  6 ++++++
 gcc/varasm.c                                                   | 10 ++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/aarch64/morello/relro-attribute-accepted.c b/gcc/testsuite/gcc.target/aarch64/morello/relro-attribute-accepted.c
new file mode 100644
index 00000000000..b71acbaea3b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/morello/relro-attribute-accepted.c
@@ -0,0 +1,6 @@
+static __uintcap_t testvar __attribute__ ((section (".data.rel.ro")));
+int main()
+{
+  testvar = 1;
+  return 0;
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 029a1e9fbac..b52b8ab6a5d 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6669,12 +6669,7 @@ default_section_type_flags (tree decl, const char *name, int reloc)
 	flags = SECTION_WRITE;
     }
   else
-    {
-      flags = SECTION_WRITE;
-      if (strcmp (name, ".data.rel.ro") == 0
-	  || strcmp (name, ".data.rel.ro.local") == 0)
-	flags |= SECTION_RELRO;
-    }
+    flags = SECTION_WRITE;
 
   if (decl && DECL_P (decl) && DECL_COMDAT_GROUP (decl))
     flags |= SECTION_LINKONCE;
@@ -6694,6 +6689,9 @@ default_section_type_flags (tree decl, const char *name, int reloc)
       || strncmp (name, ".gnu.linkonce.sb.", 17) == 0)
     flags |= SECTION_BSS;
 
+  if (strncmp (name, ".data.rel.ro", 12) == 0)
+    flags |= SECTION_WRITE | SECTION_RELRO;
+
   if (strcmp (name, ".tdata") == 0
       || strncmp (name, ".tdata.", 7) == 0
       || strncmp (name, ".gnu.linkonce.td.", 17) == 0)