From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id 9405D385780C for ; Sat, 19 Mar 2022 14:41:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9405D385780C Received: by mail-wr1-x42e.google.com with SMTP id p9so15204695wra.12 for ; Sat, 19 Mar 2022 07:41:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=yZeEDY+pvhKY4Ytb7fyVF5WeF3hytX4Tx7Zd+TAW3uo=; b=SLIzoPEAK927W/XmpvZI5Ycq3EYZ4LRhJwN8ZbE/jqFRuOyRdv2J3PVEuuTKNAq49U q739givLA/5K8dK51Z6S05nO1d0YpeRd4tP6Hik3Mom/fyvfHNAX9Er1VBKP+HB61FBN WgMrJvYVpPnNv+bWamqvVXEcgg5IMfUirkhYMa9PIFmdJHspQNDnlcff9Pgq3OgjZtNg KX5RiAcmn1dz4imIZfZwKvzgphhLeiEBgcvy7w4EAstqKbsJvLLsBDtTRp5OC2lQ41k3 /eXC+hr7slaWTjgAoKuxEJhCUFf4Cl81YU4wulRouBhze1pxHoX2peLY/nq1z53DxlMu sR/A== X-Gm-Message-State: AOAM532iCs/aEaeNv8gKywRmobabQep1/DFqScFtV9WCDw1Z3d4NHizv Z9VwvdejWnykqlaxDSdbd77F4kS0Zw1W X-Google-Smtp-Source: ABdhPJwJcf+BgEEhdzdp+bEZMUk0sk77vcIZAhWADMRZT+P0DoxqA/7ZgcA/nGF0bZ8CM8HxGm9cxA== X-Received: by 2002:a5d:59ad:0:b0:203:99d0:fcc7 with SMTP id p13-20020a5d59ad000000b0020399d0fcc7mr11881071wrr.592.1647700893481; Sat, 19 Mar 2022 07:41:33 -0700 (PDT) Received: from takamaka.home ([2a01:cb22:1d5:1100:190c:401a:9e2d:8ecc]) by smtp.gmail.com with ESMTPSA id l12-20020a05600c4f0c00b0038be825b774sm10224395wmq.45.2022.03.19.07.41.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 19 Mar 2022 07:41:32 -0700 (PDT) Received: by takamaka.home (Postfix, from userid 1000) id 5B169A162A; Sat, 19 Mar 2022 18:41:29 +0400 (+04) Date: Sat, 19 Mar 2022 18:41:29 +0400 From: Joel Brobecker To: Bruno Larsen via Gdb-patches Cc: Joel Brobecker Subject: Re: [PATCH v3] [gdb/testsuite] test a function call by hand from pretty printer Message-ID: References: <20220223185729.25609-1-blarsen@redhat.com> <20220314203926.58662-1-blarsen@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220314203926.58662-1-blarsen@redhat.com> X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Sat, 19 Mar 2022 14:41:38 -0000 On Mon, Mar 14, 2022 at 05:39:26PM -0300, Bruno Larsen via Gdb-patches wrote: > The test case added here is testing the bug gdb/28856, where calling a > function by hand from a pretty printer makes GDB crash. There are 6 > mechanisms to trigger this crash in the current test, using the commands > backtrace, up, down, finish, step and continue. Since the failure happens > because of use-after-free (more details below) the tests will always > have a chance of passing through sheer luck, but anecdotally they seem > to fail all of the time. > > The reason GDB is crashing is a use-after-free problem. The above > mentioned functions save a pointer to the current frame's information, > then calls the pretty printer, and uses the saved pointer for different > reasons, depending on the function. The issue happens because > call_function_by_hand needs to reset the obstack to get the current > frame, invalidating the saved pointer. > --- > > Changes from v2: > * Make .c follow GDB's coding style 2 - electric boogaloo > * .exp tests outputs, not just if GDB breaks or not > * added copyright header to .py file > > Changes from v1: > * make .c follow GDB's coding guidelines more closely > * Documented .exp file better > * renamed and refactored TCL proc to make it less confusing Thanks you Bruno. This version of the patch looks good to me. > --- > .../gdb.python/pretty-print-call-by-hand.c | 53 ++++++++ > .../gdb.python/pretty-print-call-by-hand.exp | 127 ++++++++++++++++++ > .../gdb.python/pretty-print-call-by-hand.py | 41 ++++++ > 3 files changed, 221 insertions(+) > create mode 100644 gdb/testsuite/gdb.python/pretty-print-call-by-hand.c > create mode 100644 gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp > create mode 100644 gdb/testsuite/gdb.python/pretty-print-call-by-hand.py > > diff --git a/gdb/testsuite/gdb.python/pretty-print-call-by-hand.c b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.c > new file mode 100644 > index 00000000000..3be5675b096 > --- /dev/null > +++ b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.c > @@ -0,0 +1,53 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2022 Free Software Foundation, Inc. > + > + 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 . */ > + > +struct mytype > +{ > + char *x; > +}; > + > +void > +rec (int i) > +{ > + if (i <= 0) > + return; > + rec (i-1); > +} > + > +int > +f () > +{ > + rec (100); > + return 2; > +} > + > +void > +g (struct mytype mt, int depth) > +{ > + if (depth <= 0) > + return; /* TAG: final frame */ > + g (mt, depth - 1); /* TAG: first frame */ > +} > + > +int > +main () > +{ > + struct mytype mt; > + mt.x = "hello world"; > + g (mt, 10); /* TAG: outside the frame */ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp > new file mode 100644 > index 00000000000..0a6d014cf12 > --- /dev/null > +++ b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.exp > @@ -0,0 +1,127 @@ > +# Copyright (C) 2022 Free Software Foundation, Inc. > + > +# 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 . > + > +# This file is part of the GDB testsuite. It tests a pretty printer that > +# calls an inferior function by hand, triggering a Use-after-Free bug > +# (PR gdb/28856). > + > +load_lib gdb-python.exp > + > +standard_testfile > + > +# gdb needs to be started here for skip_python_tests to work. > +# prepare_for_testing could be used instead, but it could compile the program > +# unnecessarily, so starting GDB like this is preferable. > +gdb_start > + > +# Skip all tests if Python scripting is not enabled. > +if { [skip_python_tests] } { continue } > + > +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } { > + untested "failed to compile" > + return -1 > +} > + > +# This proc restarts GDB, makes the inferior reach the desired spot - marked > +# by a comment in the .c file - and turns on the pretty printer for testing. > +# Starting with a new GDB is important because the test may crash GDB. The > +# return values are here to avoid us trying to test the pretty printer if > +# there was a problem getting to main. > +proc start_test { breakpoint_comment } { > + global srcdir subdir testfile binfile > + > + # Start with a fresh gdb. > + # This is important because the test can crash GDB. > + > + clean_restart ${binfile} > + > + if { ![runto_main] } then { > + untested "couldn't run to breakpoint" > + return -1 > + } > + > + # Let GDB get to the return line. > + gdb_breakpoint [gdb_get_line_number ${breakpoint_comment} ${testfile}.c ] > + gdb_continue_to_breakpoint ${breakpoint_comment} ".*" > + > + gdb_test_no_output "set print pretty on" "starting to pretty print" > + > + set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] > + gdb_test_no_output "source ${remote_python_file}" "load python file" > + > + return 0 > +} > + > +# Testing the backtrace command. > +with_test_prefix "frame print" { > + if { [start_test "TAG: final frame"] == 0 } { > + setup_kfail gdb/28856 "*-*-*" > + gdb_test "backtrace -frame-arguments all" [multi_line \ > + "#0 .*g \\(mt=mytype is .*\\).*"\ > + "#1 .*g \\(mt=mytype is .*\\).*"\ > + "#2 .*g \\(mt=mytype is .*\\).*"\ > + "#3 .*g \\(mt=mytype is .*\\).*"\ > + "#4 .*g \\(mt=mytype is .*\\).*"\ > + "#5 .*g \\(mt=mytype is .*\\).*"\ > + "#6 .*g \\(mt=mytype is .*\\).*"\ > + "#7 .*g \\(mt=mytype is .*\\).*"\ > + "#8 .*g \\(mt=mytype is .*\\).*"\ > + "#9 .*g \\(mt=mytype is .*\\).*"\ > + "#10 .*g \\(mt=mytype is .*\\).*"\ > + "#11 .*main \\(\\).*"] \ > + "backtrace test" > + } > +} > +# Testing the down command. > +with_test_prefix "frame movement down" { > + if { [start_test "TAG: first frame"] == 0 } { > + gdb_test "up" [multi_line "#1 .*in main \\(\\) at .*" ".*outside the frame.*"] > + setup_kfail gdb/28856 "*-*-*" > + gdb_test "down" [multi_line "#0\\s+g \\(mt=mytype is .*\\).*" ".*first frame.*"] > + } > +} > + > +# Testing the up command. > +with_test_prefix "frame movement up" { > + if { [start_test "TAG: final frame"] == 0 } { > + setup_kfail gdb/28856 "*-*-*" > + gdb_test "up" [multi_line "#1 .*in g \\(mt=mytype is .*\\).*" ".*first frame.*"] > + } > +} > + > +# Testing the finish command. > +with_test_prefix "frame exit through finish" { > + if { [start_test "TAG: final frame"] == 0 } { > + setup_kfail gdb/28856 "*-*-*" > + gdb_test "finish" [multi_line "Run till exit from #0 .*" ".* g \\(mt=mytype is .*\\, depth=1\\).*" ".*first frame.*"] > + } > +} > + > +# Testing the step command. > +with_test_prefix "frame enter through step" { > + if { [start_test "TAG: outside the frame"] == 0 } { > + setup_kfail gdb/28856 "*-*-*" > + gdb_test "step" [multi_line "g \\(mt=mytype is .*\\, depth=10\\).*" "41.*if \\(depth \\<= 0\\)"] > + } > +} > + > +# Testing the continue command. > +with_test_prefix "frame enter through continue" { > + if { [start_test "TAG: outside the frame"] == 0 } { > + setup_kfail gdb/28856 "*-*-*" > + gdb_breakpoint [gdb_get_line_number "TAG: first frame" ${testfile}.c ] > + gdb_continue_to_breakpoint "TAG: first frame" ".*TAG: first frame.*" > + } > +} > diff --git a/gdb/testsuite/gdb.python/pretty-print-call-by-hand.py b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.py > new file mode 100644 > index 00000000000..f8f5df678f8 > --- /dev/null > +++ b/gdb/testsuite/gdb.python/pretty-print-call-by-hand.py > @@ -0,0 +1,41 @@ > +# Copyright (C) 2022 Free Software Foundation, Inc. > + > +# 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 . > + > + > +class MytypePrinter: > + """pretty print my type""" > + > + def __init__(self, val): > + self.val = val > + > + def to_string(self): > + calls = gdb.parse_and_eval('f()') > + return "mytype is %s" % self.val['x'] > + > +def ec_lookup_function(val): > + typ = val.type > + if typ.code == gdb.TYPE_CODE_REF: > + typ = typ.target() > + if str(typ) == 'struct mytype': > + return MytypePrinter(val) > + return None > + > +def disable_lookup_function(): > + ec_lookup_function.enabled = False > + > +def enable_lookup_function(): > + ec_lookup_function.enabled = True > + > +gdb.pretty_printers.append(ec_lookup_function) > -- > 2.31.1 > -- Joel