On Thu, Apr 28, 2022 at 01:03:26PM +0200, Ilya Leoshkevich wrote:
> This is determined by default_elf_select_rtx_section (). If we don't
> want to mix non-reloc and reloc constants, we need to define a special
> section there.
>
> It seems to me, however, that this all would be made purely for the
> sake of .LASANPC, which is quite special: it's local, but at the same
> time it might need to be comdat. I don't think anything like this can
> appear from compiling C/C++ code.
>
> Therefore I wonder if we could just drop it altogether like this?
>
> @@ -1928,22 +1919,7 @@ asan_emit_stack_protection (rtx base, rtx pbase,
> unsigned int alignb,
> ...
> - emit_move_insn (mem, expand_normal (build_fold_addr_expr (decl)));
> + emit_move_insn (mem, expand_normal (build_fold_addr_expr
> (current_function_decl)));
> ...
>
> That's what LLVM is already doing. This will also solve the alignment
> problem I referred to earlier.
LLVM is doing a wrong thing here. The global symbol can be overridden by
a symbol in another shared library, that is definitely not what we want,
because the ASAN record is for the particular implementation, not the other
one which could be quite different.
I think the right fix would be:
--- gcc/varasm.cc.jj 2022-03-07 15:00:17.255592497 +0100
+++ gcc/varasm.cc 2022-04-28 13:22:44.463147066 +0200
@@ -7326,6 +7326,9 @@ default_elf_select_rtx_section (machine_
return get_named_section (NULL, ".data.rel.ro", 3);
}
+ if (reloc)
+ return readonly_data_section;
+
return mergeable_constant_section (mode, align, 0);
}
which matches what we do in categorize_decl_for_section:
else if (reloc & targetm.asm_out.reloc_rw_mask ())
ret = reloc == 1 ? SECCAT_DATA_REL_RO_LOCAL : SECCAT_DATA_REL_RO;
else if (reloc || flag_merge_constants < 2
...
/* C and C++ don't allow different variables to share the same
location. -fmerge-all-constants allows even that (at the
expense of not conforming). */
ret = SECCAT_RODATA;
else if (DECL_INITIAL (decl)
&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
ret = SECCAT_RODATA_MERGE_STR_INIT;
else
ret = SECCAT_RODATA_MERGE_CONST;
i.e. if reloc is true, it goes into .data.rel.ro* for -fpic and .rodata
for non-pic, and mergeable sections are only used if there are no
relocations.
Anyway, I'd feel much safer to change it only in GCC 13, at least initially.
Or are some linkers (say lld or mold, fod ld.bfd I'm pretty sure it doesn't,
for gold no idea but unlikely) able to merge even constants with
relocations against them?
Jakub
