From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id EEnwJYP2iGfACxEAWB0awg (envelope-from ) for ; Thu, 16 Jan 2025 07:07:31 -0500 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=LXejPf4c; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8BF851E100; Thu, 16 Jan 2025 07:07:31 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.1 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_INVALID,DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 807B81E08E for ; Thu, 16 Jan 2025 07:07:29 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 11F903850206 for ; Thu, 16 Jan 2025 12:07:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 11F903850206 Authentication-Results: sourceware.org; dkim=fail reason="signature verification failed" (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=LXejPf4c 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 807A33850206 for ; Thu, 16 Jan 2025 12:06:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 807A33850206 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 807A33850206 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737029197; cv=none; b=rMWLisboHxeZJC/l2p1+nEeqhr8JB6ShYTVbTkJGmWaEezaVMRTQnO+7jMh0PtIjJ/cxTo+XyIHQ60SjQkeeIW9jFfi2NRQ09r8lxsSN9W4S3beorZrHBG1l+fvLc1jz57t/vrUSU9veQUw1BWuw97rtZJbmOvAyKLAESRDtENk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1737029197; c=relaxed/simple; bh=k9J/raHgkOx16o47dI7KiNE5qFZ4iNWLePPpLtG7En4=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=vOf7Dr+2nvNuNZeCNHnUM5SbDPmJoNtlWeqCX9MP8D0sacgePQQhIXU4EcOmXJ7chvdjrzWyi+iitGEDdfJ30iy70g3Om6yimyY3l6za7AEw63Xj8tj87+mZgSxeEEVD2vK0xfOVFkoQZa+z8ve1hfpU+sA5n5wGmmyGZPsIy1c= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 807A33850206 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1737029197; 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=PtGY9V3oesP/2+8R/JjX2VWv8gbHNLtiwOl8ZyMGw8k=; b=LXejPf4cLs1ClLeXMcwHKmYz7NH5HoaLLbzo4HqwqtwbMzNxAra+n+w9PY5nw0aecW1LEq GYxNdcZXCO1HC+m3o5duozxOKIR/AokfmiIecRjiTa71enu3oOr5VoGHZm5LiR0Tn56a9j x+ATfiI88vDNRjT/NhRgX9UnUy0gK0E= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-228-gpRrmoUUOhWdwPuVo5alPg-1; Thu, 16 Jan 2025 07:06:36 -0500 X-MC-Unique: gpRrmoUUOhWdwPuVo5alPg-1 X-Mimecast-MFC-AGG-ID: gpRrmoUUOhWdwPuVo5alPg Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4362f893bfaso3949885e9.1 for ; Thu, 16 Jan 2025 04:06:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737029195; x=1737633995; 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=bgM3chTaUm1fLjM37OxQgEGby6Nkv079+feyua0hQgo=; b=PovSZel9u74+TeQSN96w3XOXAZNX78/y5LeOJVQa0tfDr4fiTcnAc6fKzZ6IwBlAK9 rEChc2am7xke4r4HCEY74k8C+yB3x72KmBP5AjfzcPBOqNn5gloAjTkJ+BPCswU3bgDz 1fZ6v/Wk03iUGJYzmGM/lwnbjQnCwNea/+SetO8Btt3942CHaINJtXK5tZVINVn1BAiB xHwOLlCaIQNZCnY81dgokEhE9wjKM6U5p5ZF0tJ+hv8uzCjmH6akg/Hd+ZvX7W9VknMl rqQ2tl0MbYLeQ7dPC/4dMi0SSbD+x2aG67xnrNkXG8Sn3fbgQgOe5QasmbHi9B4Yhr8g s7zw== X-Forwarded-Encrypted: i=1; AJvYcCWDhU5cHsVfArtyzkK7bxOE53LYMYn/7yThaIZUnPRYJaClcmB9JrIFS+99jqBiEBW1k7GacGIas8zW9Q==@sourceware.org X-Gm-Message-State: AOJu0Yzu+wSofLv0ufo+G7bSDytBeVR2kiZ9m2t8IYQgpPdAMacxQB+3 WkpXPTc/7g/UQhY5mHd+JaxRPLpfUhFeWjc/XVtwRwKR8nq76bJtFPBLxT6v5t1ufiK0LtxOOcQ EU5I/E+tZbZrkMhh+1q8YqoZg/I/VAS5Lg7QMBO5dB896bCGF3AGsN/puAeY= X-Gm-Gg: ASbGncsRoDyqhDj2Uq/WVb9NdQ9Zfvfv5VLPZdRKuxSdMThOEZEjWha8oN5Ng9d+oWO ghecGY9S4RuhOP/Dnedv+7qfo6J7pZmmItli8fe7RQMhekyB2TDodqyvtDjDky7+g0IjfZjV5d3 43lHudFL7SC7g688UkfdkpxQMniZNWMYjcLJGfPhhopwPYXl+k2IMpsXudKnHVlmmYN1VJHduhs jd3RY/+fSMKTLwBXWUndkVuMjPxAIVhqp4Te/0fhZlAr3eV3zMV9FF+rH29AEEoLlg74r8Z4pkn d95KwQ== X-Received: by 2002:a5d:6da6:0:b0:385:f573:1f78 with SMTP id ffacd0b85a97d-38a8730abc6mr27189234f8f.24.1737029193909; Thu, 16 Jan 2025 04:06:33 -0800 (PST) X-Google-Smtp-Source: AGHT+IFb2goSBGUZMueJ+z0ElE3XfYTMUBk2N6DaMeyc3b+MiA2a7klXaImoTNlkIEApuuQOtbFQNg== X-Received: by 2002:a5d:6da6:0:b0:385:f573:1f78 with SMTP id ffacd0b85a97d-38a8730abc6mr27189168f8f.24.1737029192930; Thu, 16 Jan 2025 04:06:32 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-437c73e370fsm57912405e9.0.2025.01.16.04.06.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Jan 2025 04:06:32 -0800 (PST) From: Andrew Burgess To: Guinevere Larsen , gdb-patches@sourceware.org Cc: Guinevere Larsen , Eli Zaretskii , Thiago Jung Bauermann Subject: Re: [PATCH v8 4/5] gdb: introduce ability to disable frame unwinders In-Reply-To: <20241210195115.3046370-5-guinevere@redhat.com> References: <20241210195115.3046370-1-guinevere@redhat.com> <20241210195115.3046370-5-guinevere@redhat.com> Date: Thu, 16 Jan 2025 12:06:31 +0000 Message-ID: <87a5brotdk.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: snMuASiroROy1-YYEA_1e2ApbYjaUmJfA8TCrOgCLRY_1737029195 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Guinevere Larsen writes: > Sometimes, in the GDB testsuite, we want to test the ability of specific > unwinders to handle some piece of code. Usually this is done by trying > to outsmart GDB, or by coercing the compiler to remove information that > GDB would rely on. Both approaches have problems as GDB gets smarter > with time, and that compilers might differ in version and behavior, or > simply introduce new useful information. This was requested back in 2003 > in PR backtrace/8434. > > To improve our ability to thoroughly test GDB, this patch introduces a > new maintenance command that allows a user to disable some unwinders, > based on either the name of the unwinder or on its class. With this > change, it will now be possible for GDB to not find any frame unwinders > for a given frame, which would previously cause GDB to assert. GDB will > now check if any frame unwinder has been disabled, and if some has, it > will just error out instead of asserting. > > Unwinders can be disabled or re-enabled in 3 different ways: > * Disabling/enabling all at once (using '-all'). > * By specifying an unwinder class to be disabled (option '-class'). > * By specifying the name of an unwinder (option '-name'). > > If you give no options to the command, GDB assumes the input is an > unwinder class. '-class' would make no difference if used, is just here > for completeness. > > This command is meant to be used once the inferior is already at the > desired location for the test. An example session would be: > > (gdb) start > Temporary breakpoint 1, main () at omp.c:17 > 17 func(); > (gdb) maint frame-unwinder disable ARCH > (gdb) bt > \#0 main () at omp.c:17 > (gdb) maint frame-unwinder enable ARCH > (gdb) cont > Continuing. > > This commit is a more generic version of commit 3c3bb0580be0, > and so, based on the final paragraph of the commit message: > gdb: Add switch to disable DWARF stack unwinders > <...> > If in the future we find ourselves adding more switches to disable > different unwinders, then we should probably move to a more generic > solution, and remove this patch. > this patch also reverts 3c3bb0580be0 > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=8434 > Reviewed-By: Eli Zaretskii > Reviewed-by: Thiago Jung Bauermann > --- > gdb/NEWS | 17 ++ > gdb/doc/gdb.texinfo | 49 ++-- > gdb/dwarf2/frame-tailcall.c | 3 - > gdb/dwarf2/frame.c | 30 --- > gdb/dwarf2/frame.h | 6 - > gdb/frame-unwind.c | 237 +++++++++++++++++- > gdb/frame-unwind.h | 9 + > .../gdb.base/frame-info-consistent.exp | 8 +- > gdb/testsuite/gdb.base/frame-unwind-disable.c | 22 ++ > .../gdb.base/frame-unwind-disable.exp | 137 ++++++++++ > gdb/testsuite/gdb.base/maint.exp | 4 - > 11 files changed, 437 insertions(+), 85 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/frame-unwind-disable.c > create mode 100644 gdb/testsuite/gdb.base/frame-unwind-disable.exp > > diff --git a/gdb/NEWS b/gdb/NEWS > index 245b355669a..ef3317c11cb 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -125,6 +125,13 @@ maintenance info blocks [ADDRESS] > are listed starting at the inner global block out to the most inner > block. > > +maintenance frame-unwinder disable [-all | -name NAME | [-class] CLASS] > +maintenance frame-unwinder enable [-all | -name NAME | [-class] CLASS] > + Enable or disable frame unwinders. This is only meant to be used when > + testing unwinders themselves, and you want to ensure that a fallback > + algorithm won't obscure a regression. GDB is not expected to behave > + well if you try to execute the inferior with unwinders disabled. > + > info missing-objfile-handlers > List all the registered missing-objfile handlers. > > @@ -167,6 +174,16 @@ maintenance print remote-registers > mainenance info frame-unwinders > Add a CLASS column to the output. This class is a somewhat arbitrary > grouping of unwinders, based on which area of GDB adds the unwinder. > + Also add an ENABLED column, that will show if the unwinder is enabled > + or not. > + > +maintenance set dwarf unwinders (on|off) > + This command has been removed because the same functionality can be > + achieved with maint frame-unwinder (enable|disable) DEBUGINFO. > + > +maintenance show dwarf unwinders > + This command has been removed since the functionality can be achieved > + by checking the last column of maint info frame-unwinders. > > show configuration > Now includes the version of GNU Readline library that GDB is using. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 115c1f46b7f..10e6654a03b 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -42210,31 +42210,6 @@ On hosts without threading, or where worker threads have been disabled > at runtime, this setting has no effect, as DWARF reading is always > done on the main thread, and is therefore always synchronous. > > -@kindex maint set dwarf unwinders > -@kindex maint show dwarf unwinders > -@item maint set dwarf unwinders > -@itemx maint show dwarf unwinders > -Control use of the DWARF frame unwinders. > - > -@cindex DWARF frame unwinders > -Many targets that support DWARF debugging use @value{GDBN}'s DWARF > -frame unwinders to build the backtrace. Many of these targets will > -also have a second mechanism for building the backtrace for use in > -cases where DWARF information is not available, this second mechanism > -is often an analysis of a function's prologue. > - > -In order to extend testing coverage of the second level stack > -unwinding mechanisms it is helpful to be able to disable the DWARF > -stack unwinders, this can be done with this switch. > - > -In normal use of @value{GDBN} disabling the DWARF unwinders is not > -advisable, there are cases that are better handled through DWARF than > -prologue analysis, and the debug experience is likely to be better > -with the DWARF frame unwinders enabled. > - > -If DWARF frame unwinders are not supported for a particular target > -architecture, then enabling this flag does not cause them to be used. > - > @kindex maint info frame-unwinders > @item maint info frame-unwinders > List the frame unwinders currently in effect, starting with the highest > @@ -42252,6 +42227,30 @@ Unwinders installed by debug information readers. > Unwinders installed by the architecture specific code. > @end table > > +@kindex maint frame-unwinder disable > +@kindex maint frame-unwinder enable > +@item maint frame-unwinder disable [@code{-all} | @code{-name} @var{name} | [@code{-class}] @var{class}] > +@item maint frame-unwinder enable [@code{-all} | @code{-name} @var{name} | [@code{-class}] @var{class}] > +Disable or enable frame unwinders. This is meant only as a testing tool, and > +@value{GDBN} is not guaranteed to work properly if it is unable to find > +valid frame unwinders. > + > +The meaning of each of the invocations are as follows: > + > +@table @samp > +@item @code{-all} > +Disable or enable all unwinders. > +@item @code{-name} @var{name} > +@var{name} is the exact name of the unwinder to be disabled or enabled. > +@item [@code{-class}] @var{class} > +@var{class} is the class of frame unwinders to be disabled or enabled. > +The class may include the prefix @code{FRAME_UNWINDER_}, but it is not > +required. > +@end table > + > +@var{name} and @var{class} are always case insensitive. If no option > +starting wth @code{-} is given, @value{GDBN} assumes @code{-class}. > + > @kindex maint set worker-threads > @kindex maint show worker-threads > @item maint set worker-threads > diff --git a/gdb/dwarf2/frame-tailcall.c b/gdb/dwarf2/frame-tailcall.c > index 54813d00b03..2d7ab740f62 100644 > --- a/gdb/dwarf2/frame-tailcall.c > +++ b/gdb/dwarf2/frame-tailcall.c > @@ -321,9 +321,6 @@ tailcall_frame_sniffer (const struct frame_unwind *self, > int next_levels; > struct tailcall_cache *cache; > > - if (!dwarf2_frame_unwinders_enabled_p) > - return 0; > - > /* Inner tail call element does not make sense for a sentinel frame. */ > next_frame = get_next_frame (this_frame); > if (next_frame == NULL) > diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c > index 85e1d59bc09..e0e8eb5dbec 100644 > --- a/gdb/dwarf2/frame.c > +++ b/gdb/dwarf2/frame.c > @@ -183,9 +183,6 @@ static ULONGEST read_encoded_value (struct comp_unit *unit, gdb_byte encoding, > unrelocated_addr func_base); > > > -/* See dwarf2/frame.h. */ > -bool dwarf2_frame_unwinders_enabled_p = true; > - > /* Store the length the expression for the CFA in the `cfa_reg' field, > which is unused in that case. */ > #define cfa_exp_len cfa_reg > @@ -1306,9 +1303,6 @@ static int > dwarf2_frame_sniffer (const struct frame_unwind *self, > const frame_info_ptr &this_frame, void **this_cache) > { > - if (!dwarf2_frame_unwinders_enabled_p) > - return 0; > - > /* Grab an address that is guaranteed to reside somewhere within the > function. get_frame_pc(), with a no-return next function, can > end up returning something past the end of this function's body. > @@ -2253,34 +2247,10 @@ dwarf2_build_frame_info (struct objfile *objfile) > set_comp_unit (objfile, unit.release ()); > } > > -/* Handle 'maintenance show dwarf unwinders'. */ > - > -static void > -show_dwarf_unwinders_enabled_p (struct ui_file *file, int from_tty, > - struct cmd_list_element *c, > - const char *value) > -{ > - gdb_printf (file, > - _("The DWARF stack unwinders are currently %s.\n"), > - value); > -} > - > void _initialize_dwarf2_frame (); > void > _initialize_dwarf2_frame () > { > - add_setshow_boolean_cmd ("unwinders", class_obscure, > - &dwarf2_frame_unwinders_enabled_p , _("\ > -Set whether the DWARF stack frame unwinders are used."), _("\ > -Show whether the DWARF stack frame unwinders are used."), _("\ > -When enabled the DWARF stack frame unwinders can be used for architectures\n\ > -that support the DWARF unwinders. Enabling the DWARF unwinders for an\n\ > -architecture that doesn't support them will have no effect."), > - NULL, > - show_dwarf_unwinders_enabled_p, > - &set_dwarf_cmdlist, > - &show_dwarf_cmdlist); > - > #if GDB_SELF_TEST > selftests::register_test_foreach_arch ("execute_cfa_program", > selftests::execute_cfa_program_test); > diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h > index 2167310fbdf..fe2d669a983 100644 > --- a/gdb/dwarf2/frame.h > +++ b/gdb/dwarf2/frame.h > @@ -198,12 +198,6 @@ struct dwarf2_frame_state > bool armcc_cfa_offsets_reversed = false; > }; > > -/* When this is true the DWARF frame unwinders can be used if they are > - registered with the gdbarch. Not all architectures can or do use the > - DWARF unwinders. Setting this to true on a target that does not > - otherwise support the DWARF unwinders has no effect. */ > -extern bool dwarf2_frame_unwinders_enabled_p; > - > /* Set the architecture-specific register state initialization > function for GDBARCH to INIT_REG. */ > > diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c > index 3ab6af1c4da..968bb846402 100644 > --- a/gdb/frame-unwind.c > +++ b/gdb/frame-unwind.c > @@ -29,6 +29,7 @@ > #include "gdbarch.h" > #include "dwarf2/frame-tailcall.h" > #include "cli/cli-cmds.h" > +#include "cli/cli-option.h" > #include "inferior.h" > > /* Conversion list between the enum for frame_unwind_class and > @@ -89,6 +90,20 @@ frame_unwinder_class_str (frame_unwind_class uclass) > return unwind_class_conversion[uclass]; > } > > +/* Case insensitive search for a frame_unwind_class based on the given > + string. */ > +static enum frame_unwind_class > +str_to_frame_unwind_class (const char *class_str) > +{ > + for (int i = 0; i < UNWIND_CLASS_NUMBER; i++) > + { > + if (strcasecmp (unwind_class_conversion[i], class_str) == 0) > + return (frame_unwind_class) i; > + } > + > + error (_("Unknown frame unwind class: %s"), class_str); > +} > + > void > frame_unwind_prepend_unwinder (struct gdbarch *gdbarch, > const struct frame_unwind *unwinder) > @@ -175,27 +190,46 @@ frame_unwind_find_by_frame (const frame_info_ptr &this_frame, void **this_cache) > FRAME_SCOPED_DEBUG_ENTER_EXIT; > frame_debug_printf ("this_frame=%d", frame_relative_level (this_frame)); > > - const struct frame_unwind *unwinder_from_target; > + /* If we see a disabled unwinder, we assume some test is being run on > + GDB, and we don't want to internal_error at the end of this function. */ > + bool seen_disabled_unwinder = false; > + /* Lambda to factor out the logic of checking if an unwinder is enabled, > + testing it and otherwise recording if we saw a disable unwinder. */ > + auto test_unwinder = [&] (const struct frame_unwind *unwinder) > + { > + if (unwinder == nullptr) > + return false; > + > + if (!unwinder->enabled ()) > + { > + seen_disabled_unwinder = true; > + return false; > + } > + > + return frame_unwind_try_unwinder (this_frame, > + this_cache, > + unwinder); > + }; > > - unwinder_from_target = target_get_unwinder (); > - if (unwinder_from_target != NULL > - && frame_unwind_try_unwinder (this_frame, this_cache, > - unwinder_from_target)) > + if (test_unwinder (target_get_unwinder ())) > return; > > - unwinder_from_target = target_get_tailcall_unwinder (); > - if (unwinder_from_target != NULL > - && frame_unwind_try_unwinder (this_frame, this_cache, > - unwinder_from_target)) > + if (test_unwinder (target_get_tailcall_unwinder ())) > return; > > struct gdbarch *gdbarch = get_frame_arch (this_frame); > std::vector *table = get_frame_unwind_table (gdbarch); > for (auto unwinder : *table) > - if (frame_unwind_try_unwinder (this_frame, this_cache, unwinder)) > - return; > + { > + if (test_unwinder (unwinder)) > + return; > + } > > - internal_error (_("frame_unwind_find_by_frame failed")); > + if (seen_disabled_unwinder) > + error (_("Required frame unwinder may have been disabled" > + ", see 'maint info frame-unwinders'")); > + else > + internal_error (_("frame_unwind_find_by_frame failed")); > } > > /* A default frame sniffer which always accepts the frame. Used by > @@ -394,10 +428,11 @@ maintenance_info_frame_unwinders (const char *args, int from_tty) > std::vector *table = get_frame_unwind_table (gdbarch); > > ui_out *uiout = current_uiout; > - ui_out_emit_table table_emitter (uiout, 3, -1, "FrameUnwinders"); > + ui_out_emit_table table_emitter (uiout, 4, -1, "FrameUnwinders"); > uiout->table_header (27, ui_left, "name", "Name"); > uiout->table_header (25, ui_left, "type", "Type"); > uiout->table_header (9, ui_left, "class", "Class"); > + uiout->table_header (8, ui_left, "enabled", "Enabled"); > uiout->table_body (); > > for (auto unwinder : *table) > @@ -407,10 +442,145 @@ maintenance_info_frame_unwinders (const char *args, int from_tty) > uiout->field_string ("type", frame_type_str (unwinder->type ())); > uiout->field_string ("class", frame_unwinder_class_str ( > unwinder->unwinder_class ())); > + uiout->field_string ("enabled", unwinder->enabled () ? "Y" : "N"); > uiout->text ("\n"); > } > } > > +/* Options for disabling frame unwinders. */ > +struct maint_frame_unwind_options > +{ > + std::string unwinder_name; > + const char *unwinder_class = nullptr; > + bool all = false; > +}; > + > +static const gdb::option::option_def maint_frame_unwind_opt_defs[] = { > + > + gdb::option::flag_option_def { > + "all", > + [] (maint_frame_unwind_options *opt) { return &opt->all; }, > + nullptr, > + N_("Change the state of all unwinders") > + }, > + gdb::option::string_option_def { > + "name", > + [] (maint_frame_unwind_options *opt) { return &opt->unwinder_name; }, > + nullptr, > + N_("The name of the unwinder to have its state changed") > + }, > + gdb::option::enum_option_def { > + "class", > + unwind_class_conversion, > + [] (maint_frame_unwind_options *opt) { return &opt->unwinder_class; }, > + nullptr, > + N_("The class of unwinders to have their states changed") > + } > +}; > + > +static inline gdb::option::option_def_group > +make_frame_unwind_enable_disable_options (maint_frame_unwind_options *opts) > +{ > + return {{maint_frame_unwind_opt_defs}, opts}; > +} > + > +/* Helper function to both enable and disable frame unwinders. > + If ENABLE is true, this call will be enabling unwinders, > + otherwise the unwinders will be disabled. */ > +static void > +enable_disable_frame_unwinders (const char *args, int from_tty, bool enable) > +{ > + if (args == nullptr) > + { > + if (enable) > + error (_("Specify which frame unwinder(s) should be enabled")); > + else > + error (_("Specify which frame unwinder(s) should be disabled")); > + } > + > + struct gdbarch* gdbarch = current_inferior ()->arch (); > + std::vector *unwinder_list > + = get_frame_unwind_table (gdbarch); > + > + maint_frame_unwind_options opts; > + gdb::option::process_options > + (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, > + make_frame_unwind_enable_disable_options (&opts)); > + > + if ((opts.all && !opts.unwinder_name.empty ()) > + || (opts.all && opts.unwinder_class != nullptr) > + || (!opts.unwinder_name.empty () && opts.unwinder_class != nullptr)) > + error (_("Options are mutually exclusive")); > + > + /* First see if the user wants to change all unwinders. */ > + if (opts.all) > + { > + for (const frame_unwind *u : *unwinder_list) > + u->set_enabled (enable); > + > + reinit_frame_cache (); > + return; > + } > + > + /* If user entered a specific unwinder name, handle it here. If the > + unwinder is already at the expected state, error out. */ > + if (!opts.unwinder_name.empty ()) > + { > + bool did_something = false; > + for (const frame_unwind *unwinder : *unwinder_list) > + { > + if (strcasecmp (unwinder->name (), > + opts.unwinder_name.c_str ()) == 0) > + { > + if (unwinder->enabled () == enable) > + { > + if (unwinder->enabled ()) > + error (_("unwinder %s is already enabled"), > + unwinder->name ()); > + else > + error (_("unwinder %s is already disabled"), > + unwinder->name ()); > + } > + unwinder->set_enabled (enable); > + > + did_something = true; > + break; > + } > + } > + if (!did_something) > + error (_("couldn't find unwinder named %s"), > + opts.unwinder_name.c_str ()); > + } > + else > + { > + if (opts.unwinder_class == nullptr) > + opts.unwinder_class = args; > + enum frame_unwind_class dclass = str_to_frame_unwind_class > + (opts.unwinder_class); > + for (auto unwinder: *unwinder_list) I think you should stick with the form you use earlier: (const frame_unwind *unwinder : *unwinder_list) But if you really want to stick with auto, then I think: (const auto &unwinder : *unwinder_list) would be OK. > + { > + if (unwinder->unwinder_class () == dclass) > + unwinder->set_enabled (enable); > + } > + } > + > + reinit_frame_cache (); > +} > + > +/* Implement "maint frame-unwinder disable" command. */ > +static void > +maintenance_disable_frame_unwinders (const char *args, int from_tty) > +{ > + enable_disable_frame_unwinders (args, from_tty, false); > +} > + > +/* Implement "maint frame-unwinder enable" command. */ > +static void > +maintenance_enable_frame_unwinders (const char *args, int from_tty) > +{ > + enable_disable_frame_unwinders (args, from_tty, true); > +} > + > void _initialize_frame_unwind (); > void > _initialize_frame_unwind () > @@ -423,4 +593,45 @@ _initialize_frame_unwind () > List the frame unwinders currently in effect.\n\ > Unwinders are listed starting with the highest priority."), > &maintenanceinfolist); > + > + /* Add "maint frame-unwinder disable/enable". */ > + static struct cmd_list_element *maint_frame_unwinder; > + > + add_basic_prefix_cmd ("frame-unwinder", class_maintenance, > + _("Commands handling frame unwinders."), > + &maint_frame_unwinder, 0, &maintenancelist); > + > + add_cmd ("disable", class_maintenance, maintenance_disable_frame_unwinders, > + _("\ > +Disable one or more frame unwinder(s).\n\ > +Usage: maint frame-unwinder disable [-all | -name NAME | [-class] CLASS]\n\ > +\n\ > +These are the meanings of the options:\n\ > +\t-all - All available unwinders will be disabled\n\ > +\t-name - NAME is the exact name of the frame unwinder to be disabled\n\ > +\t-class - CLASS is the class of unwinders to be disabled. Valid classes are:\n\ > +\t\tGDB - Unwinders added by GDB core;\n\ > +\t\tEXTENSION - Unwinders added by extension languages;\n\ > +\t\tDEBUGINFO - Unwinders that handle debug information;\n\ > +\t\tARCH - Unwinders that use architecture-specific information;\n\ > +\n\ > +UNWINDER and NAME are case insensitive."), > + &maint_frame_unwinder); > + > + add_cmd ("enable", class_maintenance, maintenance_enable_frame_unwinders, > + _("\ > +Enable one or more frame unwinder(s).\n\ > +Usage: maint frame-unwinder enable [-all | -name NAME | [-class] CLASS]\n\ > +\n\ > +These are the meanings of the options:\n\ > +\t-all - All available unwinders will be enabled\n\ > +\t-name - NAME is the exact name of the frame unwinder to be enabled\n\ > +\t-class - CLASS is the class of unwinders to be enabled. Valid classes are:\n\ > +\t\tGDB - Unwinders added by GDB core;\n\ > +\t\tEXTENSION - Unwinders added by extension languages;\n\ > +\t\tDEBUGINFO - Unwinders that handle debug information;\n\ > +\t\tARCH - Unwinders that use architecture-specific information;\n\ > +\n\ > +UNWINDER and NAME are case insensitive."), > + &maint_frame_unwinder); > } At the bottom of this email you'll find a small patch which applies on top of this and adds some basic command completion. It doesn't complete the NAMEs, I'm not sure if there's a good way to do that (yet). It also doesn't switch the help text to use the %OPTIONS% mechanism, I tried that, but I think there may be some bugs with the formatting (I'll look into it), so I've left that off. You don't have to, we can add completion later, but I do think it would be good to merge the patch below with this one. But it is up to you. Take a look and see what you think. > diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h > index 13f18618d24..adf8b47982c 100644 > --- a/gdb/frame-unwind.h > +++ b/gdb/frame-unwind.h > @@ -192,6 +192,12 @@ class frame_unwind > const frame_data *unwind_data () const > { return m_unwind_data; } > > + bool enabled () const > + { return m_enabled; } > + > + void set_enabled (bool state) const > + { m_enabled = state; } > + > /* Default stop_reason implementation. It reports NO_REASON, unless the > frame is the outermost. */ > > @@ -245,6 +251,9 @@ class frame_unwind > frame_unwind_class m_unwinder_class; > > const frame_data *m_unwind_data; > + > + /* Whether this unwinder can be used when sniffing. */ > + mutable bool m_enabled = true; > }; > > /* This is a legacy version of the frame unwinder. The original struct > diff --git a/gdb/testsuite/gdb.base/frame-info-consistent.exp b/gdb/testsuite/gdb.base/frame-info-consistent.exp > index fe0cfad95bc..4f483111a91 100644 > --- a/gdb/testsuite/gdb.base/frame-info-consistent.exp > +++ b/gdb/testsuite/gdb.base/frame-info-consistent.exp > @@ -95,11 +95,11 @@ proc compare_frames {frames} { > } > } > > -proc test {dwarf_unwinder} { > +proc test {enable} { > > clean_restart $::binfile > > - gdb_test_no_output "maint set dwarf unwinder $dwarf_unwinder" > + gdb_test_no_output "maint frame-unwinder $enable DEBUGINFO" > > if {![runto_main]} { > return 0 > @@ -134,6 +134,6 @@ proc test {dwarf_unwinder} { > } > } > > -foreach_with_prefix dwarf {"off" "on"} { > - test $dwarf > +foreach_with_prefix action {"disable" "enable"} { > + test $action > } > diff --git a/gdb/testsuite/gdb.base/frame-unwind-disable.c b/gdb/testsuite/gdb.base/frame-unwind-disable.c > new file mode 100644 > index 00000000000..bbcfb01316e > --- /dev/null > +++ b/gdb/testsuite/gdb.base/frame-unwind-disable.c > @@ -0,0 +1,22 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2024 Free Software Foundation, Inc. Remember to update the copyright date when rebasing. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program 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 General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +int > +main () > +{ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/frame-unwind-disable.exp b/gdb/testsuite/gdb.base/frame-unwind-disable.exp > new file mode 100644 > index 00000000000..f4cad9a47fd > --- /dev/null > +++ b/gdb/testsuite/gdb.base/frame-unwind-disable.exp > @@ -0,0 +1,137 @@ > +# Copyright 2024 Free Software Foundation, Inc. And again with the date. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program 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 General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Test multiple situations in which we may use the maintenance command to > +# disable and enable frame unwinders, and check that they really are > +# disabled when they say the are. > + > +standard_testfile .c You can drop '.c' here, it's the default. > + > +# Proc to check if the unwinder of the given name is in the desired state. > +# STATE can be either Y or N. > +proc check_unwinder_state { unwinder_name state {testname ""} } { > + set should_pass false > + set command "maint info frame-unwinders" > + if {${testname} == ""} { > + set testname "checking state ${state} for ${unwinder_name}" > + } > + gdb_test_multiple "${command}" "${testname}" -lbl { > + -re "${unwinder_name}\\s+\\w+\\s+\\w+\\s+${state}\\s+(?=\r\n)" { > + set should_pass true > + exp_continue > + } > + -re "${command}" { > + exp_continue > + } > + -re "\\w+\\s+\\w+\\s+\\w+\\s+\\w+\\s+(?=\r\n)" { > + exp_continue > + } > + -re -wrap "" { > + gdb_assert ${should_pass} "${gdb_test_name}" I don't think the quotes around "${gdb_test_name}" are needed here? In fact, there seem to be a few places where you place quotes around expansion of a single variable (e.g. "${command}" on the gdb_test_multiple line), I don't think any of these are needed. With these changes (and with or without the completion patch): Approved-By: Andrew Burgess Thanks, Andrew > + } > + } > +} > + > +# Check if all unwinders of class UNWINDER_CLASS are in the state STATE. > +# STATE can be either Y or N. > +# UNWINDER_CLASS can be one of: GDB, ARCH, EXTENSION, DEBUGINFO. It can > +# also be \\w+ if checking all unwinders. > +proc check_unwinder_class { unwinder_class state {testname ""} } { > + set command "maint info frame-unwinders" > + set should_pass true > + if {$testname == ""} { > + set testname "checking if ${unwinder_class} state is ${state}" > + } > + gdb_test_multiple "${command}" "${testname}" -lbl { > + -re "^\[^\r\n\]+\\s+\\w+\\s+${unwinder_class}\\s+\(\[YN\]\)\\s+(?=\r\n)" { > + # The unwinder name may have multiple words, so we need to use the > + # more generic [^\r\n] pattern to match the unwinders. > + set cur_state $expect_out(1,string) > + if {$cur_state == $state} { > + set should_pass false > + } > + exp_continue > + } > + -re "${command}" { > + exp_continue > + } > + -re -wrap "" { > + gdb_assert ${should_pass} "${gdb_test_name}" > + } > + } > +} > + > +if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} { > + return -1 > +} > + > +if {![runto_main]} { > + untested "couldn't run to main" > + return > +} > + > +# Test disabling all unwinders. > +check_unwinder_class "\\w+" "Y" \ > + "all unwinders enabled before any changes" > +gdb_test_no_output "maint frame-unwinder disable -all" > +check_unwinder_class "\\w+" "N" \ > + "all unwinders were properly disabled" > + > +# Test if GDB can still make a backtrace once all unwinders are disabled. > +# It should be impossible. > +gdb_test "backtrace" \ > + ".*Required frame unwinder may have been disabled, see 'maint info frame-unwinders'.*" \ > + "no suitable unwinders should be found" > + > +# Reenable all unwinders. > +gdb_test_no_output "maint frame-unwinder enable -all" > +check_unwinder_class "\\w+" "Y" \ > + "all unwinders should be re-enabled" > + > +# Check that we are able to get backtraces once again. > +gdb_test "backtrace" ".0\\s+main .. at.*" \ > + "we can get usable backtraces again" > + > +# Check if we can disable an unwinder based on the name. > +check_unwinder_state "dummy" "Y" > +gdb_test_no_output "maint frame-unwinder disable -name dummy" > +check_unwinder_state "dummy" "N" > +# And verify what happens if you try to disable it again. > +gdb_test "maint frame-unwinder disable -name dummy" \ > + "unwinder dummy is already disabled" \ > + "disable already disabled unwinder" > +check_unwinder_state "dummy" "N" "dummy should continue disabled" > + > +foreach class {GDB ARCH DEBUGINFO EXTENSION} { > + # Disable all unwinders of type CLASS, and check that the command worked. > + gdb_test_no_output "maint frame-unwinder disable ${class}" > + check_unwinder_class "${class}" "N" > +} > + > +# Now check if we are able to enable a single unwinder, and what happens if we > +# enable it twice. > +gdb_test_no_output "maint frame-unwinder enable -name dummy" > +check_unwinder_state "dummy" "Y" "successfully enabled dummy unwinder" > +gdb_test "maint frame-unwinder enable -name dummy" \ > + "unwinder dummy is already enabled" \ > + "enable already enabled unwinder" > +check_unwinder_state "dummy" "Y" "dummy should continue enabled" > + > +foreach class {GDB ARCH DEBUGINFO EXTENSION} { > + # Enable all unwinders of type CLASS, and check that the command worked, > + # using "-class" option to ensure it works. Should make no difference. > + gdb_test_no_output "maint frame-unwinder enable -class ${class}" > + check_unwinder_class "${class}" "Y" > +} > diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp > index 2c58ffa36c5..ae7ad0787e6 100644 > --- a/gdb/testsuite/gdb.base/maint.exp > +++ b/gdb/testsuite/gdb.base/maint.exp > @@ -454,10 +454,6 @@ gdb_test_no_output "maint info line-table xxx.c" \ > > set timeout $oldtimeout > > -# Just check that the DWARF unwinders control flag is visible. > -gdb_test "maint show dwarf unwinders" \ > - "The DWARF stack unwinders are currently (on|off)\\." > - > #============test help on maint commands > > test_prefix_command_help {"maint info" "maintenance info"} { > -- > 2.47.0 --- diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c index 1417651108f..f3019b7ec18 100644 --- a/gdb/frame-unwind.c +++ b/gdb/frame-unwind.c @@ -40,6 +40,7 @@ static const char * unwind_class_conversion[] = "EXTENSION", "DEBUGINFO", "ARCH", + nullptr }; /* Default sniffers, that must always be the first in the unwinder list, @@ -579,6 +580,29 @@ enable_disable_frame_unwinders (const char *args, int from_tty, bool enable) reinit_frame_cache (); } +/* Completer for the "maint frame-unwinder enable|disable" commands. */ + +static void +enable_disable_frame_unwinders_completer (struct cmd_list_element *ignore, + completion_tracker &tracker, + const char *text, + const char */*word*/) +{ + maint_frame_unwind_options opts; + const auto group = make_frame_unwind_enable_disable_options (&opts); + + const char *start = text; + if (gdb::option::complete_options + (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group)) + return; + + /* Only complete a class name as a stand-alone operand when no options + are given. */ + if (start == text) + complete_on_enum (tracker, unwind_class_conversion, text, text); + return; +} + /* Implement "maint frame-unwinder disable" command. */ static void maintenance_disable_frame_unwinders (const char *args, int from_tty) @@ -613,8 +637,9 @@ Unwinders are listed starting with the highest priority."), _("Commands handling frame unwinders."), &maint_frame_unwinder, 0, &maintenancelist); - add_cmd ("disable", class_maintenance, maintenance_disable_frame_unwinders, - _("\ + cmd_list_element *c + = add_cmd ("disable", class_maintenance, maintenance_disable_frame_unwinders, + _("\ Disable one or more frame unwinder(s).\n\ Usage: maint frame-unwinder disable [-all | -name NAME | [-class] CLASS]\n\ \n\ @@ -628,10 +653,13 @@ These are the meanings of the options:\n\ \t\tARCH - Unwinders that use architecture-specific information;\n\ \n\ UNWINDER and NAME are case insensitive."), - &maint_frame_unwinder); + &maint_frame_unwinder); + set_cmd_completer_handle_brkchars (c, + enable_disable_frame_unwinders_completer); - add_cmd ("enable", class_maintenance, maintenance_enable_frame_unwinders, - _("\ + c = + add_cmd ("enable", class_maintenance, maintenance_enable_frame_unwinders, + _("\ Enable one or more frame unwinder(s).\n\ Usage: maint frame-unwinder enable [-all | -name NAME | [-class] CLASS]\n\ \n\ @@ -645,5 +673,7 @@ These are the meanings of the options:\n\ \t\tARCH - Unwinders that use architecture-specific information;\n\ \n\ UNWINDER and NAME are case insensitive."), - &maint_frame_unwinder); + &maint_frame_unwinder); + set_cmd_completer_handle_brkchars (c, + enable_disable_frame_unwinders_completer); }