This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/17640
- From: Ilya Tocar <tocarip dot intel at gmail dot com>
- To: Cary Coutant <ccoutant at google 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 13:46:26 +0300
- 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>
On 19 Feb 14:27, Cary Coutant wrote:
> >> Scan::local is called from gold::scan_relocs, so passing view into it
> >> will require changes in other targets, which I'd like to avoid.
> >> Also I'm not sure how to get view into instruction that uses
> >> corresponding relocation.
> >
> > Gold will fail some TLS testcases in ld, like the ones in
> >
> > https://sourceware.org/ml/binutils/2007-08/msg00353.html
> >
> > Target_i386::optimize_tls_reloc should check TLS instruction sequence,
> > similar to elf_i386_check_tls_transition. To do that, you need view. You
> > can add it and update other targets.
>
> We've tried very hard to avoid having to read the actual section data
> while scanning relocations; ideally, everything we need to know should
> be available from the relocation and the symbol it points to.
> (Checking the instruction stream, as we've seen recently, can be
> problematic, especially for x86.) In less-than-ideal situations like
> this one, if you really must access the contents of the section being
> relocated, you should be able to call
> obj->section_contents(data_shndx) to get a view for the data. While
> running a Scan_relocs task on an object file, the file is locked, and
> the view will be released when the file is unlocked at the end of that
> task. I'd prefer that you do this only when necessary in the
> target-specific code, rather than refactor the world.
>
> (By the way, when posting a patch, please edit out the diffs for
> generated files like Makefile.in.)
>
I've implemented this in target-specific code.
Version bellow both converts mov to lea and doesn't create useless GOT
entries.
Ok for trunk?
---
gold/i386.cc | 106 ++++++++++++++++++++++++++------------
gold/testsuite/Makefile.am | 15 ++++++
gold/testsuite/i386_mov_to_lea.s | 10 ++++
gold/testsuite/i386_mov_to_lea.sh | 29 +++++++++++
4 files changed, 128 insertions(+), 32 deletions(-)
create mode 100644 gold/testsuite/i386_mov_to_lea.s
create mode 100755 gold/testsuite/i386_mov_to_lea.sh
diff --git a/gold/i386.cc b/gold/i386.cc
index 24f4103..4a47b64 100644
--- a/gold/i386.cc
+++ b/gold/i386.cc
@@ -1835,8 +1835,24 @@ Target_i386::Scan::local(Symbol_table* symtab,
case elfcpp::R_386_GOT32:
{
- // The symbol requires a GOT entry.
+ section_size_type stype;
+ const unsigned char* view = object->section_contents(data_shndx,
+ &stype, true);
+
+ // We need GOT section.
Output_data_got<32, false>* got = target->got_section(symtab, layout);
+
+ // 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
+ && lsym.get_st_type() != elfcpp::STT_GNU_IFUNC)
+ break;
+
+ // Otherwise, the symbol requires a GOT entry.
unsigned int r_sym = elfcpp::elf_r_sym<32>(reloc.get_r_info());
// For a STT_GNU_IFUNC symbol we want the PLT offset. That
@@ -2229,8 +2245,26 @@ Target_i386::Scan::global(Symbol_table* symtab,
case elfcpp::R_386_GOT32:
{
+ section_size_type stype;
+ const unsigned char* view = object->section_contents(data_shndx,
+ &stype, true);
+
// The symbol requires a GOT entry.
Output_data_got<32, false>* got = target->got_section(symtab, layout);
+
+ // 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;
+
if (gsym->final_value_is_known())
{
// For a STT_GNU_IFUNC symbol we want the PLT address.
@@ -2732,35 +2766,6 @@ Target_i386::Relocate::relocate(const Relocate_info<32, false>* relinfo,
}
}
- // Get the GOT offset if needed.
- // The GOT pointer points to the end of the GOT section.
- // We need to subtract the size of the GOT section to get
- // the actual offset to use in the relocation.
- bool have_got_offset = false;
- unsigned int got_offset = 0;
- switch (r_type)
- {
- case elfcpp::R_386_GOT32:
- if (gsym != NULL)
- {
- gold_assert(gsym->has_got_offset(GOT_TYPE_STANDARD));
- got_offset = (gsym->got_offset(GOT_TYPE_STANDARD)
- - target->got_size());
- }
- else
- {
- unsigned int r_sym = elfcpp::elf_r_sym<32>(rel.get_r_info());
- gold_assert(object->local_has_got_offset(r_sym, GOT_TYPE_STANDARD));
- got_offset = (object->local_got_offset(r_sym, GOT_TYPE_STANDARD)
- - target->got_size());
- }
- have_got_offset = true;
- break;
-
- default:
- break;
- }
-
switch (r_type)
{
case elfcpp::R_386_NONE:
@@ -2809,8 +2814,45 @@ Target_i386::Relocate::relocate(const Relocate_info<32, false>* relinfo,
break;
case elfcpp::R_386_GOT32:
- gold_assert(have_got_offset);
- Relocate_functions<32, false>::rel32(view, got_offset);
+ // 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
+ && ((gsym == NULL && !psymval->is_ifunc_symbol())
+ || (gsym != NULL
+ && gsym->type() != elfcpp::STT_GNU_IFUNC
+ && !gsym->is_from_dynobj()
+ && !gsym->is_undefined())))
+ {
+ view[-2] = 0x8d;
+ elfcpp::Elf_types<32>::Elf_Addr value;
+ value = (psymval->value(object, 0)
+ - target->got_plt_section()->address());
+ Relocate_functions<32, false>::rel32(view, value);
+ }
+ else
+ {
+ // The GOT pointer points to the end of the GOT section.
+ // We need to subtract the size of the GOT section to get
+ // the actual offset to use in the relocation.
+ unsigned int got_offset = 0;
+ if (gsym != NULL)
+ {
+ gold_assert(gsym->has_got_offset(GOT_TYPE_STANDARD));
+ got_offset = (gsym->got_offset(GOT_TYPE_STANDARD)
+ - target->got_size());
+ }
+ else
+ {
+ unsigned int r_sym = elfcpp::elf_r_sym<32>(rel.get_r_info());
+ gold_assert(object->local_has_got_offset(r_sym, GOT_TYPE_STANDARD));
+ got_offset = (object->local_got_offset(r_sym, GOT_TYPE_STANDARD)
+ - target->got_size());
+ }
+ Relocate_functions<32, false>::rel32(view, got_offset);
+ }
break;
case elfcpp::R_386_GOTOFF:
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index f767c21..e826167 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -957,6 +957,21 @@ endif FN_PTRS_IN_SO_WITHOUT_PIC
endif TLS
+if DEFAULT_TARGET_I386
+
+check_SCRIPTS += i386_mov_to_lea.sh
+check_DATA += i386_mov_to_lea.stdout
+MOSTLYCLEANFILES += i386_mov_to_lea
+
+i386_mov_to_lea.o: i386_mov_to_lea.s
+ $(TEST_AS) --32 -o $@ $<
+i386_mov_to_lea: i386_mov_to_lea.o
+ ../ld-new -m elf_i386 -shared -o $@ $<
+i386_mov_to_lea.stdout: i386_mov_to_lea
+ $(TEST_OBJDUMP) -dw $< > $@
+
+endif DEFAULT_TARGET_I386
+
check_PROGRAMS += many_sections_test
many_sections_test_SOURCES = many_sections_test.cc
many_sections_test_DEPENDENCIES = gcctestdir/ld
diff --git a/gold/testsuite/i386_mov_to_lea.s b/gold/testsuite/i386_mov_to_lea.s
new file mode 100644
index 0000000..65f1bfd
--- /dev/null
+++ b/gold/testsuite/i386_mov_to_lea.s
@@ -0,0 +1,10 @@
+ .text
+ .type foo, @function
+foo:
+ ret
+ .size foo, .-foo
+ .globl bar
+ .type bar, @function
+bar:
+ movl foo@GOT(%ecx), %eax
+ .size bar, .-bar
diff --git a/gold/testsuite/i386_mov_to_lea.sh b/gold/testsuite/i386_mov_to_lea.sh
new file mode 100755
index 0000000..43306f0
--- /dev/null
+++ b/gold/testsuite/i386_mov_to_lea.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# i386_mov_to_lea.sh -- a test for mov2lea conversion.
+
+# Copyright (C) 2010-2015 Free Software Foundation, Inc.
+# Written by Tocar Ilya <ilya.tocar@intel.com>
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+set -e
+
+grep -q "lea" i386_mov_to_lea.stdout
+
+exit 0
--
1.8.3.1