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

Segher Boessenkool segher@kernel.crashing.org
Fri Aug 14 23:39:01 GMT 2020


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


More information about the Gcc-patches mailing list