Add support for putting jump table into relocation read-only section

Hi,

This patch adds a section selection for jump tables. The jump tables can 
be put into read-only data section or relocation read-only data section 
by the relocation flags. When the PIC flag is set and jump table is 
non-relative, the jump table is put into relocation read-only 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 attachments are the patch diff file and change log file.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  
Is this okay for trunk? Any recommendations? Thanks a lot.
* 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/varasm.c b/gcc/varasm.c

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

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
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 6e7d9dc54a9..3ff7527f2cc 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7712,6 +7712,15 @@ if function is in @code{.text.name}, and the normal readonly-data section
 otherwise.
 @end deftypefn
 
+@deftypefn {Target Hook} {section *} TARGET_ASM_FUNCTION_RELRO_SECTION (tree @var{decl})
+Return the relro 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{.data.rel.ro.name} if function is in @code{.text.name}, 
+and the normal data relro section otherwise.
+@end deftypefn
+
 @deftypevr {Target Hook} {const char *} TARGET_ASM_MERGEABLE_RODATA_PREFIX
 Usually, the compiler uses the prefix @code{".rodata"} to construct
 section names for mergeable constant data.  Define this macro to override
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
+
 @hook TARGET_ASM_MERGEABLE_RODATA_PREFIX
 
 @hook TARGET_ASM_TM_CLONE_TABLE_SECTION
diff --git a/gcc/final.c b/gcc/final.c
index a3601964a8d..ba198182663 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -107,6 +107,10 @@ along with GCC; see the file COPYING3.  If not see
 #define JUMP_TABLES_IN_TEXT_SECTION 0
 #endif
 
+#ifndef JUMP_TABLES_IN_RELRO
+#define JUMP_TABLES_IN_RELRO 0
+#endif
+
 /* Bitflags used by final_scan_insn.  */
 #define SEEN_NOTE	1
 #define SEEN_EMITTED	2
@@ -2154,6 +2158,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 relco;
+
+  relco = JUMP_TABLES_IN_RELRO;
+
+  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);
+}
+
 /* The final scan for one insn, INSN.
    Args are same as in `final', except that INSN
    is the insn being scanned.
@@ -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));
 
 #ifdef ADDR_VEC_ALIGN
@@ -2571,7 +2592,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..9a463c42955 100644
--- a/gcc/output.h
+++ b/gcc/output.h
@@ -573,6 +573,7 @@ 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_relro_section (tree);
 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..9bd61b4e018 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -561,6 +561,18 @@ otherwise.",
  section *, (tree decl),
  default_function_rodata_section)
 
+/* Return the relro section associated with function DECL.  */
+DEFHOOK
+(function_relro_section,
+ "Return the relro 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}, \n\
+@code{.data.rel.ro.name} if function is in @code{.text.name}, \n\
+and the normal data relro section otherwise.",
+ section *, (tree decl),
+ default_function_relro_section)
+
 /* Nonnull if the target wants to override the default ".rodata" prefix
    for mergeable data sections.  */
 DEFHOOKPOD
diff --git a/gcc/varasm.c b/gcc/varasm.c
* 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.

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

Hi!

On Mon, Aug 17, 2020 at 10:28:31AM +0800, HAO CHEN GUI wrote:
> >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.

Yes, I did say the *existing* code seems sub-optimal, too :-)

> >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.

.data.rel.ro is just another kind of .rodata, one that *can* be
relocated.  So when we use it, fPIC or not doesn't matter.  Also, we can
just use the existing rodata functions for generating .data.rel.ro, and
it should simplify all code even.

> -@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

Should this take the 2-bit int "reloc" field like other functions,
instead of this bool?


Segher

Hi,

I revised the patch according to the advice. The attachment is the 
revised change log and diff file. Thanks a lot.

Now it first judges if it's absolute jump table or not. Then decide if 
the relocation is needed.

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

       reloc = (abs_jump_table && targetm.asm_out.reloc_rw_mask ());


       switch_to_section (targetm.asm_out.function_rodata_section
                                      (current_function_decl, reloc));

On 17/8/2020 下午 10:50, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Aug 17, 2020 at 10:28:31AM +0800, HAO CHEN GUI wrote:
>>> 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.
> Yes, I did say the *existing* code seems sub-optimal, too :-)
>
>>> 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.
> .data.rel.ro is just another kind of .rodata, one that *can* be
> relocated.  So when we use it, fPIC or not doesn't matter.  Also, we can
> just use the existing rodata functions for generating .data.rel.ro, and
> it should simplify all code even.
>
>> -@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
> Should this take the 2-bit int "reloc" field like other functions,
> instead of this bool?
>
>
> Segher
* final.c (final_scan_insn_1): Judge if a jump table is relative or
	absoluted, and if it needs to be relocated.
	* 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): Add the second argument
	to default_function_rodata_section. It indicates the section should be
	read-only or relocated read-only.
	(default_no_function_rodata_section): Add the second argument.
	(function_mergeable_rodata_prefix): 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..b22a7161052 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -9315,10 +9315,10 @@ 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 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))
     {
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..b19609f9c96 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{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..54947818320 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -2491,9 +2491,17 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 	  if (! JUMP_TABLES_IN_TEXT_SECTION)
 	    {
 	      int log_align;
+	      bool reloc;
+	      bool abs_jump_table;
+
+	      abs_jump_table = (!CASE_VECTOR_PC_RELATIVE
+		&& !targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 : 0;
+
+	      reloc = (abs_jump_table && targetm.asm_out.reloc_rw_mask ());
+
 
 	      switch_to_section (targetm.asm_out.function_rodata_section
-				 (current_function_decl));
+				 (current_function_decl, reloc));
 
 #ifdef ADDR_VEC_ALIGN
 	      log_align = ADDR_VEC_ALIGN (table);
@@ -2569,10 +2577,16 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 #if !(defined(ASM_OUTPUT_ADDR_VEC) || defined(ASM_OUTPUT_ADDR_DIFF_VEC))
 	    int vlen, idx;
 #endif
+	    bool reloc;
+	    bool abs_jump_table;
+
+	    abs_jump_table = (!CASE_VECTOR_PC_RELATIVE
+		&& !targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 : 0;
+	    reloc = (abs_jump_table && targetm.asm_out.reloc_rw_mask ());
 
 	    if (! JUMP_TABLES_IN_TEXT_SECTION)
 	      switch_to_section (targetm.asm_out.function_rodata_section
-				 (current_function_decl));
+				 (current_function_decl, reloc));
 	    else
 	      switch_to_section (current_function_section ());
 
diff --git a/gcc/output.h b/gcc/output.h
index eb253c50329..d64963b0ffa 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 reloc);
+extern section *default_no_function_rodata_section (tree, bool reloc);
 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..b2e82fcfedc 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -549,16 +549,17 @@ 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 relocated 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 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..19b47ea7c46 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -726,12 +726,26 @@ 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 relocated read-only data section
+   associated with function DECL.  */
 
 section *
-default_function_rodata_section (tree decl)
+default_function_rodata_section (tree decl, bool reloc)
 {
-  if (decl != NULL_TREE && DECL_SECTION_NAME (decl))
+  const char* sname;
+  unsigned int flags;
+
+  flags = 0;
+
+  if (reloc)
+    {
+      sname = ".data.rel.ro.local";
+      flags = (SECTION_WRITE | SECTION_RELRO);
+    }
+  else
+    sname = ".rodata";
+
+  if (decl && DECL_SECTION_NAME (decl))
     {
       const char *name = DECL_SECTION_NAME (decl);
 
@@ -744,12 +758,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 +781,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 (reloc)
+    return get_section (sname, flags, decl);
+  else
+    return readonly_data_section;
 }
 
 /* Return the read-only data section associated with function DECL
@@ -783,7 +800,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 reloc ATTRIBUTE_UNUSED)
 {
   return readonly_data_section;
 }
@@ -793,7 +811,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

Hi!

On Mon, Aug 24, 2020 at 03:53:44PM +0800, HAO CHEN GUI wrote:
>       abs_jump_table = (!CASE_VECTOR_PC_RELATIVE
>                                     && 
> !targetm.asm_out.generate_pic_addr_diff_vec ()) ? 1 : 0;

  x = y ? 1 : 0;

is the same as

  x = y;

if y is already only 0 or 1, and otherwise it is

  x = !!y;

You can also write this as an "if", that often is more readable.

> @@ -2491,9 +2491,17 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
>  	  if (! JUMP_TABLES_IN_TEXT_SECTION)
>  	    {
>  	      int log_align;
> +	      bool reloc;

> +extern section *default_function_rodata_section (tree, bool reloc);
> +extern section *default_no_function_rodata_section (tree, bool reloc);

"reloc" is an int elsewhere, and can have 4 values.  And you have to
interact with that code (mostly in varasm.c), so don't use the same name
meaning something else?


You need an RTL maintainer or global maintainer to review this...


Segher