From patchwork Fri Apr 16 15:56:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 43021 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0E40A3989CB2; Fri, 16 Apr 2021 15:56:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0E40A3989CB2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1618588570; bh=h5ijUtQeS2n4kZOjNe+Bc1/OMnlWs4oOrOVx/B1ng+Y=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=bvg4mrfYCdZmKJxppucLUPoFskornQY8a359ZXUmFdq14hoAnAz+MWFJAVgbiZjGt ryqfrGMgOD0nAcXlxhHoGv1wsiF2UBI2kcOILhLtSd2H+R1iuIbJhPy6vIgw+ZG8BL cCkrREubLakdQVOJGQU3ywp4UjcLIjwHgPJTZwX4= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 216C33989C82 for ; Fri, 16 Apr 2021 15:56:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 216C33989C82 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-159-juTgPFHUPIKYcsvGcQGrcA-1; Fri, 16 Apr 2021 11:56:01 -0400 X-MC-Unique: juTgPFHUPIKYcsvGcQGrcA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0450B18B613D; Fri, 16 Apr 2021 15:56:00 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-113-139.ams2.redhat.com [10.36.113.139]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 08A275D9C6; Fri, 16 Apr 2021 15:55:57 +0000 (UTC) To: libc-alpha@sourceware.org, gdb-patches@sourceware.org Subject: [PATCH glibc] nptl_db: different libpthread/ld.so load orders (bug 27744) Date: Fri, 16 Apr 2021 17:56:16 +0200 Message-ID: <87sg3qnrz3.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, KAM_STOCKGEN, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Florian Weimer via Libc-alpha From: Florian Weimer Reply-To: Florian Weimer Cc: Pedro Alves , Kevin Buettner , Simon Marchi Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" libthread_db is loaded once GDB encounters libpthread, and at this point, ld.so may not have been loaded yet. As a result, _rtld_global cannot be accessed by regular means from libthread_db. To make this work until GDB can be fixed, acess _rtld_global through a pointer stored in libpthread. The new test does not reproduce bug 27744 with --disable-hardcoded-path-in-tests, but is still a valid smoke test. With --enable-hardcoded-path-in-tests, it is necessary to avoid add-symbol-file because this can tickle a GDB bug. Fixes commit 1daccf403b1bd86370eb94edca794dc106d02039 ("nptl: Move stack list variables into _rtld_global"). Tested on i686-linux-gnu and x86_64-linux-gnu, the latter also with --enable-hardcoded-path-in-tests. --- nptl/Makefile | 19 ++++- nptl/pthread_create.c | 8 +++ nptl/tst-pthread-gdb-attach-static.c | 1 + nptl/tst-pthread-gdb-attach.c | 134 +++++++++++++++++++++++++++++++++++ nptl_db/structs.def | 3 +- nptl_db/td_init.c | 15 ++-- nptl_db/thread_dbP.h | 2 + 7 files changed, 171 insertions(+), 11 deletions(-) diff --git a/nptl/Makefile b/nptl/Makefile index 8fe92d43fa..8a6a0a0edd 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -313,7 +313,8 @@ tests = tst-attr2 tst-attr3 tst-default-attr \ tst-thread-affinity-sched \ tst-pthread-defaultattr-free \ tst-pthread-attr-sigmask \ - tst-pthread-timedlock-lockloop + tst-pthread-timedlock-lockloop \ + tst-pthread-gdb-attach tests-container = tst-pthread-getattr @@ -359,6 +360,19 @@ CPPFLAGS-test-cond-printers.c := $(CFLAGS-printers-tests) CPPFLAGS-test-rwlockattr-printers.c := $(CFLAGS-printers-tests) CPPFLAGS-test-rwlock-printers.c := $(CFLAGS-printers-tests) +# Reuse the CFLAGS setting for the GDB attaching test. It needs +# debugging information. +CFLAGS-tst-pthread-gdb-attach.c := $(CFLAGS-printers-tests) +CPPFLAGS-tst-pthread-gdb-attach.c := $(CFLAGS-printers-tests) +ifeq ($(build-shared)$(build-hardcoded-path-in-tests),yesno) +CPPFLAGS-tst-pthread-gdb-attach.c += -DDO_ADD_SYMBOL_FILE=1 +else +CPPFLAGS-tst-pthread-gdb-attach.c += -DDO_ADD_SYMBOL_FILE=0 +endif +CFLAGS-tst-pthread-gdb-attach-static.c := $(CFLAGS-printers-tests) +CPPFLAGS-tst-pthread-gdb-attach-static.c := \ + $(CFLAGS-printers-tests) -DDO_ADD_SYMBOL_FILE=0 + ifeq ($(build-shared),yes) tests-printers-libs := $(shared-thread-library) else @@ -430,7 +444,8 @@ link-libc-static := $(common-objpfx)libc.a $(static-gnulib) \ tests-static += tst-stackguard1-static \ tst-cancel24-static \ tst-mutex8-static tst-mutexpi8-static tst-sem11-static \ - tst-sem12-static tst-cond11-static + tst-sem12-static tst-cond11-static \ + tst-pthread-gdb-attach-static tests += tst-cancel24-static diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 6c645aff48..f13d8e44a4 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -51,6 +51,14 @@ static td_thr_events_t __nptl_threads_events __attribute_used__; /* Pointer to descriptor with the last event. */ static struct pthread *__nptl_last_event __attribute_used__; +#ifdef SHARED +/* This variable is used to access _rtld_global from libthread_db. If + GDB loads libpthread before ld.so, it is not possible to resolve + _rtld_global directly during libpthread initialization. */ +static struct rtld_global *__nptl_rtld_global __attribute_used__ + = &_rtld_global; +#endif + /* Number of threads running. */ unsigned int __nptl_nthreads = 1; diff --git a/nptl/tst-pthread-gdb-attach-static.c b/nptl/tst-pthread-gdb-attach-static.c new file mode 100644 index 0000000000..e159632cac --- /dev/null +++ b/nptl/tst-pthread-gdb-attach-static.c @@ -0,0 +1 @@ +#include "tst-pthread-gdb-attach.c" diff --git a/nptl/tst-pthread-gdb-attach.c b/nptl/tst-pthread-gdb-attach.c new file mode 100644 index 0000000000..36f2e4e526 --- /dev/null +++ b/nptl/tst-pthread-gdb-attach.c @@ -0,0 +1,134 @@ +/* Smoke testing GDB process attach with thread-local variable access. + Copyright (C) 2021 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 + . */ + +/* This test runs GDB against a forked copy of itself, to check + whether libthreaddb can be loaded, and that access to thread-local + variables works. */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* Starts out as zero, changed to 1 or 2 by the debugger, depending on + the thread. */ +__thread volatile int altered_by_debugger; + +/* Writes the GDB script to run the test to PATH. */ +static void +write_gdbscript (const char *path, int tested_pid) +{ + FILE *fp = xfopen (path, "w"); + fprintf (fp, + "set trace-commands on\n" + "set debug libthread-db 1\n" +#if DO_ADD_SYMBOL_FILE + /* Do not do this unconditionally to work around a GDB + assertion failure: ../../gdb/symtab.c:6404: + internal-error: CORE_ADDR get_msymbol_address(objfile*, + const minimal_symbol*): Assertion `(objf->flags & + OBJF_MAINLINE) == 0' failed. */ + "add-symbol-file %1$s/nptl/tst-pthread-gdb-attach\n" +#endif + "set auto-load safe-path %1$s/nptl_db\n" + "set libthread-db-search-path %1$s/nptl_db\n" + "attach %2$d\n", + support_objdir_root, tested_pid); + fputs ("break debugger_inspection_point\n" + "continue\n" + "thread 1\n" + "print altered_by_debugger\n" + "print altered_by_debugger = 1\n" + "thread 2\n" + "print altered_by_debugger\n" + "print altered_by_debugger = 2\n" + "continue\n", + fp); + xfclose (fp); +} + +/* The test sets a breakpoint on this function and alters the + altered_by_debugger thread-local variable. */ +void __attribute__ ((weak)) +debugger_inspection_point (void) +{ +} + +/* Thread function for the test thread in the subprocess. */ +static void * +subprocess_thread (void *closure) +{ + /* Wait until altered_by_debugger changes the value away from 0. */ + while (altered_by_debugger == 0) + { + usleep (100 * 1000); + debugger_inspection_point (); + } + + TEST_COMPARE (altered_by_debugger, 2); + return NULL; +} + +/* This function implements the subprocess un der test. It creates a + second thread, waiting for its value to change to 2, and checks + that the main thread also changed its value to 1. */ +static void +in_subprocess (void) +{ + pthread_t thr = xpthread_create (NULL, subprocess_thread, NULL); + TEST_VERIFY (xpthread_join (thr) == NULL); + TEST_COMPARE (altered_by_debugger, 1); + _exit (0); +} + +static int +do_test (void) +{ + pid_t tested_pid = xfork (); + if (tested_pid == 0) + in_subprocess (); + char *tested_pid_string = xasprintf ("%d", tested_pid); + + char *gdbscript; + xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript)); + write_gdbscript (gdbscript, tested_pid); + + pid_t gdb_pid = xfork (); + if (gdb_pid == 0) + { + clearenv (); + xdup2 (STDOUT_FILENO, STDERR_FILENO); + execlp ("gdb", "gdb", "-nx", "-batch", "-x", gdbscript, NULL); + } + + int status; + TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid); + TEST_COMPARE (status, 0); + TEST_COMPARE (xwaitpid (tested_pid, &status, 0), tested_pid); + TEST_COMPARE (status, 0); + + free (tested_pid_string); + free (gdbscript); + return 0; +} + +#include diff --git a/nptl_db/structs.def b/nptl_db/structs.def index 999a9fc35a..8a613dd2f5 100644 --- a/nptl_db/structs.def +++ b/nptl_db/structs.def @@ -100,8 +100,7 @@ DB_STRUCT_FIELD (pthread, dtvp) #endif #if !(IS_IN (libpthread) && !defined SHARED) -DB_STRUCT (rtld_global) -DB_RTLD_VARIABLE (_rtld_global) +DB_VARIABLE (__nptl_rtld_global) #endif DB_RTLD_GLOBAL_FIELD (dl_tls_dtv_slotinfo_list) DB_RTLD_GLOBAL_FIELD (dl_stack_user) diff --git a/nptl_db/td_init.c b/nptl_db/td_init.c index 1d15681228..06b5adc5c2 100644 --- a/nptl_db/td_init.c +++ b/nptl_db/td_init.c @@ -33,13 +33,14 @@ td_init (void) bool __td_ta_rtld_global (td_thragent_t *ta) { - if (ta->ta_addr__rtld_global == 0 - && td_mod_lookup (ta->ph, LD_SO, SYM__rtld_global, - &ta->ta_addr__rtld_global) != PS_OK) + if (ta->ta_addr__rtld_global == 0) { - ta->ta_addr__rtld_global = (void*)-1; - return false; + psaddr_t rtldglobalp; + if (DB_GET_VALUE (rtldglobalp, ta, __nptl_rtld_global, 0) == TD_OK) + ta->ta_addr__rtld_global = rtldglobalp; + else + ta->ta_addr__rtld_global = (void *) -1; } - else - return ta->ta_addr__rtld_global != (void*)-1; + + return ta->ta_addr__rtld_global != (void *)-1; } diff --git a/nptl_db/thread_dbP.h b/nptl_db/thread_dbP.h index 580a70c471..712fa3aeb6 100644 --- a/nptl_db/thread_dbP.h +++ b/nptl_db/thread_dbP.h @@ -108,6 +108,8 @@ struct td_thragent # undef DB_SYMBOL # undef DB_VARIABLE + psaddr_t ta_addr__rtld_global; + /* The method of locating a thread's th_unique value. */ enum {