Sorry for the slow review.
HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 513fc5fe295..6f5bf8d7d73 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 relocatable
ATTRIBUTE_UNUSED)
Now that we're C++, it's more idiomatic to leave off the parameter name:
mips_function_rodata_section (tree decl, bool)
Same for the rest of the patch.
@@ -2491,9 +2491,19 @@ final_scan_insn_1 (rtx_insn *insn, FILE
*file, int optimize_p ATTRIBUTE_UNUSED,
if (! JUMP_TABLES_IN_TEXT_SECTION)
{
int log_align;
+ bool relocatable;
+
+ relocatable = 0;
Very minor, but simpler as:
bool relocatable = false;
Same for the later hunk.
@@ -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\
Maybe add “; @var{relocatable} selects the latter over the former.”
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 relocatable),
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..91ab75aed06 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 relocatable)
{
- if (decl != NULL_TREE && DECL_SECTION_NAME (decl))
+ const char* sname;
+ unsigned int flags;
+
+ flags = 0;
+
+ if (relocatable)
+ {
+ 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);
}
}
Don't we need to handle the .gnu.linkonce.t. case too? I believe
the suffix there is “.d.rel.ro.local” (replacing “.t”)
My main concern is how this interacts with non-ELF targets.
It looks like AIX/XCOFF, Darwin and Cygwin already pick
default_no_function_rodata_section, so they should be fine.
But at the moment, all the fancy stuff in
default_function_rodata_section
is indirectly guarded by targetm_common.have_named_sections, with the
hook falling back to readonly_data_section if the function isn't in a
named section.
So I wonder if it would be safer to test
targetm_common.have_named_sections
in this condition:
+
+ /* Judge if it's a absolute jump table. Set relocatable for
+ absolute jump table if the target supports relocations. */
+
+ if (!CASE_VECTOR_PC_RELATIVE
+ && !targetm.asm_out.generate_pic_addr_diff_vec ())
+ relocatable = targetm.asm_out.reloc_rw_mask ();
switch_to_section
(targetm.asm_out.function_rodata_section
- (current_function_decl));
+ (current_function_decl, relocatable));
#ifdef ADDR_VEC_ALIGN
log_align = ADDR_VEC_ALIGN (table);
Similarly for the other case where the condition appears. I think the
condition is subtle enough that it's worth splitting out into a helper.
Thanks,
Richard
