From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id BDCA73858C27 for ; Tue, 22 Nov 2022 15:31:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BDCA73858C27 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6E39B1E0CB; Tue, 22 Nov 2022 10:31:11 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1669131071; bh=8jJlxiDzgNAr+Z9nVdlZ+24Tq4dncCugG777A1bsqGM=; h=Date:Subject:To:References:From:In-Reply-To:From; b=UcDWFw3EvIVVip8RcJcu+TuKUmmKQ65o/pMiaKw/0HikKfvydMs4T/5o/2hEBJHfm dV7r5duBVRVXZqRqen2BjAwQwoGxTkwleZwLYlGxgAaLUYPVc3rnnV6dXUu537QBEL ViWM7gRyTeuQKaV7Ogmf35l8yvigzHw/GZp8YQNE= Message-ID: <9b1fe2f9-b4a9-17a8-1175-a1a776db1afc@simark.ca> Date: Tue, 22 Nov 2022 10:31:11 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH] gdb: relax requirement for the map_failed stap probe to be present Content-Language: fr To: Andrew Burgess , gdb-patches@sourceware.org References: From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/22/22 10:09, Andrew Burgess via Gdb-patches wrote: > From glibc 2.35 and later, the "map_failed" stap probe is no longer > included in glibc. The removal of the probe looks like an accident, > but it was caused by a glibc commit which meant that the "map_failed" > probe could no longer be reached; the compiler than helpfully > optimised out the probe. > > In GDB, in solib-svr4.c, we have a list of probes that we look for > related to the shared library loading detection. If any of these > probes are missing then GDB will fall back to the non-probe based > mechanism for detecting shared library loading. The "map_failed" > probe is include in the list of required probes. > > This means that on glibc 2.35 (or later) systems, GDB is going to > always fall back to the non-probes based mechanism for detecting > shared library loading. > > I raised a glibc bug to discuss this issue: > > https://sourceware.org/bugzilla/show_bug.cgi?id=29818 > > But, whatever the ultimate decision from the glibc team, given there > are version of glibc in the wild without the "map_failed" probe, we > probably should update GDB to handle this situation. > > The "map_failed" probe is already a little strange, very early > versions of glibc didn't include this probe, so, in some cases, if > this probe is missing GDB is happy to ignore it. In this commit I > just expand this logic to make the "map_failed" probe fully optional. > > With this commit in place, then, when using a glibc 2.35 or later > system, GDB will once again use the stap probes for shared library > detection. > --- > gdb/solib-svr4.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c > index 6acaf87960b..87cd06f251a 100644 > --- a/gdb/solib-svr4.c > +++ b/gdb/solib-svr4.c > @@ -2205,10 +2205,15 @@ svr4_find_and_create_probe_breakpoints (svr4_info *info, > > probes[i] = find_probes_in_objfile (os->objfile, "rtld", name); > > - /* The "map_failed" probe did not exist in early > - versions of the probes code in which the probes' > - names were prefixed with "rtld_". */ > - if (with_prefix && streq (name, "rtld_map_failed")) > + /* The "map_failed" probe did not exist in early versions of the > + probes code in which the probes' names were prefixed with > + "rtld_". > + > + Additionally, the "map_failed" probe was accidentally removed > + from glibc 2.35 and later, when changes in glibc meant the probe > + could no longer be reached. In this case the probe name doesn't > + have the "rtld_" prefix. */ > + if (streq (probe_info[i].name, "map_failed")) > continue; > > /* Ensure at least one probe for the current name was found. */ > > base-commit: 84f9fbe90e5429adb9dee68f04f44c92fa9e2345 > -- > 2.25.4 Hi, I looked at this separately, and this was one of the fixes I considered. Another option was to make GDB not give up on the probes interface if failing to look up a probe whose action is DO_NOTHING. Probes with that action are not used by GDB for solib bookkeeping, but can be used to stop on solib events, with "set stop-on-solib-events". I was just worried if there was some cases where a probe would be missing, but the corresponding event could be caught if using the original interface. In that case, using the probes interface would be a regression. But it's probably not worth wondering about. If that happens it's just a bug that needs to be fixed. In the case we are looking at, if the map_failed probe gets optimized out, then surely the corresponding call to the r_brk function would also be optimized out. So, the patch LGTM: Approved-By: Simon Marchi Simon