From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 5F0EE3939C12 for ; Wed, 17 Jun 2020 17:38:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5F0EE3939C12 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x431.google.com with SMTP id t18so3248616wru.6 for ; Wed, 17 Jun 2020 10:38:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=NwRMC5hGgMpb3EQ7x82LMarS2XT/fi76fzr0jG6whjU=; b=ArYS56ztesmtTwX8gY//LXD+R2HscgRmwmxp8hKvNTMgyKnFZKd0dPw0avnXgPHopL x/koz5OPlDN0yr9oQqLXclbgURda+k6rsFBT9P+dsxzMGH5x41jcPaFPheyZh0Xnxevm 1bRf9ZkecJd+4WRbmH6u1oksCdGH4NRZDjCgizPk/JSA/cJbfQi3N6uAdxBLsbTevlh5 2Ua0of1Bjuvbb3lMoQTd+Ib2agV9u2lLo7NgJnTNJZ6m8n7k/fcnki9jDlchpjPyuOji FsuvIrME6YTwaommaH8jTZIEaOjhAC7kJKe9kf3rue02ny3KwERfJAJrKiKTzltb8JS4 RJ0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=NwRMC5hGgMpb3EQ7x82LMarS2XT/fi76fzr0jG6whjU=; b=rySHF4hAQHJeBWyTDSqLI8S5N/mLbmsi0WP0GdqhPbp4sHp+kwCyg3ForEJfKNH86W QPIBu3hdlQjPfkwQCrJV79EbPP7//hEM2rRXwzbMfphVu7YIsO4MWQ6ziiKbFQEihPq4 nfUGuMMikV2sQwGwwEFbZ3RaW0C04SOCE1c7NbdlgbOU7swHJk6vaD4tXAnTJhj9sULJ CLdUj4EiWLXzhamFW0L3q4FykOjeOQKI2Wnno7B5Kdgd6jjSZotHnrnpzGOrjVKVUZDo qzhmacpVhLiITJeDjjRhdrMnQhiBxNmP+jTwjLat/3KjIus0ArtKGvtQhsSPx/qE1h7w zJkQ== X-Gm-Message-State: AOAM532qggk78C6rDNZZYJafabmpPmZhHuVIusY2+7n7KdbD4FHYfhuf IJhzDPoKTRkoiHyMpjCdZizsOPVdr4o= X-Google-Smtp-Source: ABdhPJxCWF1Gz/jixwC40Z64otjiVekO7+9/fEoEmeS1tD81R2wYWgHGBW7J0LqMECfJaX++T7qwng== X-Received: by 2002:adf:9dd0:: with SMTP id q16mr389595wre.410.1592415501916; Wed, 17 Jun 2020 10:38:21 -0700 (PDT) Received: from localhost (host86-128-12-16.range86-128.btcentralplus.com. [86.128.12.16]) by smtp.gmail.com with ESMTPSA id c2sm324460wrv.47.2020.06.17.10.38.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 10:38:21 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH 5/5] gdb: Fix Python unwinders and inline frames Date: Wed, 17 Jun 2020 18:38:09 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Wed, 17 Jun 2020 17:38:25 -0000 I observed that when making use of a Python frame unwinder, if the frame sniffing code accessed any registers within an inline frame then GDB would crash with this error: gdb/frame.c:579: internal-error: frame_id get_frame_id(frame_info*): Assertion `fi->level == 0' failed. The problem is that, when in the Python unwinder I write this: pending_frame.read_register ("register-name") This is translated internally into a call to `value_of_register', which in turn becomes a call to `value_of_register_lazy'. Usually this isn't a problem, `value_of_register_lazy' requires the next frame (more inner) to have a valid frame_id, which will be the case (if we're sniffing frame #1, then frame #0 will have had its frame-id figured out). Unfortunately if frame #0 is inline within frame #1, then the frame-id for frame #0 _is_ same as the frame-id for frame #1 - the frame-id for frame #0 requires us to first compute the frame-id for frame #1. As a result it is not same to call `value_of_register_lazy' before the frame-id of the current frame is known. The solution I propose here is that `value_of_register' stops making a lazy register value (the step that requires the frame-id, and thus causes the problems), and instead calls `get_frame_register_value'. This avoids the need to calculate the frame-id and so avoids the problem. This bug has crept in because we allowed calls to the function `value_of_register_lazy' at a time when the frame-id of the frame passed to the function didn't have its frame-id calculated. This is only ever a problem for inline frames. To prevent bugs like this getting into GDB in the future I've added an assert to the function `value_of_register_lazy' to require the current frame have its id calculated. If we are calling from outside of the frame sniffer then this will _always_ be true, so is not a problem. If we are calling this function from within a frame sniffer then it will always trigger. gdb/ChangeLog: * findvar.c (value_of_register): Use get_frame_register_value rather than value_of_register_lazy. (value_of_register_lazy): Add new assert, and update comments. gdb/testsuite/ChangeLog: * gdb.python/py-unwind-inline.c: New file. * gdb.python/py-unwind-inline.exp: New file. * gdb.python/py-unwind-inline.py: New file. --- gdb/ChangeLog | 6 ++ gdb/findvar.c | 28 ++++++-- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.python/py-unwind-inline.c | 37 ++++++++++ gdb/testsuite/gdb.python/py-unwind-inline.exp | 49 +++++++++++++ gdb/testsuite/gdb.python/py-unwind-inline.py | 71 +++++++++++++++++++ 6 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.c create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.exp create mode 100644 gdb/testsuite/gdb.python/py-unwind-inline.py diff --git a/gdb/findvar.c b/gdb/findvar.c index c7cd31ce1a6..e5445b34930 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -263,16 +263,13 @@ struct value * value_of_register (int regnum, struct frame_info *frame) { struct gdbarch *gdbarch = get_frame_arch (frame); - struct value *reg_val; /* User registers lie completely outside of the range of normal registers. Catch them early so that the target never sees them. */ if (regnum >= gdbarch_num_cooked_regs (gdbarch)) return value_of_user_reg (regnum, frame); - reg_val = value_of_register_lazy (frame, regnum); - value_fetch_lazy (reg_val); - return reg_val; + return get_frame_register_value (frame, regnum); } /* Return a `value' with the contents of (virtual or cooked) register @@ -292,7 +289,28 @@ value_of_register_lazy (struct frame_info *frame, int regnum) next_frame = get_next_frame_sentinel_okay (frame); - /* We should have a valid next frame. */ + /* If NEXT_FRAME is inline within FRAME, then calculating the frame-id + for the next frame will require that FRAME has a valid frame-id. + Usually this is not a problem, however, if this function is ever + called from a frame sniffer, the small window of time when not all + frames have yet had their frame-id calculated, this function will + trigger an assert within get_frame_id that a frame at a level > 0 + doesn't have a frame id. + + Instead of waiting for an inline frame to reveal an invalid use of + this function, we instead demand that FRAME must have had its frame-id + calculated before we use this function. + + The one time it is safe to call this function when FRAME does not yet + have a frame id is when FRAME is at level 0, in this case the + assertion in get_frame_id doesn't fire, and instead get_frame_id will + correctly compute the frame id for us. */ + gdb_assert (frame_relative_level (frame) == 0 + || frame_id_computed_p (frame)); + + /* We should have a valid next frame too. Given that FRAME is more + outer than NEXT_FRAME, and we know that FRAME has its ID calculated, + then this must always be true. */ gdb_assert (frame_id_p (get_frame_id (next_frame))); reg_val = allocate_value_lazy (register_type (gdbarch, regnum)); diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.c b/gdb/testsuite/gdb.python/py-unwind-inline.c new file mode 100644 index 00000000000..0cfe06dd273 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-unwind-inline.c @@ -0,0 +1,37 @@ +/* Copyright 2019-2020 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 . */ + +volatile int global_var; + +int __attribute__ ((noinline)) +bar () +{ + return global_var; +} + +static inline int __attribute__ ((always_inline)) +foo () +{ + return bar (); +} + +int +main () +{ + int ans; + global_var = 0; + ans = foo (); + return ans; +} diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.exp b/gdb/testsuite/gdb.python/py-unwind-inline.exp new file mode 100644 index 00000000000..f7c65f6afc8 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-unwind-inline.exp @@ -0,0 +1,49 @@ +# Copyright (C) 2020 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 script tests GDB's handling of using a Python unwinder in the +# presence of inlined frames. + +load_lib gdb-python.exp + +standard_testfile + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ + debug] } { + return -1 +} + +# Skip all tests if Python scripting is not enabled. +if { [skip_python_tests] } { continue } + +# The following tests require execution. +if ![runto_main] then { + fail "can't run to main" + return 0 +} + +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] + +gdb_breakpoint "foo" + +gdb_test "source ${pyfile}" "Python script imported" \ + "import python scripts" + +gdb_continue_to_breakpoint "foo" + +gdb_test_sequence "backtrace" "backtrace with dummy unwinder" { + "\\r\\n#0 foo \\(\\)" + "\\r\\n#1 main \\(\\)" +} diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.py b/gdb/testsuite/gdb.python/py-unwind-inline.py new file mode 100644 index 00000000000..7206a0b2830 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-unwind-inline.py @@ -0,0 +1,71 @@ +# Copyright (C) 2020 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 . + +# A dummy stack unwinder used for testing the Python unwinders when we +# have inline frames. This unwinder will never claim any frames, +# instead, all it does it try to read all registers possible target +# registers as part of the frame sniffing process.. + +import gdb +from gdb.unwinder import Unwinder + +apb_global = None + +class dummy_unwinder (Unwinder): + """ A dummy unwinder that looks at a bunch of registers as part of + the unwinding process.""" + + class frame_id (object): + """ Basic frame id.""" + + def __init__ (self, sp, pc): + """ Create the frame id.""" + self.sp = sp + self.pc = pc + + def __init__ (self): + """Create the unwinder.""" + Unwinder.__init__ (self, "dummy stack unwinder") + self.void_ptr_t = gdb.lookup_type("void").pointer() + self.regs = None + + def get_regs (self, pending_frame): + """Return a list of register names that should be read. Only + gathers the list once, then caches the result.""" + if (self.regs != None): + return self.regs + + # Collect the names of all registers to read. + self.regs = list (pending_frame.architecture () + .register_names ()) + + return self.regs + + def __call__ (self, pending_frame): + """Actually performs the unwind, or at least sniffs this frame + to see if the unwinder should claim it, which is never does.""" + try: + for r in (self.get_regs (pending_frame)): + v = pending_frame.read_register (r).cast (self.void_ptr_t) + except: + print ("Dummy unwinder, exception") + raise + + return None + +# Register the ComRV stack unwinder. +gdb.unwinder.register_unwinder (None, dummy_unwinder (), True) + +print ("Python script imported") -- 2.25.4