Segher,

Please ignore the attachments in my last email. My editor cached the old things. Now they should be correct. Sorry for that.



On 17/8/2020 上午 10:20, HAO CHEN GUI wrote:
Segher,

Seems I sent the wrong diff file. Now the attachments should be correct ones. Sorry for that.

For the reloc,  my understanding is the jump table needs to be relocated if it's a non-relative jump table and PIC flag is set at the same time.

//stmt.c

 if (CASE_VECTOR_PC_RELATIVE
          || (flag_pic && targetm.asm_out.generate_pic_addr_diff_vec ()))
    emit_jump_table_data (gen_rtx_ADDR_DIFF_VEC (CASE_VECTOR_MODE,
                                                 gen_rtx_LABEL_REF (Pmode,
table_label),
                                                 gen_rtvec_v (ncases, labelvec),                                                  const0_rtx, const0_rtx));
  else
    emit_jump_table_data (gen_rtx_ADDR_VEC (CASE_VECTOR_MODE,
                                            gen_rtvec_v (ncases, labelvec)));

According to the slice of code in stmt.c,  the non-relative jump table is created with PIC flag set when CASE_VECTOR_PC_RELATIVE is false, flag_pic is true and targetm.asm_out.generate_pic_addr_diff_vec is false. So I set the reloc to

reloc = (! CASE_VECTOR_PC_RELATIVE && flag_pic &&
           ! targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 : 0;

The funcation_rodata_section is not only for jump tables. It's no relro in other cases. I am not sure if it's suitable to put selecting relro section in it. Of course, I can create a separate function for section selection of jump table and send its output to funcation_rodata_section.

Please advice. Thanks a lot.


On 15/8/2020 上午 7:39, Segher Boessenkool wrote:

Hi!

On Fri, Aug 14, 2020 at 03:20:03PM +0800, HAO CHEN GUI wrote:
section *
select_jump_table_section (tree decl)
{
   int reloc;
   bool section_reloc;

   reloc = (! CASE_VECTOR_PC_RELATIVE && flag_pic &&
            ! targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 :
0;
(Modern style is no space after "!", and your mailer wrapped the code).

   section_reloc = (reloc & targetm.asm_out.reloc_rw_mask ());
(No space after & either).

   return targetm.asm_out.function_rodata_section (decl, section_reloc);
So you are saying it should not go into relro if either there is no pic
flag, the targets are pc relative, or pic uses some address diff?

This seems too complicated, and/or not the right abstraction.


diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 3be984bbd5c..0ac1488c837 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5007,6 +5007,8 @@ it is unlikely to be called.
    @hook TARGET_ASM_FUNCTION_RODATA_SECTION
  +@hook TARGET_ASM_FUNCTION_RELRO_SECTION
I would expect people to just use TARGET_ASM_FUNCTION_RODATA_SECTION to
get the .data.rel.ro?  With another argument then.

+section *
+select_jump_table_section (tree decl)
+{
+  int relco;
"reloc"?

+  relco = JUMP_TABLES_IN_RELRO;
Just put that on the same line as the declaration?

+  if (relco & targetm.asm_out.reloc_rw_mask ())
+    return targetm.asm_out.function_relro_section (decl);
+  else
+    return targetm.asm_out.function_rodata_section (decl);
+}
(trailing spaces btw)

@@ -2492,7 +2513,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
          {
            int log_align;
  -          switch_to_section (targetm.asm_out.function_rodata_section
+          switch_to_section (select_jump_table_section
                   (current_function_decl));
          section *section
        = select_jump_table_section (current_function_decl));
          switch_to_section (section);

(but it would be better if we didn't indent so deeply here).


I think this should be split into a function just selecting the relro
section (either directly, or from the rodata selection function), and
then separately the jumptable section thing.

There is a lot of stuff here that seems confused, and a lot of that
already was there :-(


Segher
        * final.c (select_jump_table_section): Implement a function to select 
                the section of jump tables by reloc_rw_mask and other flags.
                * output.h (default_function_rodata_section, 
default_no_function_rodata_section):
                Add the second argument to the declarations.
        * target.def (function_rodata_section): Change the doc and add the 
second argument.
                * doc/tm.texi: Regenerate.
        * varasm.c (default_function_rodata_section, 
default_no_function_rodata_section, 
                function_mergeable_rodata_prefix): Add the second argument in 
default_function_rodata_section.
                It indicates the section should be read-only or relocation 
read-only.
                Add the second argument in default_function_rodata_section. Set 
the second argument to
                false when default_function_rodata_section calls 
function_rodata_section.
                * config/mips/mips.c (mips_function_rodata_section): Add the 
second arugment and set it
                to false when it calls function_rodata_section.
                * config/s390/s390.c (targetm.asm_out.function_rodata_section): 
                Set the second argument to false.
                * config/s390/s390.md Likewise.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 513fc5fe295..afb0e1a9f2f 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -9315,10 +9315,11 @@ mips_select_rtx_section (machine_mode mode, rtx x,
    default_function_rodata_section.  */
 
 static section *
-mips_function_rodata_section (tree decl)
+mips_function_rodata_section (tree decl, 
+                             bool section_reloc ATTRIBUTE_UNUSED)
 {
   if (!TARGET_ABICALLS || TARGET_ABSOLUTE_ABICALLS || TARGET_GPWORD)
-    return default_function_rodata_section (decl);
+    return default_function_rodata_section (decl, false);
 
   if (decl && DECL_SECTION_NAME (decl))
     {
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 758315c0c72..eb619d39ad7 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -11639,7 +11639,7 @@ s390_output_split_stack_data (rtx parm_block, rtx 
call_done,
   rtx ops[] = { parm_block, call_done };
 
   switch_to_section (targetm.asm_out.function_rodata_section
-                    (current_function_decl));
+                    (current_function_decl, false));
 
   if (TARGET_64BIT)
     output_asm_insn (".align\t8", NULL);
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index cd1c0634b71..6ca03ed9002 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -11218,7 +11218,7 @@
   ""
 {
   switch_to_section (targetm.asm_out.function_rodata_section
-                (current_function_decl));
+                (current_function_decl, false));
   return "";
 }
   [(set_attr "length" "0")])
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6e7d9dc54a9..fd65cc4dbe7 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7703,13 +7703,13 @@ example, the function @code{foo} would be placed in 
@code{.text.foo}.
 Whatever the actual target object format, this is often good enough.
 @end deftypefn
 
-@deftypefn {Target Hook} {section *} TARGET_ASM_FUNCTION_RODATA_SECTION (tree 
@var{decl})
-Return the readonly data section associated with
+@deftypefn {Target Hook} {section *} TARGET_ASM_FUNCTION_RODATA_SECTION (tree 
@var{decl}, bool @var{section_reloc})
+Return the readonly or reloc readonly data section associated with
 @samp{DECL_SECTION_NAME (@var{decl})}.
 The default version of this function selects @code{.gnu.linkonce.r.name} if
 the function's section is @code{.gnu.linkonce.t.name}, @code{.rodata.name}
-if function is in @code{.text.name}, and the normal readonly-data section
-otherwise.
+or @code{.data.rel.ro.name} if function is in @code{.text.name}, and 
+the normal readonly-data or reloc readonly data section otherwise.
 @end deftypefn
 
 @deftypevr {Target Hook} {const char *} TARGET_ASM_MERGEABLE_RODATA_PREFIX
diff --git a/gcc/final.c b/gcc/final.c
index a3601964a8d..380829db4f7 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -2154,6 +2154,23 @@ asm_show_source (const char *filename, int linenum)
   fputc ('\n', asm_out_file);
 }
 
+/* Select sections for jump table.
+   If the jump table need to be relocated, 
+   select relro sections. Otherwise in readonly section */
+
+section *
+select_jump_table_section (tree decl)
+{
+  int reloc;
+  bool section_reloc;
+
+  reloc = (! CASE_VECTOR_PC_RELATIVE && flag_pic && 
+           ! targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 : 0;
+
+  section_reloc = (reloc & targetm.asm_out.reloc_rw_mask ());
+  return targetm.asm_out.function_rodata_section (decl, section_reloc);
+}
+
 /* The final scan for one insn, INSN.
    Args are same as in `final', except that INSN
    is the insn being scanned.
@@ -2492,7 +2509,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int 
optimize_p ATTRIBUTE_UNUSED,
            {
              int log_align;
 
-             switch_to_section (targetm.asm_out.function_rodata_section
+             switch_to_section (select_jump_table_section
                                 (current_function_decl));
 
 #ifdef ADDR_VEC_ALIGN
@@ -2571,7 +2588,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int 
optimize_p ATTRIBUTE_UNUSED,
 #endif
 
            if (! JUMP_TABLES_IN_TEXT_SECTION)
-             switch_to_section (targetm.asm_out.function_rodata_section
+             switch_to_section (select_jump_table_section
                                 (current_function_decl));
            else
              switch_to_section (current_function_section ());
diff --git a/gcc/output.h b/gcc/output.h
index eb253c50329..661ce9074ae 100644
--- a/gcc/output.h
+++ b/gcc/output.h
@@ -571,8 +571,8 @@ extern void default_ctor_section_asm_out_constructor (rtx, 
int);
 extern section *default_select_section (tree, int, unsigned HOST_WIDE_INT);
 extern section *default_elf_select_section (tree, int, unsigned HOST_WIDE_INT);
 extern void default_unique_section (tree, int);
-extern section *default_function_rodata_section (tree);
-extern section *default_no_function_rodata_section (tree);
+extern section *default_function_rodata_section (tree, bool);
+extern section *default_no_function_rodata_section (tree, bool);
 extern section *default_clone_table_section (void);
 extern section *default_select_rtx_section (machine_mode, rtx,
                                            unsigned HOST_WIDE_INT);
diff --git a/gcc/target.def b/gcc/target.def
index 07059a87caf..fb34507dd89 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -549,16 +549,16 @@ Whatever the actual target object format, this is often 
good enough.",
  void, (tree decl, int reloc),
  default_unique_section)
 
-/* Return the readonly data section associated with function DECL.  */
+/* Return the readonly or reloc readonly data section associated with function 
DECL. */
 DEFHOOK
 (function_rodata_section,
- "Return the readonly data section associated with\n\
+ "Return the readonly or reloc readonly data section associated with\n\
 @samp{DECL_SECTION_NAME (@var{decl})}.\n\
 The default version of this function selects @code{.gnu.linkonce.r.name} if\n\
 the function's section is @code{.gnu.linkonce.t.name}, @code{.rodata.name}\n\
-if function is in @code{.text.name}, and the normal readonly-data section\n\
-otherwise.",
- section *, (tree decl),
+or @code{.data.rel.ro.name} if function is in @code{.text.name}, and \n\
+the normal readonly-data or reloc readonly data section otherwise.",
+ section *, (tree decl, bool section_reloc),
  default_function_rodata_section)
 
 /* Nonnull if the target wants to override the default ".rodata" prefix
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 4070f9c17e8..bf5a8fbb0ea 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -726,12 +726,25 @@ switch_to_other_text_partition (void)
   switch_to_section (current_function_section ());
 }
 
-/* Return the read-only data section associated with function DECL.  */
+/* Return the read-only or relocation read-only data section associated with 
function DECL.  */
 
 section *
-default_function_rodata_section (tree decl)
+default_function_rodata_section (tree decl, bool section_reloc)
 {
-  if (decl != NULL_TREE && DECL_SECTION_NAME (decl))
+  const char* sname;
+  unsigned int flags;
+
+  flags = 0;
+
+  if (section_reloc)
+    {
+      sname = ".data.rel.ro";
+      flags = (SECTION_WRITE | SECTION_RELRO);
+    }
+  else
+    sname = ".rodata";
+  
+  if (decl && DECL_SECTION_NAME (decl))
     {
       const char *name = DECL_SECTION_NAME (decl);
 
@@ -744,12 +757,12 @@ default_function_rodata_section (tree decl)
          dot = strchr (name + 1, '.');
          if (!dot)
            dot = name;
-         len = strlen (dot) + 8;
+         len = strlen (dot) + strlen (sname) + 1;
          rname = (char *) alloca (len);
 
-         strcpy (rname, ".rodata");
+         strcpy (rname, sname);
          strcat (rname, dot);
-         return get_section (rname, SECTION_LINKONCE, decl);
+         return get_section (rname, (SECTION_LINKONCE | flags), decl);
        }
       /* For .gnu.linkonce.t.foo we want to use .gnu.linkonce.r.foo.  */
       else if (DECL_COMDAT_GROUP (decl)
@@ -767,15 +780,18 @@ default_function_rodata_section (tree decl)
               && strncmp (name, ".text.", 6) == 0)
        {
          size_t len = strlen (name) + 1;
-         char *rname = (char *) alloca (len + 2);
+         char *rname = (char *) alloca (len + strlen(sname) - 5);
 
-         memcpy (rname, ".rodata", 7);
-         memcpy (rname + 7, name + 5, len - 5);
-         return get_section (rname, 0, decl);
+         memcpy (rname, sname, strlen(sname));
+         memcpy (rname + strlen(sname), name + 5, len - 5);
+         return get_section (rname, flags, decl);
        }
     }
 
-  return readonly_data_section;
+  if (section_reloc)
+    return get_section (sname, flags, decl);
+  else
+    return readonly_data_section;
 }
 
 /* Return the read-only data section associated with function DECL
@@ -783,7 +799,8 @@ default_function_rodata_section (tree decl)
    readonly data section.  */
 
 section *
-default_no_function_rodata_section (tree decl ATTRIBUTE_UNUSED)
+default_no_function_rodata_section (tree decl ATTRIBUTE_UNUSED, 
+                                   bool section_reloc ATTRIBUTE_UNUSED)
 {
   return readonly_data_section;
 }
@@ -793,7 +810,8 @@ default_no_function_rodata_section (tree decl 
ATTRIBUTE_UNUSED)
 static const char *
 function_mergeable_rodata_prefix (void)
 {
-  section *s = targetm.asm_out.function_rodata_section (current_function_decl);
+  section *s = targetm.asm_out.function_rodata_section (current_function_decl, 
+                                                       false);
   if (SECTION_STYLE (s) == SECTION_NAMED)
     return s->named.name;
   else

Reply via email to