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.129.124]) by sourceware.org (Postfix) with ESMTPS id DF486384F49F for ; Thu, 24 Nov 2022 11:39:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DF486384F49F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669289986; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VEB5QfCI3lBkDMYm3a2M6k4hYdBmeAXi4gkGz6TzcuU=; b=SR/F6oW4JnVnCxrWxwYyRBon5pK25347dboD85Bg9y9LxgwR4nq5zNdikCjHqBC8TfxpKu hJm4xuofp+igezq/gIu3pbACgMAyX/E72Sfo5xIbXPGA0yphdkmnhYx+I6zOq415sWrtGo jlSpE9sC/5FvM0qU4EXys2EGgRbvA6A= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-259-Q-mfyEfMOL6og_wgT9r1nw-1; Thu, 24 Nov 2022 06:39:43 -0500 X-MC-Unique: Q-mfyEfMOL6og_wgT9r1nw-1 Received: by mail-wm1-f72.google.com with SMTP id v188-20020a1cacc5000000b003cf76c4ae66so2497670wme.7 for ; Thu, 24 Nov 2022 03:39:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VEB5QfCI3lBkDMYm3a2M6k4hYdBmeAXi4gkGz6TzcuU=; b=jDXao8UWQZPpaMa1E4Jh+2lM8wp5TsdW738YCgaqqjsVa9SK2D7c0EzstxBjd8GVas ZY57SMch+Ky68g1YfSs2WAPrZX9fkfUyh6cby/WQEpde4z2/8VavSitcTRjKuVSCQFsu /hEqbl/rShTpaUXpVqdc3bm6Ee9FBvCe4yiS7riqKkxKuZOuN/fcuZbUX67IpzSLHdL4 S2+WDEsTnn5RzywZ/rhyXALMeOrN4aNpKfI/7X8aZlNGfVWnVlsoTpJt41Koy8sdGNY1 l173VNBXRy1kv3oClzrgf2GOsI/mEd93UNvXai0TPEh9rZ1izZ6KCCgrsJskGGGbdbDq Q1tw== X-Gm-Message-State: ANoB5pmWAVEAQZ1UHHRVmxYrvxAtKKzMUX4maT6f78kmine01N//5LkY YyV5txRawkH+APpcxaBOWQmcSuoExIFkujwPYvNr/6PPx8194rOUQ3LxO2s/4s76oUeipaPG4Cg XCynj23uCCtkL+dp3ygs49g== X-Received: by 2002:a5d:56c2:0:b0:241:94bc:2796 with SMTP id m2-20020a5d56c2000000b0024194bc2796mr11501582wrw.184.1669289982286; Thu, 24 Nov 2022 03:39:42 -0800 (PST) X-Google-Smtp-Source: AA0mqf6cOQVQFQf2iEoY25r72C3hTgmxCiwuWiFhrcWAWLCPVzFelSZE7sw3kGCI0reaLopBb9nCdg== X-Received: by 2002:a5d:56c2:0:b0:241:94bc:2796 with SMTP id m2-20020a5d56c2000000b0024194bc2796mr11501571wrw.184.1669289982034; Thu, 24 Nov 2022 03:39:42 -0800 (PST) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id r17-20020a5d4e51000000b0023c8026841csm1150588wrt.23.2022.11.24.03.39.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Nov 2022 03:39:41 -0800 (PST) From: Andrew Burgess To: Lancelot SIX Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb: relax requirement for the map_failed stap probe to be present In-Reply-To: <20221122153341.oegu3r7v4nwfukqi@ubuntu.lan> References: <20221122153341.oegu3r7v4nwfukqi@ubuntu.lan> Date: Thu, 24 Nov 2022 11:39:40 +0000 Message-ID: <87v8n437k3.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.0 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_H2,SPF_HELO_NONE,SPF_NONE,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: Lancelot SIX writes: > On Tue, Nov 22, 2022 at 03:09:40PM +0000, 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; > > Hi, > > Wouldn't this just disable the "map_failed" probe for everyone, even if > it is available for the current glibc? > > Should this become: > > if (streq (probe_info[i].name, "map_failed") && probes[i].empty ()) > continue > > to only ignore this probe if it is missing? That's a very good point. In the update below, I've moved the name check inside the immediately following 'if (probes[i].empty)' block. I've also added an assert that the action for the probe is DO_NOTHING - if that action did ever change, and we then skipped the probe we'd be in trouble. However, I've also replied to Simon with an alternative approach to this fix. I'm not really sure which of the two solutions I prefer, so I'm posting both to see if there's a preference in the group. Thanks, Andrew --- commit 137177ba43c07aa3e33f5244fcc826e26f2474eb Author: Andrew Burgess Date: Tue Nov 22 12:45:56 2022 +0000 gdb: relax requirement for the map_failed stap probe to be present 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. diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 6acaf87960b..c948808f853 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -2205,15 +2205,26 @@ 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")) - continue; - /* Ensure at least one probe for the current name was found. */ if (probes[i].empty ()) - return false; + { + /* 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, and the compiler optimized + the probe away. In this case the probe name doesn't have the + "rtld_" prefix. */ + if (streq (probe_info[i].name, "map_failed")) + { + gdb_assert (probe_info[i].action == DO_NOTHING); + continue; + } + + return false; + } /* Ensure probe arguments can be evaluated. */ for (probe *p : probes[i])