This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/17729: [gold] gold failed to build x32 libgo
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Cary Coutant <ccoutant at google dot com>, Binutils <binutils at sourceware dot org>
- Date: Sat, 21 Feb 2015 17:51:21 -0800
- Subject: Re: [PATCH] PR gold/17729: [gold] gold failed to build x32 libgo
- Authentication-results: sourceware.org; auth=none
- References: <20141218191317 dot GA367 at intel dot com> <CAHACq4orj_8aV3t7GsH8=rTZQWegK8XuS3=A9feyic0vfVDOJA at mail dot gmail dot com> <CAMe9rOqWA9t20eOWjerAmLN-WbBdhiBvn7iC6Y=caGouKdpigQ at mail dot gmail dot com>
On Tue, Jan 06, 2015 at 03:40:45PM -0800, H.J. Lu wrote:
> On Tue, Jan 6, 2015 at 3:18 PM, Cary Coutant <ccoutant@google.com> wrote:
> >> + if (size == 32)
> >> + {
> >> + // For X32
> >> + // cmp %fs:NN,%esp
> >> + cmp_insn = "\x64\x3b\x24\x25";
> >> + cmp_insn_len = 4;
> >> + // lea NN(%rsp),%r10d
> >> + // lea NN(%rsp),%r11d
> >> + lea_r10_insn = "\x44\x8d\x94\x24";
> >> + lea_r11_insn = "\x44\x8d\x9c\x24";
> >> + nop_insn_len = 7;
> >> + }
> >> ...
> >
> > The rest of this patch is OK, but in do_calls_non_split(), I'd prefer
> > something like this (note also the depedency between nop_size and the
> > test against fnsize for the cmp instruction):
> >
> > --- a/gold/x86_64.cc
> > +++ b/gold/x86_64.cc
> > @@ -4457,6 +4457,14 @@ Target_x86_64<size>::do_ehframe_datarel_base() const
> > // code. We have to change the function so that it always ensures
> > // that it has enough stack space to run some random function.
> >
> > +static const unsigned char cmp_insn_32[] = { 0x64, 0x3b, 0x24, 0x25 };
> > +static const unsigned char lea_r10_insn_32[] = { 0x44, 0x8d, 0x94, 0x24 };
> > +static const unsigned char lea_r11_insn_32[] = { 0x44, 0x8d, 0x9c, 0x24 };
> > +
> > +static const unsigned char cmp_insn_64[] = { 0x64, 0x48, 0x3b, 0x24, 0x25 };
> > +static const unsigned char lea_r10_insn_64[] = { 0x4c, 0x8d, 0x94, 0x24 };
> > +static const unsigned char lea_r11_insn_64[] = { 0x4c, 0x8d, 0x9c, 0x24 };
> > +
> > template<int size>
> > void
> > Target_x86_64<size>::do_calls_non_split(Relobj* object, unsigned int shndx,
> > @@ -4467,25 +4475,40 @@
> > Target_x86_64<size>::do_calls_non_split(Relobj* object, unsigned int
> > shndx,
> > std::string* from,
> > std::string* to) const
> > {
> > + const char* const cmp_insn = reinterpret_cast<const char*>
> > + (size == 32 ? cmp_insn_32 : cmp_insn_64);
> > + const char* const lea_r10_insn = reinterpret_cast<const char*>
> > + (size == 32 ? lea_r10_insn_32 : lea_r10_insn_64);
> > + const char* const lea_r11_insn = reinterpret_cast<const char*>
> > + (size == 32 ? lea_r11_insn_32 : lea_r11_insn_64);
> > +
> > + const size_t cmp_insn_len =
> > + (size == 32 ? sizeof(cmp_insn_32) : sizeof(cmp_insn_64));
> > + const size_t lea_r10_insn_len =
> > + (size == 32 ? sizeof(lea_r10_insn_32) : sizeof(lea_r10_insn_64));
> > + const size_t lea_r11_insn_len =
> > + (size == 32 ? sizeof(lea_r11_insn_32) : sizeof(lea_r11_insn_64));
> > + const size_t nop_len = (size == 32 ? 7 : 8);
> > +
> > // The function starts with a comparison of the stack pointer and a
> > // field in the TCB. This is followed by a jump.
> >
> > // cmp %fs:NN,%rsp
> > - if (this->match_view(view, view_size, fnoffset, "\x64\x48\x3b\x24\x25", 5)
> > - && fnsize > 9)
> > + if (this->match_view(view, view_size, fnoffset, cmp_insn, cmp_insn_len)
> > + && fnsize > nop_len + 1)
> > {
> > // We will call __morestack if the carry flag is set after this
> > // comparison. We turn the comparison into an stc instruction
> > // and some nops.
> > view[fnoffset] = '\xf9';
> > - this->set_view_to_nop(view, view_size, fnoffset + 1, 8);
> > + this->set_view_to_nop(view, view_size, fnoffset + 1, nop_len);
> > }
> > // lea NN(%rsp),%r10
> > // lea NN(%rsp),%r11
> > else if ((this->match_view(view, view_size, fnoffset,
> > - "\x4c\x8d\x94\x24", 4)
> > + lea_r10_insn, lea_r10_insn_len)
> > || this->match_view(view, view_size, fnoffset,
> > - "\x4c\x8d\x9c\x24", 4))
> > + lea_r11_insn, lea_r11_insn_len))
> > && fnsize > 8)
> > {
> > // This is loading an offset from the stack pointer for a
>
> This is what I checked in. I'd like to backport it to 2.25 branch.
> OK for 2.25 branch?
>
> Thanks.
>
> --
> H.J.
> From 4fc1b9d43cbce7571264a0011c87258b78252750 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 18 Dec 2014 11:09:28 -0800
> Subject: [PATCH] Handle stack split for x32
>
> X32 uses cmp %fs:NN,%esp, lea NN(%rsp),%r10d, lea NN(%rsp),%r11d,
> instead of cmp %fs:NN,%rsp, lea NN(%rsp),%r10, lea NN(%rsp),%r11.
> This patch handles it.
>
> PR gold/17729
> * configure.ac (DEFAULT_TARGET_X86_64): Don't set for x32.
> (DEFAULT_TARGET_X32): Set for x32.
> * x86_64.cc (cmp_insn_32): New.
> (lea_r10_insn_32): Likewise.
> (lea_r11_insn_32): Likewise.
> (cmp_insn_64): Likewise.
> (lea_r10_insn_64): Likewise.
> (lea_r11_insn_64): Likewise.
> (Target_x86_64<size>::do_calls_non_split): Handle x32.
> * testsuite/Makefile.am (check_SCRIPTS): Add split_x32.sh.
> (check_DATA): Add split_x32 files.
> (split_x32_[1234n].o): New targets.
> (split_x32_[124]): New targets.
> (split_x32_[1234r].stdout): New targets.
> * testsuite/split_x32.sh: New file.
> * testsuite/split_x32_1.s: Likewise.
> * testsuite/split_x32_2.s: Likewise.
> * testsuite/split_x32_3.s: Likewise.
> * testsuite/split_x32_4.s: Likewise.
> * testsuite/split_x32_n.s: Likewise.
> * configure: Regenerated.
> * testsuite/Makefile.in: Likewise.
> ---
> gold/ChangeLog | 27 ++++++++++++++++++++++
> gold/configure | 28 +++++++++++++++++++++-
> gold/configure.ac | 15 +++++++++++-
> gold/testsuite/Makefile.am | 37 +++++++++++++++++++++++++++++
> gold/testsuite/Makefile.in | 55 ++++++++++++++++++++++++++++++++++++--------
> gold/testsuite/split_x32.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++
> gold/testsuite/split_x32_1.s | 33 ++++++++++++++++++++++++++
> gold/testsuite/split_x32_2.s | 33 ++++++++++++++++++++++++++
> gold/testsuite/split_x32_3.s | 22 ++++++++++++++++++
> gold/testsuite/split_x32_4.s | 23 ++++++++++++++++++
> gold/testsuite/split_x32_n.s | 12 ++++++++++
> gold/x86_64.cc | 33 ++++++++++++++++++++++----
> 12 files changed, 357 insertions(+), 16 deletions(-)
> create mode 100755 gold/testsuite/split_x32.sh
> create mode 100644 gold/testsuite/split_x32_1.s
> create mode 100644 gold/testsuite/split_x32_2.s
> create mode 100644 gold/testsuite/split_x32_3.s
> create mode 100644 gold/testsuite/split_x32_4.s
> create mode 100644 gold/testsuite/split_x32_n.s
>
> diff --git a/gold/ChangeLog b/gold/ChangeLog
> index af56066..93529fe 100644
> --- a/gold/ChangeLog
> +++ b/gold/ChangeLog
> @@ -1,4 +1,31 @@
> 2015-01-06 H.J. Lu <hongjiu.lu@intel.com>
> + Cary Coutant <ccoutant@google.com>
> +
> + PR gold/17729
> + * configure.ac (DEFAULT_TARGET_X86_64): Don't set for x32.
> + (DEFAULT_TARGET_X32): Set for x32.
> + * x86_64.cc (cmp_insn_32): New.
> + (lea_r10_insn_32): Likewise.
> + (lea_r11_insn_32): Likewise.
> + (cmp_insn_64): Likewise.
> + (lea_r10_insn_64): Likewise.
> + (lea_r11_insn_64): Likewise.
> + (Target_x86_64<size>::do_calls_non_split): Handle x32.
> + * testsuite/Makefile.am (check_SCRIPTS): Add split_x32.sh.
> + (check_DATA): Add split_x32 files.
> + (split_x32_[1234n].o): New targets.
> + (split_x32_[124]): New targets.
> + (split_x32_[1234r].stdout): New targets.
> + * testsuite/split_x32.sh: New file.
> + * testsuite/split_x32_1.s: Likewise.
> + * testsuite/split_x32_2.s: Likewise.
> + * testsuite/split_x32_3.s: Likewise.
> + * testsuite/split_x32_4.s: Likewise.
> + * testsuite/split_x32_n.s: Likewise.
> + * configure: Regenerated.
> + * testsuite/Makefile.in: Likewise.
> +
> diff --git a/gold/configure.ac b/gold/configure.ac
> index b574153..a2e5895 100644
> --- a/gold/configure.ac
> +++ b/gold/configure.ac
> @@ -204,7 +204,20 @@ for targ in $target $canon_targets; do
> AM_CONDITIONAL(DEFAULT_TARGET_I386, test "$targ_obj" = "i386")
> AM_CONDITIONAL(DEFAULT_TARGET_POWERPC, test "$targ_obj" = "powerpc")
> AM_CONDITIONAL(DEFAULT_TARGET_SPARC, test "$targ_obj" = "sparc")
> - AM_CONDITIONAL(DEFAULT_TARGET_X86_64, test "$targ_obj" = "x86_64")
> + target_x86_64=no
> + target_x32=no
> + if test "$targ_obj" = "x86_64"; then
> + case "$target" in
> + x86_64*-linux-gnux32)
> + target_x32=yes
> + ;;
> + *)
> + target_x86_64=yes
> + ;;
> + esac
> + fi
> + AM_CONDITIONAL(DEFAULT_TARGET_X86_64, test "$target_x86_64" = "yes")
> + AM_CONDITIONAL(DEFAULT_TARGET_X32, test "$target_x32" = "yes")
> AM_CONDITIONAL(DEFAULT_TARGET_TILEGX, test "$targ_obj" = "tilegx")
> AM_CONDITIONAL(DEFAULT_TARGET_MIPS, test "$targ_obj" = "mips")
> DEFAULT_TARGET=${targ_obj}
I missed setting GOLD_DEFAULT_SIZE to 32 when setting default target
to x32. This patch fixes it. OK for trunk and 2.25 branch?
Thanks.
H.J.
---
diff --git a/gold/ChangeLog b/gold/ChangeLog
index 71a5ab2..b73da0a 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,8 @@
+2015-02-21 H.J. Lu <hongjiu.lu@intel.com>
+
+ * configure.ac (default_size): Set to 32 for x32.
+ * configure: Regenerated.
+
2015-02-18 Alan Modra <amodra@gmail.com>
PR 17954
diff --git a/gold/configure b/gold/configure
index 23e4735..3427074 100755
--- a/gold/configure
+++ b/gold/configure
@@ -3483,6 +3483,7 @@ fi
case "$target" in
x86_64*-linux-gnux32)
target_x32=yes
+ default_size=32
;;
*)
target_x86_64=yes
diff --git a/gold/configure.ac b/gold/configure.ac
index a2e5895..c08d0a2 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -210,6 +210,7 @@ for targ in $target $canon_targets; do
case "$target" in
x86_64*-linux-gnux32)
target_x32=yes
+ default_size=32
;;
*)
target_x86_64=yes