This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/17640
- From: Cary Coutant <ccoutant at google dot com>
- To: Ilya Tocar <tocarip dot intel at gmail dot com>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, Ian Lance Taylor <iant at google dot com>, Binutils <binutils at sourceware dot org>
- Date: Thu, 26 Feb 2015 10:15:08 -0800
- Subject: Re: [PATCH] PR gold/17640
- Authentication-results: sourceware.org; auth=none
- References: <20150202134701 dot GA91020 at msticlxl7 dot ims dot intel dot com> <CAMe9rOpzSBoiykoFG+YuAK8QYsguqzZNrvj2sE7EhafUzOjJJw at mail dot gmail dot com> <20150218150011 dot GA40373 at msticlxl7 dot ims dot intel dot com> <CAMe9rOqGecgvSMs937a2b57mGjp-ZF5XuULQoAx3Tr9NG75GcA at mail dot gmail dot com> <20150219142707 dot GA507 at msticlxl7 dot ims dot intel dot com> <CAMe9rOp+6-mKKqE4h1jUvt-wBWVU0YK62yJPgrN0LVuU4JhxRw at mail dot gmail dot com> <CAHACq4poMBVYcg=nS01tPsLuNi=BdtL4gSTz5Q1-auGBX=zA-Q at mail dot gmail dot com> <20150226104626 dot GA16554 at msticlxl7 dot ims dot intel dot com>
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