[2/2] Add an x86 IFUNC testcase for [BZ #20019]
Commit Message
On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 5 Oct 2016, H.J. Lu wrote:
>
>> I can try __builtin_memcpy, instread of __builtin_memmove. There are 2
I changed it to use __builtin_memset.
>> acceptable results. One is ld.so issues an error and the other is program runs.
>> On x86, ld.so issues an error. I don't know what should happen on others.
>
> You could make the test pass on either of those results (while failing if
> ld.so crashes).
>
I moved the test to elf. It passes if the test runs or ld.so issues an
error. Please try it on arm, powerpc and s390.
Comments
On 05/10/2016 15:16, H.J. Lu wrote:
> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>
>>> I can try __builtin_memcpy, instread of __builtin_memmove. There are 2
>
> I changed it to use __builtin_memset.
>
>>> acceptable results. One is ld.so issues an error and the other is program runs.
>>> On x86, ld.so issues an error. I don't know what should happen on others.
>>
>> You could make the test pass on either of those results (while failing if
>> ld.so crashes).
>>
>
> I moved the test to elf. It passes if the test runs or ld.so issues an
> error. Please try it on arm, powerpc and s390.
>
It shows no issue neither on arm (gcc 4.8.4, binutils 2.24) or powerpc64le
(gcc 5.4.0, binutils 2.26.1).
On 10/05/2016 02:16 PM, H.J. Lu wrote:
> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>
>>> I can try __builtin_memcpy, instread of __builtin_memmove. There are 2
>
> I changed it to use __builtin_memset.
>
>>> acceptable results. One is ld.so issues an error and the other is program runs.
>>> On x86, ld.so issues an error. I don't know what should happen on others.
>>
>> You could make the test pass on either of those results (while failing if
>> ld.so crashes).
>>
>
> I moved the test to elf. It passes if the test runs or ld.so issues an
> error. Please try it on arm, powerpc and s390.
This is the wrong way to test this.
The point of this test is this:
- Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
resolver is called, because DSO B's resolver might need global data to make
the IFUNC decision e.g. GOT setup.
The invariant we want to hold true for IFUNC is that to call the resolver
function you must have relocated the DSO which contains the resolver. This _should_
have been done by a symbol reocation dependency analysis, but that isn't working
correctly IMO or needs deeper analysis in the dynamic loader.
The solution we want in place today is to issue some kind of diagnostic until we
fix the real problem.
The test should look like this:
- DSO A with an unversioned symbol reference to 'foo'.
- DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
resolver function which references global data from DSO C to decide which of
two functions to return.
- DSO C with global data set to a value.
The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
relocated first, then B, such that B's GOT is setup to access C's global data.
When handling the reference to 'foo' in DSO A we should on x86_64 and i686
get the error about needing to relink DSO A so it depends on DSO B, to form
the initialization order of C->B->A.
I expect this test case will now crash the other arches, rather than just
avoiding the crash by relying on internal libc.so details about which ifuncs
you're using.
This is one step towards a better definition of IFUNC semantics, which need to
be more clearly defined (something I wish I had time to define and fix so
more projects could use them).
On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>
>>>> I can try __builtin_memcpy, instread of __builtin_memmove. There are 2
>>
>> I changed it to use __builtin_memset.
>>
>>>> acceptable results. One is ld.so issues an error and the other is program runs.
>>>> On x86, ld.so issues an error. I don't know what should happen on others.
>>>
>>> You could make the test pass on either of those results (while failing if
>>> ld.so crashes).
>>>
>>
>> I moved the test to elf. It passes if the test runs or ld.so issues an
>> error. Please try it on arm, powerpc and s390.
>
> This is the wrong way to test this.
>
> The point of this test is this:
>
> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
> on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
> DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
> resolver is called, because DSO B's resolver might need global data to make
> the IFUNC decision e.g. GOT setup.
>
> The invariant we want to hold true for IFUNC is that to call the resolver
> function you must have relocated the DSO which contains the resolver. This _should_
> have been done by a symbol reocation dependency analysis, but that isn't working
> correctly IMO or needs deeper analysis in the dynamic loader.
>
> The solution we want in place today is to issue some kind of diagnostic until we
> fix the real problem.
>
> The test should look like this:
>
> - DSO A with an unversioned symbol reference to 'foo'.
> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
> resolver function which references global data from DSO C to decide which of
> two functions to return.
> - DSO C with global data set to a value.
>
> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
> relocated first, then B, such that B's GOT is setup to access C's global data.
>
> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
> get the error about needing to relink DSO A so it depends on DSO B, to form
> the initialization order of C->B->A.
>
> I expect this test case will now crash the other arches, rather than just
> avoiding the crash by relying on internal libc.so details about which ifuncs
> you're using.
>
> This is one step towards a better definition of IFUNC semantics, which need to
> be more clearly defined (something I wish I had time to define and fix so
> more projects could use them).
IFUNC resolver can fail for various reasons. My goal is to make sure
that IFUNC inside of glibc works correctly or an error message is given
when glibc isn't used properly. In case of x86, CPU feature info is
retrieved and stored in ld.so very early at startup, which is used by IFUNC
and only accessible in libc.so and libm.so after they have been relocated.
My change in x86 ld.so checks it and my test verifies the check. My fix
won't cover other possible IFUNC failures.
From 45fa40391e8e46ccfe3fde6120671e638f54ed45 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 4 Oct 2016 11:29:55 -0700
Subject: [PATCH] Add an IFUNC testcase for [BZ #20019]
If an IFUNC function is called before the providing shared library is
unrelocated, ld.so may segfault. Add a testcase to verify that the
program won't segfault. It should either run or ld.so issues a
diagnostic.
[BZ #20019]
* elf/Makefile (tests): Add tst-ifunc1.
(tests-special): Add $(objpfx)tst-ifunc1.out.
($(objpfx)tst-ifunc1.out): New rule.
(extra-test-objs): Add tst-ifunc1 objects.
($(objpfx)tst-ifunc1): New.
($(objpfx)tst-ifuncmod1a.so): New rule.
($(objpfx)tst-ifuncmod1b.so): LIkewise.
($(objpfx)tst-ifuncmod1a.os): LIkewise.
($(objpfx)tst-ifuncmod1b.os): LIkewise.
* elf/tst-ifunc1.c: New file.
* elf/tst-ifunc1.sh: LIkewise.
* elf/tst-ifuncmod1a.c: LIkewise.
* elf/tst-ifuncmod1b.c: LIkewise.
---
elf/Makefile | 26 ++++++++++++++++++++++++++
elf/tst-ifunc1.c | 26 ++++++++++++++++++++++++++
elf/tst-ifunc1.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
elf/tst-ifuncmod1a.c | 26 ++++++++++++++++++++++++++
elf/tst-ifuncmod1b.c | 23 +++++++++++++++++++++++
5 files changed, 144 insertions(+)
create mode 100644 elf/tst-ifunc1.c
create mode 100755 elf/tst-ifunc1.sh
create mode 100644 elf/tst-ifuncmod1a.c
create mode 100644 elf/tst-ifuncmod1b.c
@@ -329,6 +329,11 @@ $(objpfx)tst-_dl_addr_inside_object: $(objpfx)dl-addr-obj.os
CFLAGS-tst-_dl_addr_inside_object.c += $(PIE-ccflag)
endif
+ifeq ($(run-built-tests)$(build-shared),yesyes)
+tests += tst-ifunc1
+tests-special += $(objpfx)tst-ifunc1.out
+endif
+
include ../Rules
ifeq (yes,$(build-shared))
@@ -965,6 +970,27 @@ CFLAGS-tst-pie2.c += $(pie-ccflag)
$(objpfx)tst-pie1: $(objpfx)tst-piemod1.so
+$(objpfx)tst-ifunc1.out: tst-ifunc1.sh $(objpfx)tst-ifunc1
+ $(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \
+ '$(test-wrapper-env)' '$(run-program-env)'; \
+ $(evaluate-test)
+
+# Since tst-ifuncmod1a.so and tst-ifuncmod1b.so aren't linked against
+# libc.so, use special rules to build them.
+extra-test-objs += tst-ifunc1 tst-ifuncmod1a.os tst-ifuncmod1b.os \
+ tst-ifuncmod1a.os tst-ifuncmod1b.os
+
+$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so
+$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1a.os \
+ $(objpfx)tst-ifuncmod1b.so
+ $(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1b.os
+ $(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1a.os: tst-ifuncmod1a.c
+ $(CC) -fPIC -c -o $@ $<
+$(objpfx)tst-ifuncmod1b.os: tst-ifuncmod1b.c
+ $(CC) -fPIC -c -o $@ $<
+
ifeq (yes,$(build-shared))
all-built-dso := $(common-objpfx)elf/ld.so $(common-objpfx)libc.so \
$(filter-out $(common-objpfx)linkobj/libc.so, \
new file mode 100644
@@ -0,0 +1,26 @@
+/* Test case for IFUNC.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library 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
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+extern void foo (void);
+
+int
+main (void)
+{
+ foo ();
+ return 0;
+}
new file mode 100755
@@ -0,0 +1,43 @@
+#!/bin/bash
+# An IFUNC test.
+# Copyright (C) 2016 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library 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
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <http://www.gnu.org/licenses/>.
+
+set -e
+
+objpfx=$1; shift
+test_via_rtld_prefix=$1; shift
+test_wrapper_env=$1; shift
+run_program_env=$1; shift
+logfile=${objpfx}tst-ifunc1.out
+
+> $logfile
+fail=0
+
+${test_wrapper_env} \
+${run_program_env} \
+${objpfx}tst-ifunc1 2>&1 || fail=1
+
+if test $fail = 1; then
+ # If it fails to run, check for the expected error from ld.so.
+ fail=0
+ ${test_wrapper_env} \
+ ${run_program_env} \
+ ${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1
+fi
+
+exit $fail
new file mode 100644
@@ -0,0 +1,26 @@
+/* Test case for IFUNC.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library 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
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+void bar (void *dst, unsigned int);
+
+void
+foo (void)
+{
+ char dst[50];
+ bar (dst, sizeof (dst));
+}
new file mode 100644
@@ -0,0 +1,23 @@
+/* Test case for IFUNC.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library 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
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+void
+bar (void *dst, unsigned int len)
+{
+ __builtin_memset (dst, 3, len);
+}
--
2.7.4