| Message ID | 20161004184621.GB27454@intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 4 Oct 2016, H.J. Lu wrote: > If an IFUNC function is called before the providing shared library is > unrelocated, ld.so may segfault. Add a testcase to verify that ld.so > will issue a diagnostic and won't segfault in this case. > > Tested on i686 and x86-64. OK for master? I can't see anything x86-specific about these tests. If they are meant to work, they should work on all architectures, and so should be architecture-independent. Is the architecture-specific thing the use of memmove as a function that uses IFUNCs? If so, the tests should still work on other architectures, just maybe be less effective as tests (and other architecture maintainers could always add a hook to use another function instead of memmove). If you suspect other architectures might have similar bugs requiring fixes for these tests to pass but don't want to make such changes yourself, you should follow the usual process for changes made only for some architectures: post a detailed description of how to tell whether the change is needed for your architecture and how to make the change, CC: to the relevant architecture maintainers, and add an entry to https://sourceware.org/glibc/wiki/PortStatus (once the x86 changes are in), listing the possibly affected (i.e. IFUNC-supporting?) architectures on that page.
On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote: > On Tue, 4 Oct 2016, H.J. Lu wrote: > >> If an IFUNC function is called before the providing shared library is >> unrelocated, ld.so may segfault. Add a testcase to verify that ld.so >> will issue a diagnostic and won't segfault in this case. >> >> Tested on i686 and x86-64. OK for master? > > I can't see anything x86-specific about these tests. If they are meant to > work, they should work on all architectures, and so should be > architecture-independent. Is the architecture-specific thing the use of > memmove as a function that uses IFUNCs? If so, the tests should still > work on other architectures, just maybe be less effective as tests (and > other architecture maintainers could always add a hook to use another > function instead of memmove). The result of this test is IFUNC implementation specific. The older x86 IFUNC implementation had # define INIT_ARCH() \ do \ if (__cpu_features.kind == arch_kind_unknown) \ __init_cpu_features (); \ while (0) The new implementation assumed that relocations in libc.so were processed first. It is hard for me to tell if other IFUNC implementations have the similar restriction. > If you suspect other architectures might have similar bugs requiring fixes > for these tests to pass but don't want to make such changes yourself, you > should follow the usual process for changes made only for some > architectures: post a detailed description of how to tell whether the > change is needed for your architecture and how to make the change, CC: to > the relevant architecture maintainers, and add an entry to > https://sourceware.org/glibc/wiki/PortStatus (once the x86 changes are > in), listing the possibly affected (i.e. IFUNC-supporting?) architectures > on that page. > > -- > Joseph S. Myers > joseph@codesourcery.com
On Tue, 4 Oct 2016, H.J. Lu wrote: > On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote: > > On Tue, 4 Oct 2016, H.J. Lu wrote: > > > >> If an IFUNC function is called before the providing shared library is > >> unrelocated, ld.so may segfault. Add a testcase to verify that ld.so > >> will issue a diagnostic and won't segfault in this case. > >> > >> Tested on i686 and x86-64. OK for master? > > > > I can't see anything x86-specific about these tests. If they are meant to > > work, they should work on all architectures, and so should be > > architecture-independent. Is the architecture-specific thing the use of > > memmove as a function that uses IFUNCs? If so, the tests should still > > work on other architectures, just maybe be less effective as tests (and > > other architecture maintainers could always add a hook to use another > > function instead of memmove). > > The result of this test is IFUNC implementation specific. The older > x86 IFUNC implementation had > > # define INIT_ARCH() \ > do \ > if (__cpu_features.kind == arch_kind_unknown) \ > __init_cpu_features (); \ > while (0) > > The new implementation assumed that relocations in libc.so were > processed first. It is hard for me to tell if other IFUNC implementations > have the similar restriction. You seem to be saying there was some bug in the x86 IFUNC implementation such that you don't know whether a corresponding bug might be present for other architectures or not. That is a very strong indication that the tests should be architecture-independent, so that people can use the results of those tests on other architectures to tell whether those other architectures need fixing as well.
On Tue, Oct 4, 2016 at 3:55 PM, Joseph Myers <joseph@codesourcery.com> wrote: > On Tue, 4 Oct 2016, H.J. Lu wrote: > >> On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote: >> > On Tue, 4 Oct 2016, H.J. Lu wrote: >> > >> >> If an IFUNC function is called before the providing shared library is >> >> unrelocated, ld.so may segfault. Add a testcase to verify that ld.so >> >> will issue a diagnostic and won't segfault in this case. >> >> >> >> Tested on i686 and x86-64. OK for master? >> > >> > I can't see anything x86-specific about these tests. If they are meant to >> > work, they should work on all architectures, and so should be >> > architecture-independent. Is the architecture-specific thing the use of >> > memmove as a function that uses IFUNCs? If so, the tests should still >> > work on other architectures, just maybe be less effective as tests (and >> > other architecture maintainers could always add a hook to use another >> > function instead of memmove). >> >> The result of this test is IFUNC implementation specific. The older >> x86 IFUNC implementation had >> >> # define INIT_ARCH() \ >> do \ >> if (__cpu_features.kind == arch_kind_unknown) \ >> __init_cpu_features (); \ >> while (0) >> >> The new implementation assumed that relocations in libc.so were >> processed first. It is hard for me to tell if other IFUNC implementations >> have the similar restriction. > > You seem to be saying there was some bug in the x86 IFUNC implementation It is a limitation. > such that you don't know whether a corresponding bug might be present for > other architectures or not. That is a very strong indication that the > tests should be architecture-independent, so that people can use the > results of those tests on other architectures to tell whether those other > architectures need fixing as well. > I can try __builtin_memcpy, instread of __builtin_memmove. There are 2 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.
On Wed, 5 Oct 2016, H.J. Lu wrote: > I can try __builtin_memcpy, instread of __builtin_memmove. There are 2 > 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).
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 0d0326c2..6907b46 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -7,4 +7,29 @@ sysdep-dl-routines += dl-get-cpu-features tests += tst-get-cpu-features tests-static += tst-get-cpu-features-static + +ifeq ($(build-shared),yes) +tests += tst-ifunc1 +tests-special += $(objpfx)tst-ifunc1.out +$(objpfx)tst-ifunc1.out: ../sysdeps/x86/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: ../sysdeps/x86/tst-ifuncmod1a.c + $(CC) -fPIC -c -o $@ $^ +$(objpfx)tst-ifuncmod1b.os: ../sysdeps/x86/tst-ifuncmod1b.c + $(CC) -fPIC -c -o $@ $^ +endif endif diff --git a/sysdeps/x86/tst-ifunc1.c b/sysdeps/x86/tst-ifunc1.c new file mode 100644 index 0000000..735153f --- /dev/null +++ b/sysdeps/x86/tst-ifunc1.c @@ -0,0 +1,26 @@ +/* Test case for x86 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; +} diff --git a/sysdeps/x86/tst-ifunc1.sh b/sysdeps/x86/tst-ifunc1.sh new file mode 100755 index 0000000..5f5afe0 --- /dev/null +++ b/sysdeps/x86/tst-ifunc1.sh @@ -0,0 +1,35 @@ +#!/bin/bash +# An x86 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 | grep Relink >> $logfile || fail=1 + +exit $fail diff --git a/sysdeps/x86/tst-ifuncmod1a.c b/sysdeps/x86/tst-ifuncmod1a.c new file mode 100644 index 0000000..a1c27ed --- /dev/null +++ b/sysdeps/x86/tst-ifuncmod1a.c @@ -0,0 +1,27 @@ +/* Test case for x86 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, void *src); + +void +foo (void) +{ + char dst[50]; + char src[50]; + bar (dst, src); +} diff --git a/sysdeps/x86/tst-ifuncmod1b.c b/sysdeps/x86/tst-ifuncmod1b.c new file mode 100644 index 0000000..e6e9e9b --- /dev/null +++ b/sysdeps/x86/tst-ifuncmod1b.c @@ -0,0 +1,23 @@ +/* Test case for x86 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, void *src) +{ + __builtin_memmove (dst, src, 40); +}