[PATCH v3] x86: Properly handle relocation against local ABS symbol
H.J. Lu
hjl.tools@gmail.com
Fri May 16 11:33:22 GMT 2025
On Fri, May 16, 2025 at 9:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, May 15, 2025 at 2:22 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 15.05.2025 04:00, H.J. Lu wrote:
> > > With PIC on x86-64,
> > >
> > > movq symbol@GOTPCREL(%rip), %rax
> > >
> > > is used to get the symbol address via GOT. If the symbol turns out to
> > > be a hidden symbol, linker normally converts it into
> > >
> > > leaq symbol(%rip), %rax
> > >
> > > But if the symbol is an ABS symbol, RAX won't have the correct value at
> > > run-time and it will have the symbol value + load address. If the symbol
> > > is declared as hidden, compiler will generate
> > >
> > > leaq symbol(%rip), %rax
> > >
> > > to get the symbol address. If the hidden symbol is an ABS symbol, RAX
> > > will have the same incorrect value at run-time. On i386,
> > >
> > > movl symbol@GOT(%reg1), %reg2
> > >
> > > is used to get the symbol address via GOT. If the symbol turns out to
> > > be a hidden symbol, linker normally converts it into
> > >
> > > leal symbol@GOTOFF(%reg1), %reg2
> > >
> > > If the symbol is declared as hidden, compiler will generate
> > >
> > > leal symbol@GOTOFF(%reg1), %reg2
> > >
> > > In both cases, if the hidden symbol is an ABS symbol, REG2 will have the
> > > same incorrect value at run-time. This patch changes the x86-64 linker
> > > to convert
> > >
> > > movq symbol@GOTPCREL(%rip), %rax
> > > and
> > > leaq symbol(%rip), %rax
> > > to
> > > movq $symbol, %rax
> > >
> > > and changes the i386 linker to convert
> > >
> > > movl symbol@GOT(%reg1), %reg2
> > > and
> > > leal symbol@GOTOFF(%reg1), %reg2
> > > to
> > > movl $symbol, %reg2
> > >
> > > for hidden ABS symbols.
> > >
> > > bfd/
> > >
> > > PR ld/32443
> > > * elf32-i386.c (elf_i386_convert_load_reloc): Skip relocation
> > > against linker-script hidden absolute symbol in PIC.
> > > (elf_i386_finish_dynamic_symbol): Don't generate dynamic
> > > relocation against inker-script hidden absolute symbol.
> >
> > You're again checking opcode bytes without considering prefixes, in
> > particular (but not limited to) VEX/XOP/EVEX ones. I understand this is
> > a pre-existing issue, but I'd prefer if the problem wasn't widened.
>
> I think my patch is acceptable as it fixes the wrong run-time result.
>
> > Also why is it that this is done only for STV_HIDDEN, but not also for
> > STV_PROTECTED?
>
> I changed HIDDEN_ABS_SYMBOL_P to LOCAL_ABS_SYMBOL_P.
>
> > Further, why is the conversion to MOV only done for PIC? The MOV
> > (immediate) is more efficient even outside of PIC/PIE, isn't it?
>
> Fixed:
>
> /* Return TRUE if the symbol described by a linker hash entry H is a
> local symbol which is going to be absolute. */
> #define LOCAL_ABS_SYMBOL_P(INFO, H) \
> (bfd_is_abs_symbol (&(H)->root) \
> && (bfd_link_executable (INFO) \
> || ELF_ST_VISIBILITY ((H)->other) == STV_HIDDEN \
> || ELF_ST_VISIBILITY ((H)->other) == STV_PROTECTED))
>
> with tests.
>
> > > * elf64-x86-64.c (elf_x86_64_convert_load_reloc): Skip
> > > relocation against linker-script hidden absolute symbol in PIC.
> > > (elf_x86_64_finish_dynamic_symbol): Don't generate dynamic
> > > relocation against inker-script hidden absolute symbol.
> >
> > Here afaics you only check the opcode byte, but not ModR/M (to limit
> > to the %rip-relative form only).
>
> R_X86_64_PC32 is generated only for foo(%rip).
>
> > You convert "if (opcode != 0x8b)" to "else if (opcode != 0x8b)" in
> > one of the rearrangements - is that, strictly speaking, correct? The
> > original code included the 0xff case, when that's now excluded. If it's
> > correct, the earlier
>
> R_X86_64_PC32 is new and only
>
> leaq symbol(%rip), %rax
>
> is allowed.
>
> > + else
> > + {
> > + if (opcode == 0xff)
> >
> > would want to become just
> >
> > + else if (opcode == 0xff)
> >
> > limiting code churn quite a bit.
> >
> > Like you have done in the 32-bit code, can you indent the lea_to_mov
> > label some, please?
>
> Fixed.
>
> > > * elfxx-x86.c (elf_x86_allocate_dynrelocs): Don't allocate
> > > dynamic relocation against inker-script hidden absolute symbol.
> > > * elfxx-x86.h (HIDDEN_ABS_SYMBOL_P): New.
> >
> > Why is it that HIDDEN_ABS_SYMBOL_P() doesn't use ABS_SYMBOL_P()? IOW why
> > is ->root.ldscript_def relevant to check for the latter, but not for the
> > new predicate?
>
> ABS_SYMBOL_P excludes linker defined symbols because of
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=25754
> https://sourceware.org/bugzilla/show_bug.cgi?id=25749
It turns out that we can drop linker defined symbol check in
ABS_SYMBOL_P with my new LOCAL_ABS_SYMBOL_P.
Here is the v3 patch with
#define ABS_SYMBOL_P(H) bfd_is_abs_symbol (&(H)->root)
> Here is the v2 patch. I also added the overflow check
>
> if (r == bfd_reloc_overflow)
> {
> if ((r_type == (rex_w && ABI_64_P (link_info->output_bfd)
> ? R_X86_64_32S : R_X86_64_32))
> && reloc_pc32
> && LOCAL_ABS_SYMBOL_P (link_info, h))
> {
> /* Since R_X86_64_PC32S/R_X86_64_32 relocation, which
> is converted from R_X86_64_PC32, against a local
> ABS symbol in PIC overflows, the original relocation
> will also overflow. */
> howto = elf_x86_64_rtype_to_howto (abfd,
> R_X86_64_PC32);
> link_info->callbacks->reloc_overflow
> (link_info, &h->root, h->root.root.string,
> howto->name, (bfd_vma) 0, abfd, input_section,
> irel->r_offset);
> return false;
> }
> return true;
> }
>
> with tests.
>
>
> --
> H.J.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v3-0001-x86-Properly-handle-relocation-against-local-ABS-.patch
Type: text/x-patch
Size: 41022 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20250516/9b05747e/attachment-0001.bin>
More information about the Binutils
mailing list