From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x342.google.com (mail-ot1-x342.google.com [IPv6:2607:f8b0:4864:20::342]) by sourceware.org (Postfix) with ESMTPS id 018CE394743A for ; Wed, 11 Mar 2020 20:48:44 +0000 (GMT) Received: by mail-ot1-x342.google.com with SMTP id a49so851687otc.11 for ; Wed, 11 Mar 2020 13:48:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+MDitTBn3oI3LTo+nWRunikNhBNbS3pRfBlNoTkxTf0=; b=C3KKZHIS1sNezNrViQRNYOb6qlKS6vjAbncndZE+I+ZBHXDiDRPtgDb9+iqmmdEMJ/ xGSxjPF4OvOiKT2vjNpOaanoelbRrMVHvu5wfAl9M4LVm252LM1p/bsNyWcgMbQiSyw3 l4MlTAq17sa4jt7i2qD2oHnxjW9ifZNLZL90SaV8xyC76LV9DIovNHbwc10cu+UP+kJv PXudGhoGXLwyydjWELWYP3qy/lbDGXQZux9nWxIJNLh3yy/A8x6a9aL8UimoKNCxZAjn t8kyDSu7lT0zpSWYmqCFK82mD1VQB1E8nh92nlK+m4Ngdv4+5OBTfFD/xk4vlZJzA/xS uoMw== X-Gm-Message-State: ANhLgQ1l7ILkbjAC8omEE+9mb9a7WhqNklyYpETFaqWfcspLjbZlF6da tJEHApZKmbX5/0/IkFg2JIcOrhDO0bImG9HaSyipPg== X-Google-Smtp-Source: ADFU+vtmcp7nEYJUA/CSy3wNIh+63l/ynsWph6tHIfD98YT6p0FM2WeVxKz+Bwz2nIBeMIhfaTBMly8i5K1v+wVYmxA= X-Received: by 2002:a05:6830:1c7:: with SMTP id r7mr3881691ota.129.1583959722968; Wed, 11 Mar 2020 13:48:42 -0700 (PDT) MIME-Version: 1.0 References: <20200306205900.239755-1-cbiesinger@google.com> In-Reply-To: From: Christian Biesinger Date: Wed, 11 Mar 2020 15:48:06 -0500 Message-ID: Subject: Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM) To: Simon Marchi Cc: gdb-patches , Kamil Rytarowski Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL 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, 11 Mar 2020 20:48:45 -0000 Ping -- Kamil, any thoughts on the below patch? On Fri, Mar 6, 2020 at 3:12 PM Simon Marchi wrote: > > On 2020-03-06 3:59 p.m., Christian Biesinger via gdb-patches wrote: > > This is in preparation for deleting deprecated_add_core_fns and > > related code. > > > > As a side-effect, this makes it possible to read NetBSD/ARM > > core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638. > > > > I have removed this comment: > > - /* This is ok: we're running native... */ > > Since we are using the gdbarch from the regcache, we should be > > guaranteed to be calling the right function here, so it shouldn't > > matter whether we are running native. > > > > Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM; > > the "info registers" output matches the one from the system GDB. > > > > gdb/ChangeLog: > > > > 2020-03-05 Christian Biesinger > > > > * Makefile.in (HFILES_NO_SRCDIR): Add new arm-nbsd-tdep.h file. > > * arm-nbsd-nat.c (arm_supply_gregset): Moved to arm-nbsd-tdep and > > renamed to arm_nbsd_supply_gregset. > > (fetch_register): Update to call arm_nbsd_supply_gregset. > > (fetch_regs): Remove in favor of fetch_register with a -1 regno. > > (arm_netbsd_nat_target::fetch_registers): Update. > > (fetch_elfcore_registers): Removed. > > (_initialize_arm_netbsd_nat): Removed call to deprecated_add_core_fns. > > * arm-nbsd-tdep.c (struct arm_nbsd_reg): New struct. > > (arm_nbsd_supply_gregset): Moved from arm-nbsd-nat.c and updated to > > not require NetBSD system headers. > > (arm_nbsd_regset): New struct. > > (arm_nbsd_iterate_over_regset_sections): New function. > > (arm_netbsd_init_abi_common): Updated to call > > set_gdbarch_iterate_over_regset_sections. > > * arm-nbsd-tdep.h: New file. > > Thanks, this looks good to me, but I would appreciate if Kamil to take a quick > look as well. > > I noted some nits below (no need to send another version just for them). > > > @@ -35,6 +37,70 @@ static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11}; > > static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde}; > > static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe}; > > > > +/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */ > > I think it would be nice to provide the link in the comment (a permalink, > in case the file ever moves): > > https://github.com/NetBSD/src/blob/7c13e6e6773bb171f4ed3ed53013e9d24b3c1eac/sys/arch/arm/include/reg.h#L39 > > > +struct arm_nbsd_reg { > > Curly bracket on next line. > > > + uint32_t reg[13]; > > + uint32_t sp; > > + uint32_t lr; > > + uint32_t pc; > > + uint32_t cpsr; > > +}; > > + > > +void > > +arm_nbsd_supply_gregset (const struct regset *regset, struct regcache *regcache, > > + int regnum, const void *gregs, size_t len) > > +{ > > + const arm_nbsd_reg *gregset = static_cast(gregs); > > + gdb_assert (len >= sizeof (arm_nbsd_reg)); > > + > > + /* Integer registers. */ > > + for (int i = ARM_A1_REGNUM; i < ARM_SP_REGNUM; i++) > > + if (regnum == -1 || regnum == i) > > + regcache->raw_supply (i, (char *) &gregset->reg[i]); > > + > > + if (regnum == -1 || regnum == ARM_SP_REGNUM) > > + regcache->raw_supply (ARM_SP_REGNUM, (char *) &gregset->sp); > > + if (regnum == -1 || regnum == ARM_LR_REGNUM) > > + regcache->raw_supply (ARM_LR_REGNUM, (char *) &gregset->lr); > > + if (regnum == -1 || regnum == ARM_PC_REGNUM) > > + { > > + CORE_ADDR r_pc = gdbarch_addr_bits_remove (regcache->arch (), gregset->pc); > > + regcache->raw_supply (ARM_PC_REGNUM, (char *) &r_pc); > > + } > > Can you add some empty lines between each separate condition? I find this > looks too much like if/else if. > > Simon >