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