On Sat, Jan 27, 2024 at 07:10:55AM -0800, H.J. Lu wrote:
> For function symbol reference in readonly data section, instead of putting
> it in .data.rel.ro or .rodata.cst section, call function_rodata_section to
> get the read-only or relocated read-only data section associated with the
> function DECL so that the COMDAT section will be used for a COMDAT function
> symbol.
I have to admit I still don't understand what the linker doesn't like on
what GCC emits and why references to the public symbols at the start of
comdat sections are ok in .text but not in .data.rel.ro but are in .data
or .rodata sections (or what the exact rules are, see also what we emit on
__attribute__((noinline, noipa)) inline void foo () {}
void bar () { foo (); } void (*p) () = foo; void (*const q) () = foo; void
(*const *r) () = &q;
).
I've always thought that the problematic references are when something
references non-public symbols in comdat sections, especially not at their
start, because if linker selects some comdat section(s) from some other
TU, there is no guarantee e.g. the code is identical (just in valid program
should behave the same) and if such reference comes from other comdat that
is kept or from non-comdat sections, the question is what should be
referenced.
But in this case, I believe we are referencing the function at the start of
a code comdat section.
Now, in my limited understanding what the patch does is totally wrong
for multiple reasons. On the first testcase it changes
- .section .data.rel.ro.local,"aw"
+ .section
.data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxxEEEE10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE,"awG",@progbits,_ZN26vtkStaticCellLinksTemplateIxE18ThreadedBuildLinksExxP12vtkCellArray,comdat
.align 8
.LC0:
.quad
_ZN4blah17_Function_handlerIFvvENS_5_BindIFPFvPvxxxEPN3vtk6detail3smp27vtkSMPTools_FunctorInternalIN12_GLOBAL__N_19CountUsesIxEELb0EEExxxEEEE10_M_managerERNS_9_Any_dataERKSI_NS_18_Manager_operationE
Now, I believe such a .data.rel.ro.local.* section is normally
used for .data.rel.ro.local constants from the referenced function,
if we have some relocatable constant needed in that function we
emit those there.
If linker picks up the comdat from current TU, it will be all fine,
sure, but if it picks up the comdat from another TU, the
.data.rel.ro.local._ZN4blah17_Function_handlerIFvvENS_5* section
there might not be present or might contain some unrelated stuff.
Given the handling of (const (plus (symbol_ref) (const_int)), we
also don't know whether the section holds a reference to the start,
or to some other offset of it, how many etc.
And, we refenre a non-public symbol (.LC0) from non-comdat section
to a comdat section.
If I'm wrong on this, please try to explain.
Jakub
