From patchwork Wed May 24 19:45:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 20569 Received: (qmail 96052 invoked by alias); 24 May 2017 19:45:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 95922 invoked by uid 89); 24 May 2017 19:45:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: EUR01-HE1-obe.outbound.protection.outlook.com Received: from mail-he1eur01on0050.outbound.protection.outlook.com (HELO EUR01-HE1-obe.outbound.protection.outlook.com) (104.47.0.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 May 2017 19:45:03 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) by AM3PR08MB0102.eurprd08.prod.outlook.com (10.160.211.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1124.9; Wed, 24 May 2017 19:45:01 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::f0a8:fd0f:69e1:e280]) by AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::f0a8:fd0f:69e1:e280%17]) with mapi id 15.01.1124.009; Wed, 24 May 2017 19:45:01 +0000 From: Alan Hayward To: Pedro Alves CC: Yao Qi , "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (2/4) Date: Wed, 24 May 2017 19:45:01 +0000 Message-ID: <5A105765-C70D-413C-BB35-50BAA5FD5865@arm.com> References: <3C00280E-37C9-4C0A-9DA6-F3B9DB1A6E8F@arm.com> <86y3v7uf9j.fsf@gmail.com> <806B436F-EFA1-4200-AC54-9036D166C9B9@arm.com> <867f1m8nhm.fsf@gmail.com> <8637bx9jsw.fsf@gmail.com> <78A7E8EA-7203-44DF-B7FD-63E75A5ECEF5@arm.com> <540372d8-efc3-f842-5cac-cd813bacc3f5@redhat.com> <4F90CD36-759D-4BDA-BFEC-8DD86F44A0B7@arm.com> <40597975-9458-e9af-8915-9d303bb1ed98@redhat.com> In-Reply-To: authentication-results: redhat.com; dkim=none (message not signed) header.d=none; redhat.com; dmarc=none action=none header.from=arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0102; 7:mg9/vSwTMMj+UyKxrea8rQY7j0G6Z5Lgayek1icMf5dxvlToETaiTtg7MOXbMPWk2wGjMN9qJ2U6NSSpSWuoKZkFfMCToOYfcfkPdovj+GcSvTtqYLczqSg3orcrOvUNcbUXS0pf0+a0aoyD8iqeaTh/WIJo2n7P380dvT1MbXNb0aSlS/HN6zA4DAqcU5rDpSIHbbvnOTwtdRvq+2ChgPdDmUhiLS7jfx2u+KY8PUrubpZQAHKzaIxT3pR33Ui1t1Z9H5TnpcSDsrwiLAGxCtmTra+fKa32V1ztCzcSsGzL4JZBZJmNomw55FlKgonvnsspcQP2bN9Ac3E1Dsr5RQ== x-ms-traffictypediagnostic: AM3PR08MB0102: x-ms-office365-filtering-correlation-id: 3d79a66a-067a-4e4c-f10b-08d4a2dd5df1 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(48565401081)(201703131423075)(201703031133081)(201702281549075); SRVR:AM3PR08MB0102; nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3002001)(6055026)(6041248)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(20161123564025)(20161123555025)(20161123560025)(6072148); SRVR:AM3PR08MB0102; BCL:0; PCL:0; RULEID:; SRVR:AM3PR08MB0102; x-forefront-prvs: 031763BCAF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(979002)(6009001)(39400400002)(39860400002)(39840400002)(39410400002)(39850400002)(39450400003)(51444003)(377454003)(24454002)(377424004)(76176999)(6116002)(3846002)(102836003)(6246003)(86362001)(110136004)(6436002)(38730400002)(4326008)(25786009)(50986999)(54356999)(53546009)(33656002)(5250100002)(6916009)(2950100002)(81166006)(93886004)(5660300001)(66066001)(8676002)(575784001)(2900100001)(229853002)(6506006)(6486002)(189998001)(36756003)(8936002)(3280700002)(3660700001)(72206003)(53936002)(305945005)(7736002)(83716003)(478600001)(2906002)(6512007)(39060400002)(82746002)(99286003)(54906002)(969003)(989001)(999001)(1009001)(1019001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR08MB0102; H:AM3PR08MB0101.eurprd08.prod.outlook.com; FPR:; SPF:None; MLV:ovrnspm; PTR:InfoNoRecords; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <3A15A85D1541E948B73D47803B8BB2BC@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 24 May 2017 19:45:01.2340 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0102 > On 24 May 2017, at 12:07, Pedro Alves wrote: > > On 05/24/2017 11:20 AM, Alan Hayward wrote: > >>>> Meanwhile raw_collect_integer doesn’t need a signed parameter: >>> >>> Wouldn't we need to support ADDR_LEN larger than the register size? >> >> This might be me misunderstanding gdb, >> But I assumed that addr would always be shorter than the register size. >> If addr is bigger than the register size then the most significant bits will >> be chopped off (including the sign), which I think would be a bad idea? > > Yeah, the case of a 32-bit register being given a 64-bit slot in a ptrace > register buffer is actually not unheard of. For example the > segment registers on x86-64 (cs, ss, ds, etc.) are 32-bit in > gdb's register cache, but Linux ptrace transfers them as 64-bit > [see /usr/include/sys/reg.h]. I'm not exactly sure whether > in such cases we end up needing to sign/zero extend when copying > back, or whether the kernel ignores the upper bits. I think that > on x86 we just copy the lower 4 bytes and leave the upper ones as > they were, so probably the latter. The MIPS architecture is special > around addresses being signed though, and given the existing code, > I'd play it safe and keep the collect/store functions mirrors - if > one truncates, the other fills/extends, and vice versa. There's also > > /* Is the target using 64-bit raw integer registers but only > storing a left-aligned 32-bit value in each? */ > int mips64_transfers_32bit_regs_p; > > which most probably doesn't apply in this case (FreeBSD, while > I think that was originally added for remote), but it compounds in > the weirdness. > > Thanks, > Pedro Alves > Added copy_integer_to_size, and removed the templates. Manually tested of copy_integer_to_size to make sure the signs and endian parts all work. Tested on a --enable-targets=all build using make check with board files unix and native-gdbserver. I do not have a MIPS machine to test on. Ok to commit? Alan. 2017-05-24 Alan Hayward * gdb/defs.h (copy_integer_to_size): New declaration. * gdb/findvar.c (extract_signed_integer): Removed function. (extract_unsigned_integer): Likewise. (store_signed_integer): Removed function. (store_unsigned_integer): Likewise. * mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use raw_supply_integer. (mips_fbsd_collect_reg): Use templated raw_collect_integer. * mips-linux-tdep.c (supply_32bit_reg): Use raw_supply_integer. (mips64_fill_gregset): Use raw_collect_integer. (mips64_fill_fpregset): Use raw_supply_integer. * gdb/regcache.c (regcache::raw_supply_integer): New function. (regcache::raw_collect_integer): Likewise * gdb/regcache.h (regcache::raw_supply): New declaration. (regcache::raw_collect): Likewise diff --git a/gdb/defs.h b/gdb/defs.h index a0b586f401eca205334e9f237081f4da97c83aa1..a1a97bb1e791d4f423788797d1f04c3e89877d90 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -658,7 +658,10 @@ extern void store_unsigned_integer (gdb_byte *, int, extern void store_typed_address (gdb_byte *buf, struct type *type, CORE_ADDR addr); - +extern void copy_integer_to_size (gdb_byte *dest, int dest_size, + const gdb_byte *source, int source_size, + bool is_signed, enum bfd_endian byte_order); + /* From valops.c */ extern int watchdog; diff --git a/gdb/findvar.c b/gdb/findvar.c index ed4d5c1266c9de069981b306bc8229ae5fb02350..5a82e493f9ca6d9337a22defc4377235f36acba8 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -249,7 +249,47 @@ store_typed_address (gdb_byte *buf, struct type *type, CORE_ADDR addr) gdbarch_address_to_pointer (get_type_arch (type), type, buf, addr); } +/* Copy a value from SOURCE of size SOURCE_SIZE bytes to DEST of size DEST_SIZE + bytes. If SOURCE_SIZE is greater than DEST_SIZE, then truncate the most + significant bytes. If SOURCE_SIZE is less than DEST_SIZE then either sign + or zero extended according to IS_SIGNED. Values are stored in memory with + endianess BYTE_ORDER. */ +void +copy_integer_to_size (gdb_byte *dest, int dest_size, const gdb_byte *source, + int source_size, bool is_signed, + enum bfd_endian byte_order) +{ + signed int size_diff = dest_size - source_size; + + /* Copy across everything from SOURCE that can fit into DEST. */ + + if (byte_order == BFD_ENDIAN_BIG && size_diff > 0) + memcpy (dest + size_diff, source, source_size); + else if (byte_order == BFD_ENDIAN_BIG && size_diff < 0) + memcpy (dest, source - size_diff, dest_size); + else + memcpy (dest, source, std::min (source_size, dest_size)); + + /* Fill the remaining space in DEST by either zero extending or sign + extending. */ + + if (size_diff > 0) + { + char extension = 0; + if (is_signed) + if ((byte_order == BFD_ENDIAN_BIG && source[0] & 0x80) + || (byte_order != BFD_ENDIAN_BIG + && source[source_size - 1] & 0x80)) + extension = 0xff; + + /* Extend into MSBs of SOURCE. */ + if (byte_order == BFD_ENDIAN_BIG) + memset (dest, extension, size_diff); + else + memset (dest + source_size, extension, size_diff); + } +} /* Return a `value' with the contents of (virtual or cooked) register REGNUM as found in the specified FRAME. The register's type is diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..13cf98585f96f1acfe6decbe320530d609bee646 100644 --- a/gdb/mips-fbsd-tdep.c +++ b/gdb/mips-fbsd-tdep.c @@ -47,57 +47,24 @@ 34th is a dummy for padding. */ #define MIPS_FBSD_NUM_FPREGS 34 -/* Supply a single register. If the source register size matches the - size the regcache expects, this can use regcache_raw_supply(). If - they are different, this copies the source register into a buffer - that can be passed to regcache_raw_supply(). */ +/* Supply a single register. The register size might not match, so use + regcache->raw_supply_integer (). */ static void mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr, size_t len) { - struct gdbarch *gdbarch = get_regcache_arch (regcache); - - if (register_size (gdbarch, regnum) == len) - regcache_raw_supply (regcache, regnum, addr); - else - { - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - gdb_byte buf[MAX_REGISTER_SIZE]; - LONGEST val; - - val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order); - store_signed_integer (buf, register_size (gdbarch, regnum), byte_order, - val); - regcache_raw_supply (regcache, regnum, buf); - } + regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, len, true); } -/* Collect a single register. If the destination register size - matches the size the regcache expects, this can use - regcache_raw_supply(). If they are different, this fetches the - register via regcache_raw_supply() into a buffer and then copies it - into the final destination. */ +/* Collect a single register. The register size might not match, so use + regcache->raw_collect_integer (). */ static void mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr, size_t len) { - struct gdbarch *gdbarch = get_regcache_arch (regcache); - - if (register_size (gdbarch, regnum) == len) - regcache_raw_collect (regcache, regnum, addr); - else - { - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - gdb_byte buf[MAX_REGISTER_SIZE]; - LONGEST val; - - regcache_raw_collect (regcache, regnum, buf); - val = extract_signed_integer (buf, register_size (gdbarch, regnum), - byte_order); - store_signed_integer ((gdb_byte *) addr, len, byte_order, val); - } + regcache->raw_collect_integer (regnum, (gdb_byte *) addr, len, true); } /* Supply the floating-point registers stored in FPREGS to REGCACHE. diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c index 48a582a16c934abe6e8f87c46a6009649c606d49..ccfdcdf98bc1e528cd768efaaaffaa3405708f71 100644 --- a/gdb/mips-linux-tdep.c +++ b/gdb/mips-linux-tdep.c @@ -116,13 +116,7 @@ mips_linux_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc) static void supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr) { - struct gdbarch *gdbarch = get_regcache_arch (regcache); - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - gdb_byte buf[MAX_REGISTER_SIZE]; - store_signed_integer (buf, register_size (gdbarch, regnum), byte_order, - extract_signed_integer ((const gdb_byte *) addr, 4, - byte_order)); - regcache_raw_supply (regcache, regnum, buf); + regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, 4, true); } /* Unpack an elf_gregset_t into GDB's register cache. */ @@ -417,7 +411,6 @@ mips64_fill_gregset (const struct regcache *regcache, mips64_elf_gregset_t *gregsetp, int regno) { struct gdbarch *gdbarch = get_regcache_arch (regcache); - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); int regaddr, regi; mips64_elf_greg_t *regp = *gregsetp; void *dst; @@ -460,14 +453,8 @@ mips64_fill_gregset (const struct regcache *regcache, if (regaddr != -1) { - gdb_byte buf[MAX_REGISTER_SIZE]; - LONGEST val; - - regcache_raw_collect (regcache, regno, buf); - val = extract_signed_integer (buf, register_size (gdbarch, regno), - byte_order); dst = regp + regaddr; - store_signed_integer ((gdb_byte *) dst, 8, byte_order, val); + regcache->raw_collect_integer (regno, (gdb_byte *) dst, 8, true); } } @@ -564,25 +551,13 @@ mips64_fill_fpregset (const struct regcache *regcache, } else if (regno == mips_regnum (gdbarch)->fp_control_status) { - gdb_byte buf[MAX_REGISTER_SIZE]; - LONGEST val; - - regcache_raw_collect (regcache, regno, buf); - val = extract_signed_integer (buf, register_size (gdbarch, regno), - byte_order); to = (gdb_byte *) (*fpregsetp + 32); - store_signed_integer (to, 4, byte_order, val); + regcache->raw_collect_integer (regno, to, 4, true); } else if (regno == mips_regnum (gdbarch)->fp_implementation_revision) { - gdb_byte buf[MAX_REGISTER_SIZE]; - LONGEST val; - - regcache_raw_collect (regcache, regno, buf); - val = extract_signed_integer (buf, register_size (gdbarch, regno), - byte_order); to = (gdb_byte *) (*fpregsetp + 32) + 4; - store_signed_integer (to, 4, byte_order, val); + regcache->raw_collect_integer (regno, to, 4, true); } else if (regno == -1) { diff --git a/gdb/regcache.h b/gdb/regcache.h index 4dcfccbac70f0f962bf5e5596d035fda42322795..409482d17c0542c7a53620d88d33fa9706fa72c5 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -294,8 +294,14 @@ public: void raw_collect (int regnum, void *buf) const; + void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len, + bool is_signed) const; + void raw_supply (int regnum, const void *buf); + void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len, + bool is_signed); + void raw_supply_zeroed (int regnum); void raw_copy (int regnum, struct regcache *src_regcache); diff --git a/gdb/regcache.c b/gdb/regcache.c index 660558f7ff10f9d8346b6e08422e16c38c3c4d7d..ec6446b897922a8f9f44bbf94b7f1d198b0a6d4b 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -1189,6 +1189,28 @@ regcache::raw_supply (int regnum, const void *buf) } } +/* Supply register REGNUM with an integer, whose contents are stored in ADDR, + with length ADDR_LEN and sign IS_SIGNED, to REGCACHE. */ + +void +regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len, + bool is_signed) +{ + enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch); + gdb_byte *regbuf; + size_t regsize; + + gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); + gdb_assert (!m_readonly_p); + + regbuf = register_buffer (regnum); + regsize = m_descr->sizeof_register[regnum]; + + copy_integer_to_size (regbuf, regsize, addr, addr_len, is_signed, + byte_order); + m_register_status[regnum] = REG_VALID; +} + /* Supply register REGNUM with zeroed value to REGCACHE. This is not the same as calling raw_supply with NULL (which will set the state to unavailable). */ @@ -1232,6 +1254,26 @@ regcache::raw_collect (int regnum, void *buf) const memcpy (buf, regbuf, size); } +/* Collect register REGNUM from regcache to an integer, whose contents are + stored in ADDR, with length ADDR_LEN and sign IS_SIGNED. */ + +void +regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len, + bool is_signed) const +{ + enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch); + const gdb_byte *regbuf; + size_t regsize; + + gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); + + regbuf = register_buffer (regnum); + regsize = m_descr->sizeof_register[regnum]; + + copy_integer_to_size (addr, addr_len, regbuf, regsize, is_signed, + byte_order); +} + void regcache::raw_copy (int regnum, struct regcache *src_regcache) {