| Message ID | alpine.DEB.2.20.1801161611370.17210@digraph.polyomino.org.uk |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 110705 invoked by alias); 16 Jan 2018 16:20:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 110689 invoked by uid 89); 16 Jan 2018 16:20:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy=adjusted X-HELO: relay1.mentorg.com Date: Tue, 16 Jan 2018 16:20:06 +0000 From: Joseph Myers <joseph@codesourcery.com> To: <libc-alpha@sourceware.org> CC: <carlos@redhat.com>, <dave.anglin@bell.net> Subject: backtrace for hppa Message-ID: <alpine.DEB.2.20.1801161611370.17210@digraph.polyomino.org.uk> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) |
Commit Message
Joseph Myers
Jan. 16, 2018, 4:20 p.m. UTC
As far as I can tell, the only architecture in glibc that uses the generic debug/backtrace.c is hppa. <https://sourceware.org/glibc/wiki/Release/2.26#HPPA> also shows the debug/tst-backtrace* tests as failing for hppa. That suggests the generic debug/backtrace.c is not in fact functional anywhere and so the x86_64 backtrace implementation (which many other architectures include) would be a better generic version. (a) Could someone confirm that those backtrace tests are indeed still failing for hppa with current master? If so, a bug should be filed in Bugzilla for this. (b) Does the following patch change the results for hppa? This is simply the completely mechanical use of the x86_64 version as if it were the default (adjusted to avoid hardcoding libgcc_s.so.1, since hppa uses libgcc_s.so.4); I haven't tested it at all, even to see if it compiles. (Even if this patch doesn't help backtrace results for hppa, that only indicates an hppa-specific version of backtrace is needed, not that the generic one is a useful default.)
Comments
On Tue, Jan 16, 2018 at 11:20 AM, Joseph Myers <joseph@codesourcery.com> wrote: > As far as I can tell, the only architecture in glibc that uses the generic > debug/backtrace.c is hppa. > <https://sourceware.org/glibc/wiki/Release/2.26#HPPA> also shows the > debug/tst-backtrace* tests as failing for hppa. That suggests the generic > debug/backtrace.c is not in fact functional anywhere and so the x86_64 > backtrace implementation (which many other architectures include) would be > a better generic version. I observe that sysdeps/hppa/frame.h has received no substantive changes since 2000, and contains | /* FIXME: will verify this later */ | struct layout | { | void *next; | void *return_address; | }; which suggests that the use of debug/backtrace.c for HPPA was never properly validated. > --- a/sysdeps/x86_64/backtrace.c > +++ b/sysdeps/x86_64/backtrace.c > @@ -20,6 +20,7 @@ > #include <libc-lock.h> > #include <dlfcn.h> > #include <execinfo.h> > +#include <gnu/lib-names.h> > #include <stdlib.h> > #include <unwind.h> > > @@ -49,7 +50,7 @@ dummy_getcfa (struct _Unwind_Context *ctx __attribute__ ((unused))) > static void > init (void) > { > - libgcc_handle = __libc_dlopen ("libgcc_s.so.1"); > + libgcc_handle = __libc_dlopen (LIBGCC_S_SO); > > if (libgcc_handle == NULL) > return; This change should probably be applied independently of whether it helps hppa (and I don't see any reason to wait, either). zw
On 2018-01-16 11:20 AM, Joseph Myers wrote: > (a) Could someone confirm that those backtrace tests are indeed still > failing for hppa with current master? If so, a bug should be filed in > Bugzilla for this. They still fail. I will file a bug as soon as I get a chance (maybe tonight). Dave
On Tue, 16 Jan 2018, Zack Weinberg wrote: > > --- a/sysdeps/x86_64/backtrace.c > > +++ b/sysdeps/x86_64/backtrace.c > > @@ -20,6 +20,7 @@ > > #include <libc-lock.h> > > #include <dlfcn.h> > > #include <execinfo.h> > > +#include <gnu/lib-names.h> > > #include <stdlib.h> > > #include <unwind.h> > > > > @@ -49,7 +50,7 @@ dummy_getcfa (struct _Unwind_Context *ctx __attribute__ ((unused))) > > static void > > init (void) > > { > > - libgcc_handle = __libc_dlopen ("libgcc_s.so.1"); > > + libgcc_handle = __libc_dlopen (LIBGCC_S_SO); > > > > if (libgcc_handle == NULL) > > return; > > This change should probably be applied independently of whether it > helps hppa (and I don't see any reason to wait, either). Without the hppa change, it should be a no-op (m68k uses libgcc_s.so.2, but has its own backtrace.c). So should be harmless, but purely a cleanup in the absence of the hppa change, with no specific justification for including it during the freeze. Logically, there are at least three changes. 1. Make sysdeps/x86_64/backtrace.c use LIBGCC_S_SO. 2. Make hppa use sysdeps/x86_64/backtrace.c (if that helps). 3. Move sysdeps/x86_64/backtrace.c to debug/backtrace.c, remove all the backtrace.c files that included it (seven right now, potentially hppa if my patch helps, potentially riscv), and remove all frame.h files as unused (pure cleanup, should leave installed stripped shared libraries unchanged).
On Jan 16 2018, Joseph Myers <joseph@codesourcery.com> wrote: > As far as I can tell, the only architecture in glibc that uses the generic > debug/backtrace.c is hppa. > <https://sourceware.org/glibc/wiki/Release/2.26#HPPA> also shows the > debug/tst-backtrace* tests as failing for hppa. That suggests the generic > debug/backtrace.c is not in fact functional anywhere and so the x86_64 > backtrace implementation (which many other architectures include) would be > a better generic version. Perhaps merged with the m68k version. Andreas.
On Tue, 16 Jan 2018, Andreas Schwab wrote: > On Jan 16 2018, Joseph Myers <joseph@codesourcery.com> wrote: > > > As far as I can tell, the only architecture in glibc that uses the generic > > debug/backtrace.c is hppa. > > <https://sourceware.org/glibc/wiki/Release/2.26#HPPA> also shows the > > debug/tst-backtrace* tests as failing for hppa. That suggests the generic > > debug/backtrace.c is not in fact functional anywhere and so the x86_64 > > backtrace implementation (which many other architectures include) would be > > a better generic version. > > Perhaps merged with the m68k version. There are several versions that use libgcc_s but have architecture-specific differences from x86_64 (arm i386 m68k s390-32 s390-64 sparc; microblaze and powerpc don't use libgcc_s at all). It may well be possible to factor things to reduce the duplication between those variants, but there are enough differences that I think it makes sense to make put the x86_64 version (used for x86_64 and seven other architectures at present) in debug/ before any further refactoring.
On 16/01/2018 14:48, John David Anglin wrote: > On 2018-01-16 11:20 AM, Joseph Myers wrote: >> (a) Could someone confirm that those backtrace tests are indeed still >> failing for hppa with current master? If so, a bug should be filed in >> Bugzilla for this. > They still fail. I will file a bug as soon as I get a chance (maybe tonight). > > Dave > And your patch does fix the backtrace issues on hppa.
On Tue, 16 Jan 2018 08:53:48 PST (-0800), joseph@codesourcery.com wrote: > On Tue, 16 Jan 2018, Zack Weinberg wrote: > >> > --- a/sysdeps/x86_64/backtrace.c >> > +++ b/sysdeps/x86_64/backtrace.c >> > @@ -20,6 +20,7 @@ >> > #include <libc-lock.h> >> > #include <dlfcn.h> >> > #include <execinfo.h> >> > +#include <gnu/lib-names.h> >> > #include <stdlib.h> >> > #include <unwind.h> >> > >> > @@ -49,7 +50,7 @@ dummy_getcfa (struct _Unwind_Context *ctx __attribute__ ((unused))) >> > static void >> > init (void) >> > { >> > - libgcc_handle = __libc_dlopen ("libgcc_s.so.1"); >> > + libgcc_handle = __libc_dlopen (LIBGCC_S_SO); >> > >> > if (libgcc_handle == NULL) >> > return; >> >> This change should probably be applied independently of whether it >> helps hppa (and I don't see any reason to wait, either). > > Without the hppa change, it should be a no-op (m68k uses libgcc_s.so.2, > but has its own backtrace.c). So should be harmless, but purely a cleanup > in the absence of the hppa change, with no specific justification for > including it during the freeze. > > Logically, there are at least three changes. > > 1. Make sysdeps/x86_64/backtrace.c use LIBGCC_S_SO. > > 2. Make hppa use sysdeps/x86_64/backtrace.c (if that helps). > > 3. Move sysdeps/x86_64/backtrace.c to debug/backtrace.c, remove all the > backtrace.c files that included it (seven right now, potentially hppa if > my patch helps, potentially riscv), and remove all frame.h files as unused > (pure cleanup, should leave installed stripped shared libraries > unchanged). Thanks for the heads up, I think this fixed some of our bugs!
On Thu, 18 Jan 2018, Palmer Dabbelt wrote:
> Thanks for the heads up, I think this fixed some of our bugs!
To be clear, if it works for RISC-V my recommendation is a #include of the
x86_64 version of backtrace. Not a copy, and not the larger refactoring
which can wait for 2.28 as it doesn't fix any user-visible bugs.
On 18/01/2018 16:26, Joseph Myers wrote: > On Thu, 18 Jan 2018, Palmer Dabbelt wrote: > >> Thanks for the heads up, I think this fixed some of our bugs! > > To be clear, if it works for RISC-V my recommendation is a #include of the > x86_64 version of backtrace. Not a copy, and not the larger refactoring > which can wait for 2.28 as it doesn't fix any user-visible bugs. > Yes, I intend to refactor this for 2.28. Although this could simpler to this release point for RISC-V, I think these cross-arch references bring more issues than solutions we should avoid pushing these further.
On 2018-01-18 1:26 PM, Joseph Myers wrote: > On Thu, 18 Jan 2018, Palmer Dabbelt wrote: > >> Thanks for the heads up, I think this fixed some of our bugs! > To be clear, if it works for RISC-V my recommendation is a #include of the > x86_64 version of backtrace. Not a copy, and not the larger refactoring > which can wait for 2.28 as it doesn't fix any user-visible bugs. > Just want to add my thanks for fixing these tests on hppa. Dave
diff --git a/sysdeps/hppa/backtrace.c b/sysdeps/hppa/backtrace.c new file mode 100644 index 0000000..27ce597 --- /dev/null +++ b/sysdeps/hppa/backtrace.c @@ -0,0 +1 @@ +#include <sysdeps/x86_64/backtrace.c> diff --git a/sysdeps/x86_64/backtrace.c b/sysdeps/x86_64/backtrace.c index 2706b50..d423cc0 100644 --- a/sysdeps/x86_64/backtrace.c +++ b/sysdeps/x86_64/backtrace.c @@ -20,6 +20,7 @@ #include <libc-lock.h> #include <dlfcn.h> #include <execinfo.h> +#include <gnu/lib-names.h> #include <stdlib.h> #include <unwind.h> @@ -49,7 +50,7 @@ dummy_getcfa (struct _Unwind_Context *ctx __attribute__ ((unused))) static void init (void) { - libgcc_handle = __libc_dlopen ("libgcc_s.so.1"); + libgcc_handle = __libc_dlopen (LIBGCC_S_SO); if (libgcc_handle == NULL) return;