From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 8F22F3858004 for ; Thu, 15 Apr 2021 03:46:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8F22F3858004 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-154-Nv0bPcrMMLiyMA7-bq3i-w-1; Wed, 14 Apr 2021 23:46:09 -0400 X-MC-Unique: Nv0bPcrMMLiyMA7-bq3i-w-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 168F5501E0; Thu, 15 Apr 2021 03:46:08 +0000 (UTC) Received: from f33-m1.lan (ovpn-112-52.phx2.redhat.com [10.3.112.52]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A3DCA2ABBF; Thu, 15 Apr 2021 03:46:07 +0000 (UTC) Date: Wed, 14 Apr 2021 20:46:06 -0700 From: Kevin Buettner To: Simon Marchi Cc: gdb-patches@sourceware.org, Joel Brobecker , Pedro Alves , Florian Weimer Subject: Re: GDB 10.2 Release -- 2021-04-11 update Message-ID: <20210414204606.2204d20d@f33-m1.lan> In-Reply-To: References: <20210411053920.GA1629728@adacore.com> <20210411230018.30827d60@f33-m1.lan> <20210412211855.771ff58a@f33-m1.lan> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Apr 2021 03:46:14 -0000 On Tue, 13 Apr 2021 21:45:53 -0400 Simon Marchi wrote: > On 2021-04-13 12:18 a.m., Kevin Buettner wrote:> On Mon, 12 Apr 2021 10:04:44 -0400 [...] > > > > diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc > > index 055a0fa970f..9403b419710 100644 > > --- a/gdbserver/thread-db.cc > > +++ b/gdbserver/thread-db.cc > > @@ -44,10 +44,14 @@ struct thread_db > > /* Connection to the libthread_db library. */ > > td_thragent_t *thread_agent; > > > > - /* If this flag has been set, we've already asked GDB for all > > - symbols we might need; assume symbol cache misses are > > - failures. */ > > - int all_symbols_looked_up; > > + /* This flag is set after asking GDB for symbols that we need. It > > + does not, however, indicate that all necessary symbols have found > > + as these may be split among several shared objects. */ > > + bool initialized; > > + > > + /* Flag which indicates that it's okay to ask GDB for symbols via > > + a qSymbol reply. */ > > + bool qsymbol_reply_okay; > > > > #ifndef USE_LIBTHREAD_DB_DIRECTLY > > /* Handle of the libthread_db from dlopen. */ > > @@ -359,19 +363,20 @@ thread_db_look_up_symbols (void) > > const char **sym_list; > > CORE_ADDR unused; > > > > + thread_db->qsymbol_reply_okay = true; > > + > > for (sym_list = thread_db->td_symbol_list_p (); *sym_list; sym_list++) > > look_up_one_symbol (*sym_list, &unused, 1); > > > > - /* We're not interested in any other libraries loaded after this > > - point, only in symbols in libpthread.so. */ > > - thread_db->all_symbols_looked_up = 1; > > + thread_db->qsymbol_reply_okay = false; > > + thread_db->initialized = true; > > } > > > > int > > thread_db_look_up_one_symbol (const char *name, CORE_ADDR *addrp) > > { > > struct thread_db *thread_db = current_process ()->priv->thread_db; > > - int may_ask_gdb = !thread_db->all_symbols_looked_up; > > + int may_ask_gdb = thread_db->qsymbol_reply_okay || !thread_db->initialized; > > > > /* If we've passed the call to thread_db_look_up_symbols, then > > anything not in the cache must not exist; we're not interested > > @@ -396,7 +401,7 @@ thread_db_get_tls_address (struct thread_info *thread, CORE_ADDR offset, > > thread_db = proc->priv->thread_db; > > > > /* If the thread layer is not (yet) initialized, fail. */ > > - if (thread_db == NULL || !thread_db->all_symbols_looked_up) > > + if (thread_db == NULL || !thread_db->initialized) > > return TD_ERR; > > > > /* If td_thr_tls_get_addr is missing rather do not expect td_thr_tlsbase > > @@ -888,7 +893,7 @@ thread_db_notice_clone (struct thread_info *parent_thr, ptid_t child_ptid) > > > > /* If the thread layer isn't initialized, return. It may just > > be that the program uses clone, but does not use libthread_db. */ > > - if (thread_db == NULL || !thread_db->all_symbols_looked_up) > > + if (thread_db == NULL || !thread_db->initialized) > > return; > > > > /* find_one_thread calls into libthread_db which accesses memory via > > > > Thanks! > > So from what I understand, the problem is that GDB offers GDBserver the > opportunity to look up some symbols in between each objfile read in the > initial library scan. After GDB has loaded lipthread (but not ld-linux > yet), libthread_db fails to work because GDB doesn't know the ld-linux > symbols yet. It sounds very much like the non-GDBserver case. > > I see that you are trying to fix that GDBserver-side (although I don't > quite get what you are trying to do, perhaps a quick explanation would > help). On the gdbserver side of things, I noticed that there's a flag named "all_symbols_looked_up". This flag is used for two purposes: 1) It's used by thread_db_get_tls_address() and thread_db_notice_clone() to test whether the thread_db layer is initialized enough to proceed. 2) It's used by thread_db_look_up_one_symbol() to find out whether it's okay to send a qSymbol reply. Regarding this particular change, it currently looks like this: - int may_ask_gdb = !thread_db->all_symbols_looked_up; + int may_ask_gdb = thread_db->qsymbol_reply_okay || !thread_db->initialized; However, I had originally (re)written that line as follows: + int may_ask_gdb = thread_db->qsymbol_reply_okay; This looks correct to me, and it caused gdb.threads/fork-plus-threads.exp to work as well as it does on a glibc 2.32 machine. Unfortunately, it caused gdb.threads/tls.exp (and other tests too) to regress. So, anyway, my thought was to split that one flag which serves two purposes into separate flags. This would allow us to process additional qSymbol requests from GDB when new shared libraries are loaded. I'm not confident that this approach can be made to work, though it looked promising for a while. > Could we instead tackle it GDB-side, with the same idea as what > I proposed the native debugging? We could suppress the qsymbol packets > sent in between each objfile loaded from the initial library scan. We > would have a single qsymbol sent after finishing loading all the initial > libraries. Sure, I'm willing to work on this, or at least help out... > For example, I just hacked this, this makes it work for the simple > non-stop attach case with gdbserver, where you run gdbserver with > --attach and then connect with GDB (I haven't tried the other modes). > The final qsymbol, that lets GDBserver load libthread_db, is sent in > remote_target::start_remote. I tested your patch today (against the test suite) using both --target_board=native-extended-gdbserver/-m64 and --target_board=native-gdbserver. Unfortunately, I didn't see a test which was helped by your patch. (Though there was the usual noise from racy tests.) For the test that I've been focused on, I see the following results on a glibc 2.32 machine: [kev@f33-1 gdb]$ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver/-m64" TESTS=gdb.threads/fork-plus-threads.exp ... FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: only inferior 1 left === gdb Summary === # of expected passes 15 # of unexpected failures 1 With or without your patch, I see this result on a glibc 2.33 machine: [kev@f33-3 gdb]$ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver/-m64" TESTS=gdb.threads/fork-plus-threads.exp ... FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: inferior 1 exited FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: no threads left FAIL: gdb.threads/fork-plus-threads.exp: detach-on-fork=off: only inferior 1 left WARNING: Timed out waiting for EOF in server after monitor exit === gdb Summary === # of expected passes 13 # of unexpected failures 3 (It also leaves a gdbserver running which must be killed with -KILL.) The other thing that I should mention is that Florian Weimer has told me that (he thinks) glibc 2.33 can be changed so that no GDB changes are needed. So, if we can't figure it out, we might want to ask Florian to do this. Florian has also said that things will change yet again in glibc 2.34. It's not clear to me that the fix we're investigating for glibc 2.33 will still be required. Florian told me: "So ideally we'd figure out a way to load libthread_db even if there is no libpthread in the inferior." This sounds like a different problem than the one we're currently investigating. Kevin