From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Z2aWA5pYFmBxYAAAWB0awg (envelope-from ) for ; Sun, 31 Jan 2021 02:13:30 -0500 Received: by simark.ca (Postfix, from userid 112) id E56F11EF80; Sun, 31 Jan 2021 02:13:29 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.3 required=5.0 tests=MAILING_LIST_MULTI,RDNS_NONE, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 533511E945 for ; Sun, 31 Jan 2021 02:13:29 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C11553851C3E; Sun, 31 Jan 2021 07:13:28 +0000 (GMT) Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 5C95E3857020 for ; Sun, 31 Jan 2021 07:13:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5C95E3857020 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 3D4FA1176E5 for ; Sun, 31 Jan 2021 02:13:25 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id bX57rjMKbIe5 for ; Sun, 31 Jan 2021 02:13:25 -0500 (EST) Received: from float.home (localhost.localdomain [127.0.0.1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id CDEE8116B4C for ; Sun, 31 Jan 2021 02:13:24 -0500 (EST) Received: by float.home (Postfix, from userid 1000) id CE99CA188E; Sun, 31 Jan 2021 11:13:19 +0400 (+04) Date: Sun, 31 Jan 2021 11:13:19 +0400 From: Joel Brobecker To: Hannes Domani via Gdb-patches Subject: Re: [PATCH 4/4] Fix raw-frame-arguments in combination with frame-filters Message-ID: <20210131071319.GA78197@adacore.com> References: <20201229170227.821-1-ssbssa@yahoo.de> <20201229170227.821-4-ssbssa@yahoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201229170227.821-4-ssbssa@yahoo.de> 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: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi Hannes, On Tue, Dec 29, 2020 at 06:02:27PM +0100, Hannes Domani via Gdb-patches wrote: > Currently, if frame-filters are active, raw-values is used instead of > raw-frame-arguments to decide if a pretty-printer should be invoked for > frame arguments in a backtrace. > > This adds the PRINT_RAW_FRAME_ARGUMENTS flag to frame_filter_flag which is > then used in the frame-filter to override the raw flag in enumerate_args. > > gdb/ChangeLog: > > 2020-12-29 Hannes Domani > > * extension.h (enum frame_filter_flag): Add > PRINT_RAW_FRAME_ARGUMENTS. > * python/py-framefilter.c (enumerate_args): Override raw flag. > raw_frame_args argument. > (py_mi_print_variables): Forward raw flag. > (py_print_args): Forward raw_frame_args flag. > (py_print_frame): Handle PRINT_RAW_FRAME_ARGUMENTS. > * stack.c (backtrace_command_1): Set PRINT_RAW_FRAME_ARGUMENTS. > > gdb/testsuite/ChangeLog: > > 2020-12-29 Hannes Domani > > * gdb.python/py-frame-args.exp: Add bt raw-frame-arguments tests. > * gdb.python/py-frame-args.py: Add basic frame-filter. I'm sure someone more versed in the Python extension API would immediately understand what you mean. Unfortunately, we seeem to be in short supply of this at the moment, as highlighted by the lack of meaninful review. Simon seems to have tried to look at the patch, and so did I, and one barrier to the review is the fact that we need to understand what problem exactly you are trying to resolve. I think an example, with behavior behavior, why this is wrong, and behavior after, would go a long way towards lowering that barrier... In fact, speaking more generally, I think that commit messages that do not make any assumption as to the level of expertise of the person looking at the patch are always desirable, as they are more inclusive (being accessible to a larger population). If you can provide that, I will try to review your patch next weekend if someone else hasn't done so already. > --- > gdb/extension.h | 4 ++++ > gdb/python/py-framefilter.c | 14 ++++++++++---- > gdb/stack.c | 2 ++ > gdb/testsuite/gdb.python/py-frame-args.exp | 22 ++++++++++++++++++++++ > gdb/testsuite/gdb.python/py-frame-args.py | 13 +++++++++++++ > 5 files changed, 51 insertions(+), 4 deletions(-) > > diff --git a/gdb/extension.h b/gdb/extension.h > index c840dbc704..f3ec61b163 100644 > --- a/gdb/extension.h > +++ b/gdb/extension.h > @@ -103,6 +103,10 @@ enum frame_filter_flag > > /* Set this flag if elided frames should not be printed. */ > PRINT_HIDE = 1 << 5, > + > + /* Set this flag if pretty printers for frame arguments should not > + be invoked. */ > + PRINT_RAW_FRAME_ARGUMENTS = 1 << 6, > }; > > DEF_ENUM_FLAGS_TYPE (enum frame_filter_flag, frame_filter_flags); > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > index 944a0eee50..c61bfa4c2e 100644 > --- a/gdb/python/py-framefilter.c > +++ b/gdb/python/py-framefilter.c > @@ -422,12 +422,14 @@ static enum ext_lang_bt_status > enumerate_args (PyObject *iter, > struct ui_out *out, > enum ext_lang_frame_args args_type, > + bool raw_frame_args, > int print_args_field, > struct frame_info *frame) > { > struct value_print_options opts; > > get_user_print_options (&opts); > + opts.raw = raw_frame_args; > > if (args_type == CLI_SCALAR_VALUES) > { > @@ -655,8 +657,8 @@ py_mi_print_variables (PyObject *filter, struct ui_out *out, > ui_out_emit_list list_emitter (out, "variables"); > > if (args_iter != Py_None > - && (enumerate_args (args_iter.get (), out, args_type, 1, frame) > - == EXT_LANG_BT_ERROR)) > + && (enumerate_args (args_iter.get (), out, args_type, opts->raw, 1, > + frame) == EXT_LANG_BT_ERROR)) > return EXT_LANG_BT_ERROR; > > if (locals_iter != Py_None > @@ -701,6 +703,7 @@ static enum ext_lang_bt_status > py_print_args (PyObject *filter, > struct ui_out *out, > enum ext_lang_frame_args args_type, > + bool raw_frame_args, > struct frame_info *frame) > { > gdbpy_ref<> args_iter (get_py_iter_from_func (filter, "frame_args")); > @@ -726,7 +729,8 @@ py_print_args (PyObject *filter, > } > } > else if (args_iter != Py_None > - && (enumerate_args (args_iter.get (), out, args_type, 0, frame) > + && (enumerate_args (args_iter.get (), out, args_type, > + raw_frame_args, 0, frame) > == EXT_LANG_BT_ERROR)) > return EXT_LANG_BT_ERROR; > > @@ -957,7 +961,9 @@ py_print_frame (PyObject *filter, frame_filter_flags flags, > wrong. */ > if (print_args && (location_print || out->is_mi_like_p ())) > { > - if (py_print_args (filter, out, args_type, frame) == EXT_LANG_BT_ERROR) > + bool raw_frame_args = (flags & PRINT_RAW_FRAME_ARGUMENTS) != 0; > + if (py_print_args (filter, out, args_type, raw_frame_args, frame) > + == EXT_LANG_BT_ERROR) > return EXT_LANG_BT_ERROR; > } > > diff --git a/gdb/stack.c b/gdb/stack.c > index 943b3db087..07abdea8c1 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -2028,6 +2028,8 @@ backtrace_command_1 (const frame_print_options &fp_opts, > flags |= PRINT_LOCALS; > if (bt_opts.hide) > flags |= PRINT_HIDE; > + if (fp_opts.print_raw_frame_arguments) > + flags |= PRINT_RAW_FRAME_ARGUMENTS; > > if (!bt_opts.no_filters) > { > diff --git a/gdb/testsuite/gdb.python/py-frame-args.exp b/gdb/testsuite/gdb.python/py-frame-args.exp > index fd9c1f4342..7c621e1302 100644 > --- a/gdb/testsuite/gdb.python/py-frame-args.exp > +++ b/gdb/testsuite/gdb.python/py-frame-args.exp > @@ -34,6 +34,28 @@ gdb_test_no_output "source ${remote_python_file}" "load python file" > gdb_breakpoint [gdb_get_line_number "break-here"] > gdb_continue_to_breakpoint "break-here" ".* break-here .*" > > +# Test raw-frame-arguments on backtrace with and without frame-filter > +foreach filtered [list "enable" "disable"] { > + gdb_test_no_output "$filtered frame-filter global BasicFrameFilter" > + > + gdb_test "bt 1" \ > + ".*foo \\(x=42, ss=super struct = {\[.\]{3}}\\).*" \ > + "bt frame-filter=$filtered,pretty" > + > + gdb_test "bt -raw-frame-arguments on 1" \ > + ".*foo \\(x=42, ss=\[.\]{3}\\).*" \ > + "bt frame-filter=$filtered,raw" > + > + # "set print raw-values" should not affect frame arguments > + gdb_test_no_output "set print raw-values on" \ > + "raw-values-on,frame-filter=$filtered" > + gdb_test "bt 1" \ > + ".*foo \\(x=42, ss=super struct = {\[.\]{3}}\\).*" \ > + "bt frame-filter=$filtered,pretty,raw-values" > + gdb_test_no_output "set print raw-values off" \ > + "raw-values-off,frame-filter=$filtered" > +} > + > # Test all combinations with raw off. > > gdb_test_no_output "set print raw-frame-arguments off" > diff --git a/gdb/testsuite/gdb.python/py-frame-args.py b/gdb/testsuite/gdb.python/py-frame-args.py > index c2908829c5..aed2070d94 100644 > --- a/gdb/testsuite/gdb.python/py-frame-args.py > +++ b/gdb/testsuite/gdb.python/py-frame-args.py > @@ -73,3 +73,16 @@ pretty_printers_dict = {} > > register_pretty_printers () > gdb.pretty_printers.append (lookup_function) > + > + > +class BasicFrameFilter (object): > + def __init__ (self): > + self.name = "BasicFrameFilter" > + self.priority = 100 > + self.enabled = True > + gdb.frame_filters[self.name] = self > + > + def filter (self, frame_iter): > + return frame_iter > + > +BasicFrameFilter () > -- > 2.29.2 -- Joel