This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR gold/17640


Thanks for working on this!

Please write a ChangeLog entry.

> +       // If the relocation symbol isn't IFUNC,
> +       // and is local, then we will convert
> +       // mov foo@GOT(%reg), %reg
> +       // to
> +       // lea foo@GOTOFF(%reg), %reg
> +       // in Relocate::relocate
> +       if (view[reloc.get_r_offset() - 2] == 0x8b

You also need to check that reloc.get_r_offset() >= 2. If that's
false, or if the symbol is an IFUNC symbol, you could avoid getting
the section contents.

> +       // If we convert this from
> +       // mov foo@GOT(%reg), %reg
> +       // to
> +       // lea foo@GOTOFF(%reg), %reg
> +       // in Relocate::relocate, then there is nothing to do here.
> +       // Avoid converting _DYNAMIC, because it's address may be used.
> +       if (view[reloc.get_r_offset() - 2] == 0x8b
> +           && gsym->type() != elfcpp::STT_GNU_IFUNC
> +           && !gsym->is_undefined()
> +           && !gsym->is_from_dynobj()
> +           && strcmp(gsym->name(), "_DYNAMIC"))
> +         break;

Same here.

s/it's/its/

Could you explain more about the exception for _DYNAMIC? If its
address is used by some other relocation, won't we end up creating the
GOT entry anyway when we process that relocation? And if it's not,
isn't it OK to omit the GOT entry?

> +      // If the relocation symbol isn't IFUNC,
> +      // and is local, then we convert
> +      // mov foo@GOT(%reg), %reg
> +      // to
> +      // lea foo@GOTOFF(%reg), %reg
> +      if (view[-2] == 0x8b

Again, you need to check that r_offset is in range. See calls to
tls::check_tls() later in the source file.

> +         && ((gsym == NULL && !psymval->is_ifunc_symbol())
> +             || (gsym != NULL
> +                 && gsym->type() != elfcpp::STT_GNU_IFUNC
> +                 && !gsym->is_from_dynobj()
> +                 && !gsym->is_undefined())))

What about _DYNAMIC? You need to make sure you make the same decision
here that you made in Scan::local or Scan::global. It would probably
be a good idea to factor out the condition (at least the part where
it's a global symbol).

> +set -e
> +
> +grep -q "lea" i386_mov_to_lea.stdout

I'd worry here that "lea" might somehow appear in the objdump output
in some irrelevant place. Could you make your regex a little bit more
specific?

-cary


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]