From patchwork Tue Feb 28 16:51:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 65792 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 878F2385781A for ; Tue, 28 Feb 2023 16:51:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 878F2385781A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677603107; bh=JA8Friwz1c/dfOasowp4D6lek5vlCIxbpjgie+gJyBw=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=X3SsFYXryMITqUki57mUkjBh6fl2QWepwAAcPbIsxYNYqvT3CS3mbkuSRMtMzlVjr COJqn4tR8Vkn7HON76MLRP7Lp5tDhCnICNQ7q2ypBb5Y6K0Eep3v1vpyrcAn1ssPBs aYHn2zXPqJGybqrTAifqPuyQUzoBSUZM+oSvqqKQ= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 040AC3858C2B for ; Tue, 28 Feb 2023 16:51:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 040AC3858C2B Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-416-hmXEVnPYOCSofvhP8UMlLg-1; Tue, 28 Feb 2023 11:51:18 -0500 X-MC-Unique: hmXEVnPYOCSofvhP8UMlLg-1 Received: by mail-ed1-f72.google.com with SMTP id v12-20020a056402174c00b004acb5c6e52bso14673318edx.1 for ; Tue, 28 Feb 2023 08:51:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677603077; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JA8Friwz1c/dfOasowp4D6lek5vlCIxbpjgie+gJyBw=; b=yrH+cBzJzCB/ktWBIFdr9axvy8+WbkVdllSP4pXkb/VMUM1FQzppHetOh7FXXpui6G mkNMUromw0f2AA3unb7+i4rptVTyEn6jYLPj/Ku9tqlRRzQcc9ZiWO+EJWzL/MTJf6Xh MlnYyDEVTo0ILmJBsWdgWZ1dDovnxOq5NGCCw0xsZVN29wpF7bDI4Fs3NBjexYCTkt0H CRDjZsL1yMCaOexdELJjj3BYsvlEU3vVxqGQp4M3E+AqWDl9r0bvPfoxhl2XH73rNtJa if3YPxKOnFl+SIsZ8Is5gRC0fLodqTscDAxEVjB/nexFvkhAscNfOIuztPWeHQ6n+JUJ l3Sg== X-Gm-Message-State: AO0yUKU5QfZ5W8rknjkt7OS0YND2S7DyTd5yXLQlSnLd32UjximEtBte 8Q063N8NJXt3tg9xPlNS6XNwZnFc9c3ncP71RYif9JFd+GRrXU3qdg9nB8VQsTjoGDpWbNFvrN3 mwc76/dsVwsXgLaVs1K2Fi1wfq9g6K0YDTKaEoe0owYYgRM7iZIsW2WZDZ8zLVsTsJRHL6TJasz 5/OO0= X-Received: by 2002:a17:906:564a:b0:8af:4684:91cf with SMTP id v10-20020a170906564a00b008af468491cfmr3136432ejr.32.1677603077154; Tue, 28 Feb 2023 08:51:17 -0800 (PST) X-Google-Smtp-Source: AK7set/yGP8SYL+fhcTYytLl2fNI37uIc5r+BUtE7hFatQFNPUSOCsI8LkyBXaZeuLeqvTGjpo15fA== X-Received: by 2002:a17:906:564a:b0:8af:4684:91cf with SMTP id v10-20020a170906564a00b008af468491cfmr3136413ejr.32.1677603076786; Tue, 28 Feb 2023 08:51:16 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id p17-20020a170906229100b008e68d2c11d8sm4725244eja.218.2023.02.28.08.51.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 08:51:16 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 1/2] gdb: updates to gdbarch.py algorithm Date: Tue, 28 Feb 2023 16:51:12 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 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_NONE, 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 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-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Restructure how gdbarch.py generates the verify_gdbarch function. Previously the postdefault handling was bundled together with the validation. This means that a field can't have both a postdefault, and set its invalid attribute to a string. This doesn't seem reasonable to me, I see no reason why a field can't have both a postdefault (used when the tdep doesn't set the field), and an invalid expression, which can be used to validate the value that a tdep might set. In this commit I restructure the verify_gdbarch generation code to allow the above, there is no change in the actual generated code in this commit, that will come in later commit. I did end up having to remove the "invalid" attribute (where the attribute was set to True) from a number of fields in this commit. This invalid attribute was never having an effect as these components all have a postdefault. Consider; the "postdefault" is applied if the field still has its initial value, while an "invalid" attribute set to True means error if the field still has its default value. But the field never will have its default value, it will always have its postdefault value. --- gdb/gdbarch.py | 31 ++++++++++++++++--------- gdb/gdbarch_components.py | 49 ++++++++++++++------------------------- 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/gdb/gdbarch.py b/gdb/gdbarch.py index 93b1e8bf84e..7fea41c9572 100755 --- a/gdb/gdbarch.py +++ b/gdb/gdbarch.py @@ -203,35 +203,44 @@ with open("gdbarch.c", "w") as f: file=f, ) for c in filter(not_info, components): - if c.invalid is False: - print(f" /* Skip verify of {c.name}, invalid_p == 0 */", file=f) - elif c.predicate: - print(f" /* Skip verify of {c.name}, has predicate. */", file=f) - elif isinstance(c.invalid, str) and c.postdefault is not None: - print(f" if ({c.invalid})", file=f) - print(f" gdbarch->{c.name} = {c.postdefault};", file=f) - elif c.predefault is not None and c.postdefault is not None: + # An opportunity to write in the 'postdefault' value. + if c.postdefault is not None and c.predefault is not None: print(f" if (gdbarch->{c.name} == {c.predefault})", file=f) print(f" gdbarch->{c.name} = {c.postdefault};", file=f) elif c.postdefault is not None: print(f" if (gdbarch->{c.name} == 0)", file=f) print(f" gdbarch->{c.name} = {c.postdefault};", file=f) + + # Now validate the value. + if c.invalid is False: + print(f" /* Skip verify of {c.name}, invalid_p == 0 */", file=f) + elif c.predicate: + print(f" /* Skip verify of {c.name}, has predicate. */", file=f) + elif c.invalid is None: + # No validation has been requested for this component. + pass elif isinstance(c.invalid, str): print(f" if ({c.invalid})", file=f) print(f""" log.puts ("\\n\\t{c.name}");""", file=f) - elif c.predefault is not None: + elif c.invalid is True and c.predefault is not None: print(f" if (gdbarch->{c.name} == {c.predefault})", file=f) print(f""" log.puts ("\\n\\t{c.name}");""", file=f) - elif c.invalid is True: + elif c.invalid is True and c.postdefault is None: print(f" if (gdbarch->{c.name} == 0)", file=f) print(f""" log.puts ("\\n\\t{c.name}");""", file=f) + elif c.invalid is True: + # This component has its 'invalid' field set to True, but + # also has a postdefault. This makes no sense, the + # postdefault will have been applied above, so this field + # will not have a zero value. + raise Exception(f"component {c.name} has postdefault and invalid set to True") else: # We should not allow ourselves to simply do nothing here # because no other case applies. If we end up here then # either the input data needs adjusting so one of the # above cases matches, or we need additional cases adding # here. - raise Exception("unhandled case when generating gdbarch validation") + raise Exception(f"unhandled case when generating gdbarch validation: {c.name}") print(" if (!log.empty ())", file=f) print( """ internal_error (_("verify_gdbarch: the following are invalid ...%s"),""", diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py index caa65c334ec..1d420a513f9 100644 --- a/gdb/gdbarch_components.py +++ b/gdb/gdbarch_components.py @@ -63,34 +63,28 @@ # * "predefault", "postdefault", and "invalid" - These are used for # the initialization and verification steps: # -# A gdbarch is zero-initialized. Then, if a field has a pre-default, -# the field is set to that value. After initialization is complete -# (that is, after the tdep code has a chance to change the settings), -# the post-initialization step is done. +# A gdbarch is zero-initialized. Then, if a field has a "predefault", +# the field is set to that value. This becomes the fields initial +# value. # -# There is a generic algorithm to generate a "validation function" for -# all fields. If the field has an "invalid" attribute with a string -# value, then this string is the expression (note that a string-valued -# "invalid" and "predicate" are mutually exclusive; and the case where -# invalid is True means to ignore this field and instead use the -# default checking that is about to be described). Otherwise, if -# there is a "predefault", then the field is valid if it differs from -# the predefault. Otherwise, the check is done against 0 (really NULL -# for function pointers, but same idea). -# -# In post-initialization / validation, there are several cases. +# After initialization is complete (that is, after the tdep code has a +# chance to change the settings), the post-initialization step is +# done. # -# * If "invalid" is False, or if the field specifies "predicate", -# validation is skipped. Otherwise, a validation step is emitted. +# If the field still has its initial value (see above), and the field +# has a "postdefault", then the post field is set to this value. # -# * Otherwise, the validity is checked using the usual validation -# function (see above). If the field is considered valid, nothing is -# done. +# After the possible "postdefault" assignment, validation is +# performed for fields that don't have a "predicate". # -# * Otherwise, the field's value is invalid. If there is a -# "postdefault", then the field is assigned that value. +# If the field has an "invalid" attribute with a string value, then +# this string is the expression that should evaluate to true when the +# field is invalid. # -# * Otherwise, the gdbarch will fail validation and gdb will crash. +# Otherwise, if "invalid" is True, then the generic validation +# function is used: the field is considered invalid it it still +# contains its default value. This validation is what is used within +# the _p predicate function if the field has "predicate" set to True. # # Function and Method share: # @@ -206,7 +200,6 @@ Value( type="const struct floatformat **", name="bfloat16_format", postdefault="floatformats_bfloat16", - invalid=True, printer="pformat (gdbarch, gdbarch->bfloat16_format)", ) @@ -221,7 +214,6 @@ Value( type="const struct floatformat **", name="half_format", postdefault="floatformats_ieee_half", - invalid=True, printer="pformat (gdbarch, gdbarch->half_format)", ) @@ -236,7 +228,6 @@ Value( type="const struct floatformat **", name="float_format", postdefault="floatformats_ieee_single", - invalid=True, printer="pformat (gdbarch, gdbarch->float_format)", ) @@ -251,7 +242,6 @@ Value( type="const struct floatformat **", name="double_format", postdefault="floatformats_ieee_double", - invalid=True, printer="pformat (gdbarch, gdbarch->double_format)", ) @@ -266,7 +256,6 @@ Value( type="const struct floatformat **", name="long_double_format", postdefault="floatformats_ieee_double", - invalid=True, printer="pformat (gdbarch, gdbarch->long_double_format)", ) @@ -289,7 +278,6 @@ One if `wchar_t' is signed, zero if unsigned. name="wchar_signed", predefault="-1", postdefault="1", - invalid=True, ) Method( @@ -332,7 +320,6 @@ addr_bit is the size of a target address as represented in gdb name="addr_bit", predefault="0", postdefault="gdbarch_ptr_bit (gdbarch)", - invalid=True, ) Value( @@ -355,7 +342,6 @@ and if Dwarf versions < 4 need to be supported. name="dwarf2_addr_size", predefault="0", postdefault="gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT", - invalid=True, ) Value( @@ -366,7 +352,6 @@ One if `char' acts like `signed char', zero if `unsigned char'. name="char_signed", predefault="-1", postdefault="1", - invalid=True, ) Function( From patchwork Tue Feb 28 16:51:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 65793 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7589D385B529 for ; Tue, 28 Feb 2023 16:52:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7589D385B529 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677603149; bh=VZJM6bOMcSkaIsoDkd9i5J+z0jE1rLctoNKbnScLNLQ=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=mfEAfCTuKKsBj2LjHpGo24vuIIuajz5HM/giCRFjT2uiDcHMGRItEDakFKFDPmCAL 1H2UCS0Q0bompsNgoXpOJ8wgf/Rl/2LtPwUX5+DeZrdp7BUHOSIIUmWLFqIO+iyKu3 tjazxydrdShqS2vaz6M54OJI0OS+xc6H7R4KLIRg= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org 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 6FA39385841C for ; Tue, 28 Feb 2023 16:51:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6FA39385841C Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-625-ybHETMCkNt29-nQgQPYWrQ-1; Tue, 28 Feb 2023 11:51:19 -0500 X-MC-Unique: ybHETMCkNt29-nQgQPYWrQ-1 Received: by mail-ed1-f71.google.com with SMTP id eh16-20020a0564020f9000b004acc4f8aa3fso14702835edb.3 for ; Tue, 28 Feb 2023 08:51:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677603078; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VZJM6bOMcSkaIsoDkd9i5J+z0jE1rLctoNKbnScLNLQ=; b=FJTOdJ8XGwNObGuuItOrjoqAknScGF8au4lRV5dFfHS9lflG4Jflt00vuHtc5VFxPH Hce87EVRfAEViUFGZ3cz0Kmr1/OgtZP+SPu97s2HayOcrpwtH/XUQb7VZvbKqdut1Gyq VHOqeCOhpwc9lDqsQcT1oY3hMTv97rI8weLvAEGIKorFkXSWw46Aijlz/kEGXNpNg6lw uLDMT8ypObyLkWwCV7Z2UFXCj6uQ9C2+zINTpIxn6LqohnoXb9ZZuR1U2WREotuCuF7s vOULT/STMrKJc11RTkT9rGHyVrn+hldCFQanmdkGsW8FFaoxJEaqZ2iP9+HbReCX7ue2 mcFQ== X-Gm-Message-State: AO0yUKW39qXfR05bLFVpHDgN1NowEqfaFLbdn/9r1nhTg1bMoU5ieRUi 9/rnXoYHSep8GZUBRXK8K5DA2EB5tdCCPKN7ozgOK4rsvLn5X3wQEsB8vGrlyOJ3UkRMb9rOji+ pEhgm22ZptwUHuToyS3FVvfhB8oOSmZGF1yDfBA+5yX+YLtfAPZAXN1e5nqtLQEXTeVvZhNjArc 2UHmg= X-Received: by 2002:a17:907:11dd:b0:889:1eb1:7517 with SMTP id va29-20020a17090711dd00b008891eb17517mr3249562ejb.30.1677603078359; Tue, 28 Feb 2023 08:51:18 -0800 (PST) X-Google-Smtp-Source: AK7set8QneNBSER2eDNfW/LKFf4XrePPz3am1GNfBbw5w18Ggjooqo8ngajwD0qeH6Gz+FWytY+qdA== X-Received: by 2002:a17:907:11dd:b0:889:1eb1:7517 with SMTP id va29-20020a17090711dd00b008891eb17517mr3249543ejb.30.1677603078036; Tue, 28 Feb 2023 08:51:18 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id fg1-20020a1709069c4100b008c979c74732sm4734613ejc.156.2023.02.28.08.51.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 08:51:17 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: add gdbarch::displaced_step_max_buffer_length Date: Tue, 28 Feb 2023 16:51:13 +0000 Message-Id: <8b214d8ebfe903530c99e5c61c97021fd6a5ade6.1677602918.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 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_NONE, 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 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-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" The gdbarch::max_insn_length field is used mostly to support displaced stepping; it controls the size of the buffers allocated for the displaced-step instruction, and is also used when first copying the instruction, and later, when fixing up the instruction, in order to read in and parse the instruction being stepped. However, it has started to be used in other places in GDB, for example, it's used in the Python disassembler API, and it is used on amd64 as part of branch-tracing instruction classification. The problem is that the value assigned to max_insn_length is not always the maximum instruction length, but sometimes is a multiple of that length, as required to support displaced stepping, see rs600, ARM, and AArch64 for examples of this. It seems to me that we are overloading the meaning of the max_insn_length field, and I think that could potentially lead to confusion. I propose that we add a new gdbarch field, gdbarch::displaced_step_max_buffer_length, this new field will do exactly what it says on the tin; represent the required displaced step buffer size. The max_insn_length field can then do exactly what it claims to do; represent the maximum length of a single instruction. As some architectures (e.g. i386, and amd64) only require their displaced step buffers to be a single instruction in size, I propose that the default for displaced_step_max_buffer_length will be the value of max_insn_length. Architectures than need more buffer space can then override this default as needed. I've updated all architectures to setup the new field if appropriate, and I've audited all calls to gdbarch_max_insn_length and switched to gdbarch_displaced_step_max_buffer_length where appropriate. There should be no user visible changes after this commit. --- gdb/aarch64-linux-tdep.c | 4 +++- gdb/arm-tdep.c | 4 +++- gdb/displaced-stepping.c | 6 +++--- gdb/gdbarch-gen.h | 17 ++++++++++++++--- gdb/gdbarch.c | 26 ++++++++++++++++++++++++++ gdb/gdbarch_components.py | 23 ++++++++++++++++++++--- gdb/linux-tdep.c | 2 +- gdb/rs6000-tdep.c | 6 ++++-- 8 files changed, 74 insertions(+), 14 deletions(-) diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index 0000b498f89..3eaf4e1131f 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -2240,7 +2240,9 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_number); /* Displaced stepping. */ - set_gdbarch_max_insn_length (gdbarch, 4 * AARCH64_DISPLACED_MODIFIED_INSNS); + set_gdbarch_max_insn_length (gdbarch, 4); + set_gdbarch_displaced_step_max_buffer_length + (gdbarch, 4 * AARCH64_DISPLACED_MODIFIED_INSNS); set_gdbarch_displaced_step_copy_insn (gdbarch, aarch64_displaced_step_copy_insn); set_gdbarch_displaced_step_fixup (gdbarch, aarch64_displaced_step_fixup); diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index b64c21ce68f..d1941992c77 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -10662,7 +10662,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) /* Note: for displaced stepping, this includes the breakpoint, and one word of additional scratch space. This setting isn't used for anything beside displaced stepping at present. */ - set_gdbarch_max_insn_length (gdbarch, 4 * ARM_DISPLACED_MODIFIED_INSNS); + set_gdbarch_displaced_step_max_buffer_length + (gdbarch, 4 * ARM_DISPLACED_MODIFIED_INSNS); + set_gdbarch_max_insn_length (gdbarch, 4); /* This should be low enough for everything. */ tdep->lowest_pc = 0x20; diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c index 06b32a80f6a..72b6366390f 100644 --- a/gdb/displaced-stepping.c +++ b/gdb/displaced-stepping.c @@ -55,7 +55,7 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc) regcache *regcache = get_thread_regcache (thread); const address_space *aspace = regcache->aspace (); gdbarch *arch = regcache->arch (); - ULONGEST len = gdbarch_max_insn_length (arch); + ULONGEST len = gdbarch_displaced_step_max_buffer_length (arch); /* Search for an unused buffer. */ displaced_step_buffer *buffer = nullptr; @@ -243,7 +243,7 @@ displaced_step_buffers::finish (gdbarch *arch, thread_info *thread, below. */ thread->inf->displaced_step_state.unavailable = false; - ULONGEST len = gdbarch_max_insn_length (arch); + ULONGEST len = gdbarch_displaced_step_max_buffer_length (arch); /* Restore memory of the buffer. */ write_memory_ptid (thread->ptid, buffer->addr, @@ -302,7 +302,7 @@ displaced_step_buffers::restore_in_ptid (ptid_t ptid) regcache *regcache = get_thread_regcache (buffer.current_thread); gdbarch *arch = regcache->arch (); - ULONGEST len = gdbarch_max_insn_length (arch); + ULONGEST len = gdbarch_displaced_step_max_buffer_length (arch); write_memory_ptid (ptid, buffer.addr, buffer.saved_copy.data (), len); diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index ddb97f60315..1cfbbcf1730 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -1018,7 +1018,10 @@ typedef void (gdbarch_skip_permanent_breakpoint_ftype) (struct regcache *regcach extern void gdbarch_skip_permanent_breakpoint (struct gdbarch *gdbarch, struct regcache *regcache); extern void set_gdbarch_skip_permanent_breakpoint (struct gdbarch *gdbarch, gdbarch_skip_permanent_breakpoint_ftype *skip_permanent_breakpoint); -/* The maximum length of an instruction on this architecture in bytes. */ +/* The maximum length of an instruction on this architecture in octets. + This must be set for architectures that support displaced-stepping. + Setting this for other architectures improves error detection within + the Python disassembler API. */ extern bool gdbarch_max_insn_length_p (struct gdbarch *gdbarch); @@ -1039,8 +1042,8 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i see the comments in infrun.c. The TO area is only guaranteed to have space for - gdbarch_max_insn_length (arch) bytes, so this function must not - write more bytes than that to that area. + gdbarch_displaced_step_max_buffer_length (arch) octets, so this + function must not write more octets than that to this area. If you do not provide this function, GDB assumes that the architecture does not support displaced stepping. @@ -1122,6 +1125,14 @@ typedef void (gdbarch_displaced_step_restore_all_in_ptid_ftype) (inferior *paren extern void gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch, inferior *parent_inf, ptid_t child_ptid); extern void set_gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch, gdbarch_displaced_step_restore_all_in_ptid_ftype *displaced_step_restore_all_in_ptid); +/* The maximum length in octets required for a displaced-step instruction + buffer. By default this will be the same as gdbarch::max_insn_length, + but should be overridden for architectures that might expand a + displaced-step instruction to multiple replacement instructions. */ + +extern ULONGEST gdbarch_displaced_step_max_buffer_length (struct gdbarch *gdbarch); +extern void set_gdbarch_displaced_step_max_buffer_length (struct gdbarch *gdbarch, ULONGEST displaced_step_max_buffer_length); + /* Relocate an instruction to execute at a different address. OLDLOC is the address in the inferior memory where the instruction to relocate is currently at. On input, TO points to the destination diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 7e4e34d5aca..49e2b63d9ff 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -192,6 +192,7 @@ struct gdbarch gdbarch_displaced_step_finish_ftype *displaced_step_finish = NULL; gdbarch_displaced_step_copy_insn_closure_by_addr_ftype *displaced_step_copy_insn_closure_by_addr = nullptr; gdbarch_displaced_step_restore_all_in_ptid_ftype *displaced_step_restore_all_in_ptid = nullptr; + ULONGEST displaced_step_max_buffer_length = 0; gdbarch_relocate_instruction_ftype *relocate_instruction = NULL; gdbarch_overlay_update_ftype *overlay_update = nullptr; gdbarch_core_read_description_ftype *core_read_description = nullptr; @@ -453,6 +454,10 @@ verify_gdbarch (struct gdbarch *gdbarch) log.puts ("\n\tdisplaced_step_finish"); /* Skip verify of displaced_step_copy_insn_closure_by_addr, has predicate. */ /* Skip verify of displaced_step_restore_all_in_ptid, invalid_p == 0 */ + if (gdbarch->displaced_step_max_buffer_length == 0) + gdbarch->displaced_step_max_buffer_length = gdbarch->max_insn_length; + if (gdbarch->displaced_step_max_buffer_length < gdbarch->max_insn_length) + log.puts ("\n\tdisplaced_step_max_buffer_length"); /* Skip verify of relocate_instruction, has predicate. */ /* Skip verify of overlay_update, has predicate. */ /* Skip verify of core_read_description, has predicate. */ @@ -1111,6 +1116,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) gdb_printf (file, "gdbarch_dump: displaced_step_restore_all_in_ptid = <%s>\n", host_address_to_string (gdbarch->displaced_step_restore_all_in_ptid)); + gdb_printf (file, + "gdbarch_dump: displaced_step_max_buffer_length = %s\n", + plongest (gdbarch->displaced_step_max_buffer_length)); gdb_printf (file, "gdbarch_dump: gdbarch_relocate_instruction_p() = %d\n", gdbarch_relocate_instruction_p (gdbarch)); @@ -4153,6 +4161,24 @@ set_gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch, gdbarch->displaced_step_restore_all_in_ptid = displaced_step_restore_all_in_ptid; } +ULONGEST +gdbarch_displaced_step_max_buffer_length (struct gdbarch *gdbarch) +{ + gdb_assert (gdbarch != NULL); + /* Check variable is valid. */ + gdb_assert (!(gdbarch->displaced_step_max_buffer_length < gdbarch->max_insn_length)); + if (gdbarch_debug >= 2) + gdb_printf (gdb_stdlog, "gdbarch_displaced_step_max_buffer_length called\n"); + return gdbarch->displaced_step_max_buffer_length; +} + +void +set_gdbarch_displaced_step_max_buffer_length (struct gdbarch *gdbarch, + ULONGEST displaced_step_max_buffer_length) +{ + gdbarch->displaced_step_max_buffer_length = displaced_step_max_buffer_length; +} + bool gdbarch_relocate_instruction_p (struct gdbarch *gdbarch) { diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py index 1d420a513f9..a00398bb03d 100644 --- a/gdb/gdbarch_components.py +++ b/gdb/gdbarch_components.py @@ -1752,7 +1752,10 @@ Advance PC to next instruction in order to skip a permanent breakpoint. Value( comment=""" -The maximum length of an instruction on this architecture in bytes. +The maximum length of an instruction on this architecture in octets. +This must be set for architectures that support displaced-stepping. +Setting this for other architectures improves error detection within +the Python disassembler API. """, type="ULONGEST", name="max_insn_length", @@ -1777,8 +1780,8 @@ For a general explanation of displaced stepping and how GDB uses it, see the comments in infrun.c. The TO area is only guaranteed to have space for -gdbarch_max_insn_length (arch) bytes, so this function must not -write more bytes than that to that area. +gdbarch_displaced_step_max_buffer_length (arch) octets, so this +function must not write more octets than that to this area. If you do not provide this function, GDB assumes that the architecture does not support displaced stepping. @@ -1890,6 +1893,20 @@ contents of all displaced step buffers in the child's address space. invalid=False, ) +Value( + comment=""" +The maximum length in octets required for a displaced-step instruction +buffer. By default this will be the same as gdbarch::max_insn_length, +but should be overridden for architectures that might expand a +displaced-step instruction to multiple replacement instructions. +""", + type="ULONGEST", + name="displaced_step_max_buffer_length", + predefault="0", + postdefault="gdbarch->max_insn_length", + invalid="gdbarch->displaced_step_max_buffer_length < gdbarch->max_insn_length", +) + Method( comment=""" Relocate an instruction to execute at a different address. OLDLOC diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index e6ce13a1c67..ce48515d8ca 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -2603,7 +2603,7 @@ linux_displaced_step_prepare (gdbarch *arch, thread_info *thread, at DISP_STEP_BUF_ADDR. They are all of size BUF_LEN. */ CORE_ADDR disp_step_buf_addr = linux_displaced_step_location (thread->inf->gdbarch); - int buf_len = gdbarch_max_insn_length (arch); + int buf_len = gdbarch_displaced_step_max_buffer_length (arch); linux_gdbarch_data *gdbarch_data = get_linux_gdbarch_data (arch); gdb_assert (gdbarch_data->num_disp_step_buffers > 0); diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 104515de030..9fc4574bb41 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -889,7 +889,8 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { - size_t len = gdbarch_max_insn_length (gdbarch); + size_t len = gdbarch_displaced_step_max_buffer_length (gdbarch); + gdb_assert (len > PPC_INSN_SIZE); std::unique_ptr closure (new ppc_displaced_step_copy_insn_closure (len)); gdb_byte *buf = closure->buf.data (); @@ -8363,8 +8364,9 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_displaced_step_finish (gdbarch, ppc_displaced_step_finish); set_gdbarch_displaced_step_restore_all_in_ptid (gdbarch, ppc_displaced_step_restore_all_in_ptid); + set_gdbarch_displaced_step_max_buffer_length (gdbarch, 2 * PPC_INSN_SIZE); - set_gdbarch_max_insn_length (gdbarch, 2 * PPC_INSN_SIZE); + set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE); /* Hook in ABI-specific overrides, if they have been registered. */ info.target_desc = tdesc;