Message ID | cover.1678473293.git.aburgess@redhat.com |
---|---|
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> 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 D73803857352 for <patchwork@sourceware.org>; Fri, 10 Mar 2023 18:43:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D73803857352 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678473831; bh=J81t25qAsTZmslx5uy9265hXuazwU/OdJFlB23jdM+o=; 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=o5k5z84zoFGy1R2jdHE0L1TDXBiqBhMbyxks8Q2zeAFhfEgtwqEbXPcfFtw6yl3Kx bDACuHRyTMpwZsRZxk1oFpF7R4mKI7rJIax2YEzX8YW9ZbHPIJERXQMV+ZjkjIb6vl 6tsIJbnUd+7pPHc0HG3Q0+v+24vtWd+Lseb0noms= 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 8C3AC3858423 for <gdb-patches@sourceware.org>; Fri, 10 Mar 2023 18:43:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8C3AC3858423 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-27-siRcfLcXMeOEYfn2mTNVzQ-1; Fri, 10 Mar 2023 13:43:23 -0500 X-MC-Unique: siRcfLcXMeOEYfn2mTNVzQ-1 Received: by mail-wm1-f71.google.com with SMTP id p22-20020a7bcc96000000b003e2036a1516so4235130wma.7 for <gdb-patches@sourceware.org>; Fri, 10 Mar 2023 10:43:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678473801; 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=J81t25qAsTZmslx5uy9265hXuazwU/OdJFlB23jdM+o=; b=psuqTI1p/bdStdgbhTF1sX+m0KhrQwEKh4RQb1j/nOnjlWgN7ND66f+FkTlxgMC3Fd 9J8nQbHwzE+tBkr10ZS2FyQp5YL7fNrzX40OFnnZPIFvLOUQ4gGEtLRLQ/sh9ILufgQb NImYCnEHMdCFEGmEhFIN64xYhiASOiE8GcsYVY4qDktDfsZpAhARjolgGN5r+q9qlHoi E3Z0R8sJV1+vDSkPLgcBRZyda1wXxcWaIyguvJrWY3f0dom9bwbnmuwoI6dBx454ijq2 LjcGYXNGSB9O30DDzrasLjKaJpIb0Ku7fC9lzuS3AJQmXCFr2PsfAKm0fogvlrQwYNi8 z3uQ== X-Gm-Message-State: AO0yUKUu1F8a9PtSOBEiv8/0ZE+29ToC0U9ryofXiQQ9kOqqHciNJx/d sA0BeR06l+tgovS2ZycJPTzQRVaZDiZ6YMQE6/ALQyvj8rJDjyf/cUMjJ5S5akcOqeHJaNKrnQo XfOLWyngAWWHXXwv+B0e3HepjU3xtWUea+D0NysQ32U3O5Z/yAZAU+OtCP4F7EGmUwAMV9Nxltd VL4N112A== X-Received: by 2002:a05:600c:4748:b0:3e7:f108:664c with SMTP id w8-20020a05600c474800b003e7f108664cmr3534125wmo.40.1678473801707; Fri, 10 Mar 2023 10:43:21 -0800 (PST) X-Google-Smtp-Source: AK7set/1m82CiOHyQNk/5YUfplTOPVx5gjmb56HFQxXFZ8U/UvGvA8AKi3IugGbnEypLsUti5tPR5A== X-Received: by 2002:a05:600c:4748:b0:3e7:f108:664c with SMTP id w8-20020a05600c474800b003e7f108664cmr3534099wmo.40.1678473801260; Fri, 10 Mar 2023 10:43:21 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id w7-20020a05600c474700b003e1202744f2sm695637wmo.31.2023.03.10.10.43.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Mar 2023 10:43:20 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess <aburgess@redhat.com> Subject: [PATCHv3 0/9] Add new gdbarch::displaced_step_buffer_length field Date: Fri, 10 Mar 2023 18:43:09 +0000 Message-Id: <cover.1678473293.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: <cover.1678116328.git.aburgess@redhat.com> References: <cover.1678116328.git.aburgess@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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 <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Andrew Burgess <aburgess@redhat.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
Add new gdbarch::displaced_step_buffer_length field
|
|
Message
Andrew Burgess
March 10, 2023, 6:43 p.m. UTC
Changes in V3: - Have now run 'black' on each patch in this series, so everything should be formatted correctly, - Have fixed the out of date names in the last patch of this series. Everything buildds and runs fine for me now, - Have removed the out of date text from the commit message in patch #3, - Have fixed the missing comma Tom spotted in patch #4, - I have NOT removed the comments Simon pointed out in patch #4. Removing these would require changing the generated code for more Components than I already change in this commit. And so, I think, if those comments are to go that would require a separate patch. That said, we do generate validation code within the getters for many components, so I think having a comment in the components where we don't generate validation makes sense. So for me, I think the comments do add value, I'd suggest we should keep them. - And the big one. I've changed the 'invalid' default from False to True. I know Simon suggested that False was the correct choice, but I actually think that True makes more sense. I'd rather we assume we should generate the validity checks and require than new components explicitly choose to not have that check, rather than just assuming we shouldn't check. However, in order to get to the default True state I ended up having to fix some other issues. And, so, incase people really would rather see False as the default, I've left the patch which changes to default True as the penultimate patch in the series. If you feel really strongly that False is best, I can just drop the patch that switches over to use True. Just let me know. Let me know what you think, Thanks, Andrew --- Changes in v2: - The final patch (#5), actually adding displaced_step_buffer_length has been updated inline with Simon's feedback. The changes are pretty minor so I've added the Approved-By tag. - Patches #1 and #2 are new in this version. These two patches are removing fields from gdbarch_components.py that have no effect on the generated source. Removing these fields makes it easier to refactor the gdbarch.py code in later commits. - Patch #3 is from the v1 series and hasn't changed significantly. I dropped the comment change on max_insn_length. Simon was correct in his feedback, the changes I made to this comment made little sense. The rest of the patch is basically unchagned. - Patch #4 is new in this version. This makes the changes Simon wanted to see about the 'invalid' field no longer being able to take the value 'None', and also we can now have both a predicate and a string 'invalid' field. I did play with splitting these two changes (invalid can't be None and predicate+string invalid), but this made little sense, and was really forced. Allowing the second seems to fall naturally out of refactoring the 'invalid' field to stop it taking the value 'None'. --- Andrew Burgess (9): gdb/gdbarch: remove unused 'invalid=True' from gdbarch_components.py gdb/gdbarch: remove yet more 'invalid=True' from gdbarch_components.py gdb/gdbarch: split postdefault setup from invalid check in gdbarch.py gdb/gdbarch: remove the 'invalid=None' state from gdbarch_components.py gdbarch: use predefault for more value components within gdbarch gdbarch: improve generation of validation in gdbarch getters gdbarch: remove some unneeded predefault="0" from gdbarch_components.py gdbarch: make invalid=True the default for all Components gdb: add gdbarch::displaced_step_buffer_length gdb/aarch64-linux-tdep.c | 4 +- gdb/arm-tdep.c | 4 +- gdb/displaced-stepping.c | 6 +- gdb/gdbarch-gen.h | 12 ++- gdb/gdbarch.c | 94 +++++++++++++------- gdb/gdbarch.py | 66 +++++++------- gdb/gdbarch_components.py | 182 +++++++++++--------------------------- gdb/gdbarch_types.py | 7 +- gdb/linux-tdep.c | 2 +- gdb/rs6000-tdep.c | 6 +- 10 files changed, 175 insertions(+), 208 deletions(-) base-commit: 6a208145d24c47912c8beb4f1f4b9abeb8d51134
Comments
On 3/10/23 13:43, Andrew Burgess via Gdb-patches wrote: > Changes in V3: > > - Have now run 'black' on each patch in this series, so everything > should be formatted correctly, > > - Have fixed the out of date names in the last patch of this series. > Everything buildds and runs fine for me now, > > - Have removed the out of date text from the commit message in patch > #3, > > - Have fixed the missing comma Tom spotted in patch #4, > > - I have NOT removed the comments Simon pointed out in patch #4. > Removing these would require changing the generated code for more > Components than I already change in this commit. And so, I think, > if those comments are to go that would require a separate patch. > > That said, we do generate validation code within the getters for > many components, so I think having a comment in the components > where we don't generate validation makes sense. So for me, I > think the comments do add value, I'd suggest we should keep them. > > - And the big one. I've changed the 'invalid' default from False to > True. I know Simon suggested that False was the correct choice, > but I actually think that True makes more sense. I'd rather we > assume we should generate the validity checks and require than new > components explicitly choose to not have that check, rather than > just assuming we shouldn't check. > > However, in order to get to the default True state I ended up > having to fix some other issues. And, so, incase people really > would rather see False as the default, I've left the patch which > changes to default True as the penultimate patch in the series. > If you feel really strongly that False is best, I can just drop > the patch that switches over to use True. Just let me know. > > Let me know what you think, > > Thanks, > Andrew Thanks Andrew, it all LGTM: Approved-By: Simon Marchi <simon.marchi@efficios.com> Simon
Simon Marchi <simark@simark.ca> writes: > On 3/10/23 13:43, Andrew Burgess via Gdb-patches wrote: >> Changes in V3: >> >> - Have now run 'black' on each patch in this series, so everything >> should be formatted correctly, >> >> - Have fixed the out of date names in the last patch of this series. >> Everything buildds and runs fine for me now, >> >> - Have removed the out of date text from the commit message in patch >> #3, >> >> - Have fixed the missing comma Tom spotted in patch #4, >> >> - I have NOT removed the comments Simon pointed out in patch #4. >> Removing these would require changing the generated code for more >> Components than I already change in this commit. And so, I think, >> if those comments are to go that would require a separate patch. >> >> That said, we do generate validation code within the getters for >> many components, so I think having a comment in the components >> where we don't generate validation makes sense. So for me, I >> think the comments do add value, I'd suggest we should keep them. >> >> - And the big one. I've changed the 'invalid' default from False to >> True. I know Simon suggested that False was the correct choice, >> but I actually think that True makes more sense. I'd rather we >> assume we should generate the validity checks and require than new >> components explicitly choose to not have that check, rather than >> just assuming we shouldn't check. >> >> However, in order to get to the default True state I ended up >> having to fix some other issues. And, so, incase people really >> would rather see False as the default, I've left the patch which >> changes to default True as the penultimate patch in the series. >> If you feel really strongly that False is best, I can just drop >> the patch that switches over to use True. Just let me know. >> >> Let me know what you think, >> >> Thanks, >> Andrew > > Thanks Andrew, it all LGTM: > > Approved-By: Simon Marchi <simon.marchi@efficios.com> Pushed. Thanks, Andrew