From patchwork Tue Feb 7 16:33:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 19147 Received: (qmail 38330 invoked by alias); 7 Feb 2017 16:33:46 -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 38286 invoked by uid 89); 7 Feb 2017 16:33:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL, BAYES_50, KAM_LOTSOFHASH, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=no version=3.3.2 spammy=bright, 12506, 1876, qiyaoltc@gmail.com X-HELO: EUR01-VE1-obe.outbound.protection.outlook.com Received: from mail-ve1eur01on0045.outbound.protection.outlook.com (HELO EUR01-VE1-obe.outbound.protection.outlook.com) (104.47.1.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Feb 2017 16:33:25 +0000 Received: from VI1PR0801MB1822.eurprd08.prod.outlook.com (10.168.68.7) by VI1PR0801MB1823.eurprd08.prod.outlook.com (10.168.68.8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.888.16; Tue, 7 Feb 2017 16:33:19 +0000 Received: from VI1PR0801MB1822.eurprd08.prod.outlook.com ([10.168.68.7]) by VI1PR0801MB1822.eurprd08.prod.outlook.com ([10.168.68.7]) with mapi id 15.01.0888.026; Tue, 7 Feb 2017 16:33:19 +0000 From: Alan Hayward To: Yao Qi CC: Pedro Alves , Joel Brobecker , "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE Date: Tue, 7 Feb 2017 16:33:19 +0000 Message-ID: <5F3D30AE-9A53-493A-B6DC-DF594C2FAB18@arm.com> References: <45e3a5e1-a9aa-1bc0-5d08-526b89fc458e@redhat.com> <20170201124123.GA27498@E107787-LIN> <20170202094012.dge4r6rsl2skdrii@adacore.com> <20170203102819.GA11916@E107787-LIN> <25716edf-096e-20c5-4170-fb8ca04d897b@redhat.com> <0C6A0D51-4C49-4400-8C46-E77DD512DF56@arm.com> <20170203165022.GB11916@E107787-LIN> <1E0030CE-FB37-4821-AA53-9C6D1CC285C9@arm.com> <20170206152635.GE11916@E107787-LIN> In-Reply-To: <20170206152635.GE11916@E107787-LIN> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-exchange-messagesentrepresentingtype: 1 x-ms-office365-filtering-correlation-id: 18f72d5d-45a9-4fc3-2cc6-08d44f7706ab x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081); SRVR:VI1PR0801MB1823; x-microsoft-exchange-diagnostics: 1; VI1PR0801MB1823; 7:zcwZ0UPuv2hWu10gyez0GRDJu6pjkqSZoyiq+NqRMGPwfKGkbazQ3i6dXf2+4UtN1uK+8AICC0iA80PYqpnZzwEfFrI1F6iIRidJT8yNgrq+vL5gKp+7EJQKv1orAXNIhC1RpjbhXhKbu0fymEqscq/NYEE1jW8Kn/CDWum38J8gHdF9PBcW6TF98l+peUKd/Ec3nRfHAfzoVVWXpff82impgc3Eo755at5+YUmzb8LTE29bLDj5nUH5+J2iN+vPhb+cmul1YhGsbQmtlBZ7iBOU1ydBGj00HJPMoevqKqt8M3LOQODlvhisLBFmHLS4TzRk4DA9XeR/Ueh7WhsXMLvz9/EDZRN+xi82wd/mT1UxSzSdU31Mpn9B5V7VNP37ks26Toq4RnqmeLNDTbu/qKieenNmIl/lOUk1ag5KTzOFLvZ/5TwFvhb4PssWwyGaLm7tQXWqlrPNArhEgKLPRU4ZXFIyDc1uWwbl5NttYaHZam1Tv9E6C7PM/PIgibP8VpY2oTOs4Ylhb+v/XLXwHg== nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(788757137089); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(2017020603029)(8121501046)(5005006)(20170203043)(3002001)(10201501046)(6055026)(6041248)(20161123555025)(20161123558025)(20161123560025)(20161123564025)(20161123562025)(6072148); SRVR:VI1PR0801MB1823; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0801MB1823; x-forefront-prvs: 0211965D06 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(39860400002)(39850400002)(39410400002)(39840400002)(39450400003)(377424004)(24454002)(189002)(199003)(2906002)(4326007)(53936002)(53546003)(6246003)(92566002)(82746002)(86362001)(575784001)(16200700003)(53946003)(93886004)(189998001)(66066001)(83716003)(122556002)(305945005)(7736002)(77096006)(229853002)(68736007)(25786008)(39060400001)(105586002)(106116001)(106356001)(6486002)(6506006)(6436002)(97736004)(3280700002)(8936002)(6512007)(54356999)(76176999)(50986999)(3660700001)(99286003)(54906002)(36756003)(101416001)(1411001)(102836003)(3846002)(2900100001)(6116002)(81166006)(5660300001)(81156014)(6916009)(8676002)(2950100002)(110136004)(38730400002)(33656002)(104396002)(569005); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0801MB1823; H:VI1PR0801MB1822.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <1648D5803BE0D34AA0EE3BBE5913C6B3@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Feb 2017 16:33:19.5134 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0801MB1823 > On 6 Feb 2017, at 15:26, Yao Qi wrote: > > On 17-02-06 09:33:25, Alan Hayward wrote: >> >> Ok, Given that, I've put together a quick patch to: >> * Use M68K_MAX_REGISTER_SIZE and I386_MAX_REGISTER_SIZE where possible. > > They can be in a separate patch if you want. > >> * Add max_register_size() function >> * Remove MAX_REGISTER_SIZE from all common files by using std::vector. >> Contents should be very similar to previous patches posted. >> >> This patch ok? >> >> 2017-02-06 Alan Hayward >> >> * frame.c (frame_unwind_register_signed): Use std::vector. >> (frame_unwind_register_unsigned): Likewise. >> (get_frame_register_bytes): Likewise. >> (put_frame_register_bytes): Likewise. >> * i386-tdep.c (i386_pseudo_register_read_into_value): Use >> I386_MAX_REGISTER_SIZE. >> (i386_pseudo_register_write): Likewise. >> (i386_process_record): Likewise. >> * i387-tdep.c (i387_supply_xsave): Likewise. >> * m68k-linux-nat.c (fetch_register): Use M68K_MAX_REGISTER_SIZE. >> (store_register): Likewise. >> * mi/mi-main.c (register_changed_p): Use std::vector. >> * record-full.c (record_full_exec_insn): Likewise. >> (record_full_core_open_1): Use max_register_size (). >> (record_full_core_fetch_registers): Likewise. >> (record_full_core_store_registers): Likewise. >> (init_record_full_core_ops): Likewise. >> * remote-sim.c (gdbsim_fetch_register): Likewise. >> (gdbsim_store_register): Likewise. >> * regcache.c (struct regcache_descr): Add max_register_size. >> (max_register_size): New. >> (init_regcache_descr): Find max register size. >> (regcache_save): Use std::vector. >> (regcache_restore): Likewise. >> (regcache_dump): Likewise. >> * regcache.h (max_register_size): New. >> * remote.c (remote_prepare_to_store): Use std::vector. >> * sol-thread.c (sol_thread_store_registers): Likewise. >> * stack.c (frame_info): Likewise. >> * target.c (debug_print_register): Likewise. >> >> >> diff --git a/gdb/frame.c b/gdb/frame.c >> index d98003dee7c34a63bd25356e6674721664a4b2f3..1700308371d9d795545c0a8bb26dba289b18217d 100644 >> --- a/gdb/frame.c >> +++ b/gdb/frame.c >> @@ -1252,10 +1252,10 @@ frame_unwind_register_signed (struct frame_info *frame, int regnum) >> struct gdbarch *gdbarch = frame_unwind_arch (frame); >> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >> int size = register_size (gdbarch, regnum); >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (size); >> >> - frame_unwind_register (frame, regnum, buf); >> - return extract_signed_integer (buf, size, byte_order); >> + frame_unwind_register (frame, regnum, buf.data ()); >> + return extract_signed_integer (buf.data (), size, byte_order); > > We can replace MAX_REGISTER_SIZE with 'sizeof (LONGEST)', because > extract_signed_integer checks 'size' shouldn't be greater than > 'sizeof (LONGEST)'. > > if (len > (int) sizeof (LONGEST)) > error (_("\ > That operation is not available on integers of more than %d bytes."), > (int) sizeof (LONGEST)); Ok. > >> } >> >> LONGEST >> @@ -1270,10 +1270,10 @@ frame_unwind_register_unsigned (struct frame_info *frame, int regnum) >> struct gdbarch *gdbarch = frame_unwind_arch (frame); >> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >> int size = register_size (gdbarch, regnum); >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (size); > > Likewise, > replace MAX_REGISTER_SIZE with 'sizeof (ULONGEST)’ Ok. > >> >> - frame_unwind_register (frame, regnum, buf); >> - return extract_unsigned_integer (buf, size, byte_order); >> + frame_unwind_register (frame, regnum, buf.data ()); >> + return extract_unsigned_integer (buf.data (), size, byte_order); >> } >> >> ULONGEST >> @@ -1389,6 +1389,8 @@ get_frame_register_bytes (struct frame_info *frame, int regnum, >> error (_("Bad debug information detected: " >> "Attempt to read %d bytes from registers."), len); >> >> + std::vector buf (max_register_size (gdbarch)); >> + >> /* Copy the data. */ >> while (len > 0) >> { >> @@ -1410,16 +1412,15 @@ get_frame_register_bytes (struct frame_info *frame, int regnum, >> } >> else >> { >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> enum lval_type lval; >> CORE_ADDR addr; >> int realnum; >> >> frame_register (frame, regnum, optimizedp, unavailablep, >> - &lval, &addr, &realnum, buf); >> + &lval, &addr, &realnum, buf.data ()); > > I am wondering that we can replace frame_register with > > value = frame_unwind_register_value (frame->next, regnum); > > because frame_register -> frame_register_unwind -> frame_unwind_register_value > and we can get register contents from value, so we don't need 'buf' at all. Ok. > >> if (*optimizedp || *unavailablep) >> return 0; >> - memcpy (myaddr, buf + offset, curr_len); >> + memcpy (myaddr, buf.data () + offset, curr_len); >> } >> >> myaddr += curr_len; >> @@ -1446,6 +1447,8 @@ put_frame_register_bytes (struct frame_info *frame, int regnum, >> regnum++; >> } >> >> + std::vector buf (max_register_size (gdbarch)); >> + >> /* Copy the data. */ >> while (len > 0) >> { >> @@ -1460,11 +1463,9 @@ put_frame_register_bytes (struct frame_info *frame, int regnum, >> } >> else >> { >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> - >> - deprecated_frame_register_read (frame, regnum, buf); >> - memcpy (buf + offset, myaddr, curr_len); >> - put_frame_register (frame, regnum, buf); >> + deprecated_frame_register_read (frame, regnum, buf.data ()); >> + memcpy (buf.data () + offset, myaddr, curr_len); >> + put_frame_register (frame, regnum, buf.data ()); > > Can you try using frame_unwind_register_value to read the register value? > Get the value content, patch it, and pass it to put_frame_register. Yes. Technically, the value content is a const, but we need to cast from void* to char* anyway. > >> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >> index 57c23ebf5d6b2d3b398aa40ebd9b3cb70c56125c..b07335d82723f3ee9c7602a8fa284acb7e3b3a25 100644 >> --- a/gdb/mi/mi-main.c >> +++ b/gdb/mi/mi-main.c >> @@ -1135,8 +1135,8 @@ register_changed_p (int regnum, struct regcache *prev_regs, >> struct regcache *this_regs) >> { >> struct gdbarch *gdbarch = get_regcache_arch (this_regs); >> - gdb_byte prev_buffer[MAX_REGISTER_SIZE]; >> - gdb_byte this_buffer[MAX_REGISTER_SIZE]; >> + std::vector prev_buffer (max_register_size (gdbarch)); > > register_size (gdbarch, regnum) instead of max_register_size? Ok. > >> + std::vector this_buffer (max_register_size (gdbarch)); >> enum register_status prev_status; >> enum register_status this_status; >> >> @@ -1146,13 +1146,13 @@ register_changed_p (int regnum, struct regcache *prev_regs, >> return 1; >> >> /* Get register contents and compare. */ >> - prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer); >> - this_status = regcache_cooked_read (this_regs, regnum, this_buffer); >> + prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer.data ()); >> + this_status = regcache_cooked_read (this_regs, regnum, this_buffer.data ()); >> >> if (this_status != prev_status) >> return 1; >> else if (this_status == REG_VALID) >> - return memcmp (prev_buffer, this_buffer, >> + return memcmp (prev_buffer.data (), this_buffer.data (), >> register_size (gdbarch, regnum)) != 0; >> else >> return 0; > >> diff --git a/gdb/record-full.c b/gdb/record-full.c >> index fdd613b6e41bbfcd8644b02ccfeb98b53b518bff..a619e09a646c48c632a60407d7018979d805a08f 100644 >> --- a/gdb/record-full.c >> +++ b/gdb/record-full.c >> @@ -698,7 +698,7 @@ record_full_exec_insn (struct regcache *regcache, >> { >> case record_full_reg: /* reg */ >> { >> - gdb_byte reg[MAX_REGISTER_SIZE]; >> + std::vector reg (register_size (gdbarch, entry->u.reg.num)); >> >> if (record_debug > 1) >> fprintf_unfiltered (gdb_stdlog, >> @@ -707,10 +707,10 @@ record_full_exec_insn (struct regcache *regcache, >> host_address_to_string (entry), >> entry->u.reg.num); >> >> - regcache_cooked_read (regcache, entry->u.reg.num, reg); >> - regcache_cooked_write (regcache, entry->u.reg.num, >> + regcache_cooked_read (regcache, entry->u.reg.num, reg.data ()); >> + regcache_cooked_write (regcache, entry->u.reg.num, >> record_full_get_loc (entry)); >> - memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); >> + memcpy (record_full_get_loc (entry), reg.data (), entry->u.reg.len); > > Looks the code above copies register entry->u.reg.num contents from > regcache to record_full_get_loc (entry), so we may rewrite the code using > regcache_raw_collect, > > regcache_raw_collect (regcache, entry->u.reg.num, record_full_get_loc (entry)); > > so array reg is not needed anymore. Let me post a patch to fix it. Looks to me like the three lines of code are swapping the contents of regcache and record_full_get_loc. temp <= regcache regcache <= record_full_get_loc record_full_get_loc <= temp I can’t see a way of doing this without the buffer. Left code as is in patch. > >> } >> break; >> >> @@ -792,15 +792,17 @@ static void >> record_full_core_open_1 (const char *name, int from_tty) >> { >> struct regcache *regcache = get_current_regcache (); >> - int regnum = gdbarch_num_regs (get_regcache_arch (regcache)); >> + struct gdbarch *gdbarch = get_regcache_arch (regcache); >> + int regnum = gdbarch_num_regs (gdbarch); >> + int regmaxsize = max_register_size (gdbarch); >> int i; >> >> /* Get record_full_core_regbuf. */ >> target_fetch_registers (regcache, -1); >> - record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum); >> + record_full_core_regbuf = (gdb_byte *) xmalloc (regmaxsize * regnum); >> for (i = 0; i < regnum; i ++) >> regcache_raw_collect (regcache, i, >> - record_full_core_regbuf + MAX_REGISTER_SIZE * i); >> + record_full_core_regbuf + regmaxsize * i); >> >> /* Get record_full_core_start and record_full_core_end. */ >> if (build_section_table (core_bfd, &record_full_core_start, >> @@ -2038,6 +2040,9 @@ record_full_core_fetch_registers (struct target_ops *ops, >> struct regcache *regcache, >> int regno) >> { >> + struct gdbarch *gdbarch = get_regcache_arch (regcache); >> + int regmaxsize = max_register_size (gdbarch); >> + >> if (regno < 0) >> { >> int num = gdbarch_num_regs (get_regcache_arch (regcache)); >> @@ -2045,11 +2050,11 @@ record_full_core_fetch_registers (struct target_ops *ops, >> >> for (i = 0; i < num; i ++) >> regcache_raw_supply (regcache, i, >> - record_full_core_regbuf + MAX_REGISTER_SIZE * i); >> + record_full_core_regbuf + regmaxsize * i); >> } >> else >> regcache_raw_supply (regcache, regno, >> - record_full_core_regbuf + MAX_REGISTER_SIZE * regno); >> + record_full_core_regbuf + regmaxsize * regno); >> } >> >> /* "to_prepare_to_store" method for prec over corefile. */ >> @@ -2067,9 +2072,12 @@ record_full_core_store_registers (struct target_ops *ops, >> struct regcache *regcache, >> int regno) >> { >> + struct gdbarch *gdbarch = get_regcache_arch (regcache); >> + int regmaxsize = max_register_size (gdbarch); >> + >> if (record_full_gdb_operation_disable) >> regcache_raw_collect (regcache, regno, >> - record_full_core_regbuf + MAX_REGISTER_SIZE * regno); >> + record_full_core_regbuf + regmaxsize * regno); >> else >> error (_("You can't do that without a process to debug.")); >> } >> @@ -2265,7 +2273,7 @@ init_record_full_core_ops (void) >> record_full_reg: >> 1 byte: record type (record_full_reg, see enum record_full_type). >> 8 bytes: register id (network byte order). >> - MAX_REGISTER_SIZE bytes: register value. >> + max_register_size bytes: register value. >> record_full_mem: >> 1 byte: record type (record_full_mem, see enum record_full_type). >> 8 bytes: memory length (network byte order). > > This comment is about log file format version 1, but we are using version 2, IIUC. > We probably need to remove all the comments for version 1. > Ok, removed all of Version 1 from section. > >> @@ -327,7 +341,7 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read, >> void *src) >> { >> struct gdbarch *gdbarch = dst->descr->gdbarch; >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (max_register_size (gdbarch)); >> int regnum; >> >> /* The DST should be `read-only', if it wasn't then the save would >> @@ -346,10 +360,10 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read, >> { >> if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup)) >> { >> - enum register_status status = cooked_read (src, regnum, buf); >> + enum register_status status = cooked_read (src, regnum, buf.data ()); >> >> if (status == REG_VALID) >> - memcpy (register_buffer (dst, regnum), buf, >> + memcpy (register_buffer (dst, regnum), buf.data (), >> register_size (gdbarch, regnum)); >> else >> { > > Did you try removing variable 'buf'? like this, > > enum register_status status = cooked_read (src, regnum, register_buffer (dst, regnum)); > > if (status != REG_VALID) > { > gdb_assert (status != REG_UNKNOWN); > > memset (register_buffer (dst, regnum), 0, > register_size (gdbarch, regnum)); > } > > I suppose function cooked_read shouldn't overflow the buffer (its third > argument). Ok. > >> @@ -369,7 +383,7 @@ regcache_restore (struct regcache *dst, >> void *cooked_read_context) >> { >> struct gdbarch *gdbarch = dst->descr->gdbarch; >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (max_register_size (gdbarch)); >> int regnum; >> >> /* The dst had better not be read-only. If it is, the `restore' >> @@ -385,9 +399,9 @@ regcache_restore (struct regcache *dst, >> { >> enum register_status status; >> >> - status = cooked_read (cooked_read_context, regnum, buf); >> + status = cooked_read (cooked_read_context, regnum, buf.data ()); >> if (status == REG_VALID) >> - regcache_cooked_write (dst, regnum, buf); >> + regcache_cooked_write (dst, regnum, buf.data ()); >> } >> } >> } >> @@ -1279,7 +1293,7 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, >> int footnote_register_offset = 0; >> int footnote_register_type_name_null = 0; >> long register_offset = 0; >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (max_register_size (gdbarch)); >> >> #if 0 >> fprintf_unfiltered (file, "nr_raw_registers %d\n", >> @@ -1406,8 +1420,8 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, >> fprintf_unfiltered (file, ""); >> else >> { >> - regcache_raw_read (regcache, regnum, buf); >> - print_hex_chars (file, buf, >> + regcache_raw_read (regcache, regnum, buf.data ()); >> + print_hex_chars (file, buf.data (), > > We can get register contents buffer by register_buffer (regcache, regnum), > but we need to make sure that regcache is up-to-date. > > Probably, we can add a function, > > void > regcache_raw_update (struct regcache *regcache, int regnum) > { > if (!regcache->readonly_p > && regcache_register_status (regcache, regnum) == REG_UNKNOWN) > { > struct cleanup *old_chain = save_inferior_ptid (); > > inferior_ptid = regcache->ptid; > target_fetch_registers (regcache, regnum); > do_cleanups (old_chain); > > /* A number of targets can't access the whole set of raw > registers (because the debug API provides no means to get at > them). */ > if (regcache->register_status[regnum] == REG_UNKNOWN) > regcache->register_status[regnum] = REG_UNAVAILABLE; > } > } > > in regcache_dump, we can call regcache_raw_update, and then get > register contents from register_buffer (regcache, regnum). Ok. > >> regcache->descr->sizeof_register[regnum], >> gdbarch_byte_order (gdbarch)); > > >> } >> @@ -1422,13 +1436,13 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, >> { >> enum register_status status; >> >> - status = regcache_cooked_read (regcache, regnum, buf); >> + status = regcache_cooked_read (regcache, regnum, buf.data ()); >> if (status == REG_UNKNOWN) >> fprintf_unfiltered (file, ""); >> else if (status == REG_UNAVAILABLE) >> fprintf_unfiltered (file, ""); >> else >> - print_hex_chars (file, buf, >> + print_hex_chars (file, buf.data (), >> regcache->descr->sizeof_register[regnum], >> gdbarch_byte_order (gdbarch)); >> } >> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c >> index b0c68c617e365c0ea18ac2eca525598b688ffbb5..c5b1a480c52e12af01dc2d6c959fd429735bc805 100644 >> --- a/gdb/remote-sim.c >> +++ b/gdb/remote-sim.c >> @@ -439,6 +439,9 @@ gdbsim_fetch_register (struct target_ops *ops, >> return; >> } >> >> + int regsize = register_size (gdbarch, regno); >> + std::vector buf (regsize); >> + >> switch (gdbarch_register_sim_regno (gdbarch, regno)) >> { >> case LEGACY_SIM_REGNO_IGNORE: >> @@ -447,10 +450,9 @@ gdbsim_fetch_register (struct target_ops *ops, >> { >> /* For moment treat a `does not exist' register the same way >> as an ``unavailable'' register. */ >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> int nr_bytes; >> >> - memset (buf, 0, MAX_REGISTER_SIZE); >> + memset (buf, 0, regsize); > > memset to std::vector? Doh! > >> regcache_raw_supply (regcache, regno, buf); >> break; >> } >> @@ -458,18 +460,17 @@ gdbsim_fetch_register (struct target_ops *ops, >> default: >> { >> static int warn_user = 1; >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> int nr_bytes; >> >> gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch)); >> - memset (buf, 0, MAX_REGISTER_SIZE); >> + memset (buf, 0, regsize); >> nr_bytes = sim_fetch_register (sim_data->gdbsim_desc, >> gdbarch_register_sim_regno >> (gdbarch, regno), >> buf, > > 'buf' is a std::vector, so how does it compile? Looks like remote-sim.c doesn’t get complied in if you don’t specify a sim at config time. Hacking the makefile, I can get this to build, but I don’t have a sim, so can’t link the code. Fixed up. > >> diff --git a/gdb/remote.c b/gdb/remote.c >> index c4cec910c44cf91cc7f36b7f2d87cde3f46de41e..157a1b1789d2a248c11dcc4efebd8ce54da82045 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -7757,9 +7757,10 @@ remote_fetch_registers (struct target_ops *ops, >> static void >> remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) >> { >> + struct gdbarch *gdbarch = get_regcache_arch (regcache); >> struct remote_arch_state *rsa = get_remote_arch_state (); >> int i; >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (max_register_size (gdbarch)); >> >> /* Make sure the entire registers array is valid. */ >> switch (packet_support (PACKET_P)) >> @@ -7769,7 +7770,7 @@ remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) >> /* Make sure all the necessary registers are cached. */ >> for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++) >> if (rsa->regs[i].in_g_packet) >> - regcache_raw_read (regcache, rsa->regs[i].regnum, buf); >> + regcache_raw_read (regcache, rsa->regs[i].regnum, buf.data ()); > > Use regcache_raw_update. Ok. > >> break; >> case PACKET_ENABLE: >> break; >> diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c >> index a09a3ab9a8bc56f367e3ba1537f5674f0a7f491f..5e7443a97624fbeae5b65cb534401e6371755423 100644 >> --- a/gdb/sol-thread.c >> +++ b/gdb/sol-thread.c >> @@ -514,6 +514,7 @@ sol_thread_store_registers (struct target_ops *ops, >> td_err_e val; >> prgregset_t gregset; >> prfpregset_t fpregset; >> + struct gdbarch *gdbarch = get_regcache_arch (regcache); >> >> if (!ptid_tid_p (inferior_ptid)) >> { >> @@ -535,10 +536,10 @@ sol_thread_store_registers (struct target_ops *ops, >> if (regnum != -1) >> { >> /* Not writing all the registers. */ >> - char old_value[MAX_REGISTER_SIZE]; >> + std::vector old_value (register_size (gdbarch, regnum)); >> >> /* Save new register value. */ >> - regcache_raw_collect (regcache, regnum, old_value); >> + regcache_raw_collect (regcache, regnum, old_value.data ()); >> >> val = p_td_thr_getgregs (&thandle, gregset); >> if (val != TD_OK) >> @@ -550,7 +551,7 @@ sol_thread_store_registers (struct target_ops *ops, >> td_err_string (val)); >> >> /* Restore new register value. */ >> - regcache_raw_supply (regcache, regnum, old_value); >> + regcache_raw_supply (regcache, regnum, old_value.data ()); > > I don't understand why we collect all registers and restore them later. > Will take a deeper look. Looks like the collect and supply are entirely redundant! p_td_thr_getgregs and p_td_thr_getfpregs are simple writing into the newly created gregset/fpregset The function should be doing: * If regnum is a real register number then: read all registers from OS into gregset/fpregset, copy regnum from recache into struct, write struct back to OS * If regnum is -1 then: copy all registers from regcache into gregset/fpregset, write struct to OS. I’ll remove the bogus code. > >> } >> >> fill_gregset (regcache, (gdb_gregset_t *) &gregset, regnum); >> diff --git a/gdb/stack.c b/gdb/stack.c >> index e00e2972cf20bc63917af19f86bf57f1c6b0b5b0..9b84435fe73b9af9b13f01c819f77f83bbb7117b 100644 >> --- a/gdb/stack.c >> +++ b/gdb/stack.c >> @@ -1667,16 +1667,16 @@ frame_info (char *addr_exp, int from_tty) >> { >> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >> int sp_size = register_size (gdbarch, gdbarch_sp_regnum (gdbarch)); >> - gdb_byte value[MAX_REGISTER_SIZE]; >> + std::vector value (sp_size); >> CORE_ADDR sp; >> >> frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch), >> &optimized, &unavailable, &lval, &addr, >> - &realnum, value); >> + &realnum, value.data ()); > > We can use frame_unwind_register_value. > Ok. >> /* NOTE: cagney/2003-05-22: This is assuming that the >> stack pointer was packed as an unsigned integer. That >> may or may not be valid. */ >> - sp = extract_unsigned_integer (value, sp_size, byte_order); >> + sp = extract_unsigned_integer (value.data (), sp_size, byte_order); >> printf_filtered (" Previous frame's sp is "); >> fputs_filtered (paddress (gdbarch, sp), gdb_stdout); >> printf_filtered ("\n"); >> diff --git a/gdb/target.c b/gdb/target.c >> index 3c409f0f619141205dfdcbbf8e46a277585ed683..52bb59255fcbdc74a674e08f6e168be5b6294819 100644 >> --- a/gdb/target.c >> +++ b/gdb/target.c >> @@ -3565,17 +3565,18 @@ debug_print_register (const char * func, >> { >> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >> int i, size = register_size (gdbarch, regno); >> - gdb_byte buf[MAX_REGISTER_SIZE]; >> + std::vector buf (size); >> >> - regcache_raw_collect (regcache, regno, buf); >> + regcache_raw_collect (regcache, regno, buf.data ()); >> fprintf_unfiltered (gdb_stdlog, " = "); >> for (i = 0; i < size; i++) >> { >> - fprintf_unfiltered (gdb_stdlog, "%02x", buf[i]); >> + fprintf_unfiltered (gdb_stdlog, "%02x", buf.data ()[i]); >> } >> if (size <= sizeof (LONGEST)) >> { >> - ULONGEST val = extract_unsigned_integer (buf, size, byte_order); >> + ULONGEST val = extract_unsigned_integer (buf.data (), size, >> + byte_order); >> >> fprintf_unfiltered (gdb_stdlog, " %s %s", >> core_addr_to_string_nz (val), plongest (val)); >> >> > > We can move debug_print_register to regcache.c, so that we can access > register_buffer, and don't need 'buf' at all. > Ok. > -- > Yao (齐尧) New version of the patch with all the changes above. Alan. 2017-02-07 Alan Hayward * frame.c (frame_unwind_register_signed): Use LONGEST for size. (frame_unwind_register_unsigned): Use ULONGEST for size. (get_frame_register_bytes): Unwind register. (put_frame_register_bytes): Likewise. * i386-tdep.c (i386_pseudo_register_read_into_value): Use I386_MAX_REGISTER_SIZE. (i386_pseudo_register_write): Likewise. (i386_process_record): Likewise. * i387-tdep.c (i387_supply_xsave): Likewise. * m68k-linux-nat.c (fetch_register): Use M68K_MAX_REGISTER_SIZE. (store_register): Likewise. * mi/mi-main.c (register_changed_p): Use std::vector. * record-full.c (record_full_exec_insn): Likewise. (record_full_core_open_1): Use max_register_size (). (record_full_core_fetch_registers): Likewise. (record_full_core_store_registers): Likewise. (init_record_full_core_ops): Likewise. * remote-sim.c (gdbsim_fetch_register): Likewise. (gdbsim_store_register): Likewise. * regcache.c (struct regcache_descr): Add max_register_size. (max_register_size): New. (init_regcache_descr): Find max register size. (regcache_save): Use std::vector. (regcache_restore): Likewise. (regcache_raw_update): New function. (regcache_raw_read): Move checks to regcache_raw_update. (regcache_debug_print_register): New function. (regcache_dump): Call regcache_raw_update. Use std::vector. * regcache.h (max_register_size): New. (regcache_raw_update): New declaration. (regcache_debug_print_register): New declaration. * remote.c (remote_prepare_to_store): Call regcache_raw_update. * sol-thread.c (sol_thread_store_registers): Remove superfluous code. * stack.c (frame_info): Unwind sp register. * target.c (debug_print_register): Move to regcache.c (target_fetch_registers): Call regcache_debug_print_register. (target_store_registers): Likewise. diff --git a/gdb/frame.c b/gdb/frame.c index d98003dee7c34a63bd25356e6674721664a4b2f3..22cfdea4bcb20582229ffc360ead060c43d7cd81 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1252,7 +1252,11 @@ frame_unwind_register_signed (struct frame_info *frame, int regnum) struct gdbarch *gdbarch = frame_unwind_arch (frame); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); int size = register_size (gdbarch, regnum); - gdb_byte buf[MAX_REGISTER_SIZE]; + gdb_byte buf[sizeof (LONGEST)]; + + if (size > (int) sizeof (LONGEST)) + error (_("Cannot unwind integers more than %d bytes."), + (int) sizeof (LONGEST)); frame_unwind_register (frame, regnum, buf); return extract_signed_integer (buf, size, byte_order); @@ -1270,7 +1274,11 @@ frame_unwind_register_unsigned (struct frame_info *frame, int regnum) struct gdbarch *gdbarch = frame_unwind_arch (frame); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); int size = register_size (gdbarch, regnum); - gdb_byte buf[MAX_REGISTER_SIZE]; + gdb_byte buf[sizeof (ULONGEST)]; + + if (size > (int) sizeof (ULONGEST)) + error (_("Cannot unwind integers more than %d bytes."), + (int) sizeof (ULONGEST)); frame_unwind_register (frame, regnum, buf); return extract_unsigned_integer (buf, size, byte_order); @@ -1410,16 +1418,21 @@ get_frame_register_bytes (struct frame_info *frame, int regnum, } else { - gdb_byte buf[MAX_REGISTER_SIZE]; - enum lval_type lval; - CORE_ADDR addr; - int realnum; + struct value *value = frame_unwind_register_value (frame->next, + regnum); + gdb_assert (value != NULL); + *optimizedp = value_optimized_out (value); + *unavailablep = !value_entirely_available (value); - frame_register (frame, regnum, optimizedp, unavailablep, - &lval, &addr, &realnum, buf); if (*optimizedp || *unavailablep) - return 0; - memcpy (myaddr, buf + offset, curr_len); + { + release_value (value); + value_free (value); + return 0; + } + memcpy (myaddr, value_contents_all (value) + offset, curr_len); + release_value (value); + value_free (value); } myaddr += curr_len; @@ -1460,11 +1473,15 @@ put_frame_register_bytes (struct frame_info *frame, int regnum, } else { - gdb_byte buf[MAX_REGISTER_SIZE]; - - deprecated_frame_register_read (frame, regnum, buf); - memcpy (buf + offset, myaddr, curr_len); - put_frame_register (frame, regnum, buf); + struct value *value = frame_unwind_register_value (frame->next, + regnum); + gdb_assert (value != NULL); + + memcpy ((char *) value_contents_all (value) + offset, myaddr, + curr_len); + put_frame_register (frame, regnum, value_contents_all (value)); + release_value (value); + value_free (value); } myaddr += curr_len; diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 8a4d59f6fdae8ec785462d0ceedcd6501b955cf0..081a16c6896ce7aee4db3b0be45fbbdd2c23dbdb 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -3250,7 +3250,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, int regnum, struct value *result_value) { - gdb_byte raw_buf[MAX_REGISTER_SIZE]; + gdb_byte raw_buf[I386_MAX_REGISTER_SIZE]; enum register_status status; gdb_byte *buf = value_contents_raw (result_value); @@ -3455,7 +3455,7 @@ void i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, int regnum, const gdb_byte *buf) { - gdb_byte raw_buf[MAX_REGISTER_SIZE]; + gdb_byte raw_buf[I386_MAX_REGISTER_SIZE]; if (i386_mmx_regnum_p (gdbarch, regnum)) { @@ -5037,7 +5037,7 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache, uint32_t opcode; uint8_t opcode8; ULONGEST addr; - gdb_byte buf[MAX_REGISTER_SIZE]; + gdb_byte buf[I386_MAX_REGISTER_SIZE]; struct i386_record_s ir; struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); uint8_t rex_w = -1; diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index adbe72133089bc371108d5dd79bf8d8e61ba259c..fcd5ad248d6b737b9f27e294ce166a118e4bdcad 100644 --- a/gdb/i387-tdep.c +++ b/gdb/i387-tdep.c @@ -899,7 +899,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum, const gdb_byte *regs = (const gdb_byte *) xsave; int i; unsigned int clear_bv; - static const gdb_byte zero[MAX_REGISTER_SIZE] = { 0 }; + static const gdb_byte zero[I386_MAX_REGISTER_SIZE] = { 0 }; enum { none = 0x0, diff --git a/gdb/m68k-linux-nat.c b/gdb/m68k-linux-nat.c index 6944c74eb198381135fda3ddf01b9da3a63e62d5..e5182caf39197f759c85c2321e4d66c428f5911e 100644 --- a/gdb/m68k-linux-nat.c +++ b/gdb/m68k-linux-nat.c @@ -105,7 +105,7 @@ fetch_register (struct regcache *regcache, int regno) struct gdbarch *gdbarch = get_regcache_arch (regcache); long regaddr, val; int i; - gdb_byte buf[MAX_REGISTER_SIZE]; + gdb_byte buf[M68K_MAX_REGISTER_SIZE]; int tid; /* Overload thread id onto process id. */ @@ -160,7 +160,7 @@ store_register (const struct regcache *regcache, int regno) long regaddr, val; int i; int tid; - gdb_byte buf[MAX_REGISTER_SIZE]; + gdb_byte buf[M68K_MAX_REGISTER_SIZE]; /* Overload thread id onto process id. */ tid = ptid_get_lwp (inferior_ptid); diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 57c23ebf5d6b2d3b398aa40ebd9b3cb70c56125c..11702c6c06c5cd791e03d99d56ef93d2c234862b 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1135,8 +1135,8 @@ register_changed_p (int regnum, struct regcache *prev_regs, struct regcache *this_regs) { struct gdbarch *gdbarch = get_regcache_arch (this_regs); - gdb_byte prev_buffer[MAX_REGISTER_SIZE]; - gdb_byte this_buffer[MAX_REGISTER_SIZE]; + std::vector prev_buffer (register_size (gdbarch, regnum)); + std::vector this_buffer (register_size (gdbarch, regnum)); enum register_status prev_status; enum register_status this_status; @@ -1146,13 +1146,13 @@ register_changed_p (int regnum, struct regcache *prev_regs, return 1; /* Get register contents and compare. */ - prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer); - this_status = regcache_cooked_read (this_regs, regnum, this_buffer); + prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer.data ()); + this_status = regcache_cooked_read (this_regs, regnum, this_buffer.data ()); if (this_status != prev_status) return 1; else if (this_status == REG_VALID) - return memcmp (prev_buffer, this_buffer, + return memcmp (prev_buffer.data (), this_buffer.data (), register_size (gdbarch, regnum)) != 0; else return 0; diff --git a/gdb/record-full.c b/gdb/record-full.c index fdd613b6e41bbfcd8644b02ccfeb98b53b518bff..ada86cf6726176d93f61d85ef67290bca46c5b1a 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -698,7 +698,7 @@ record_full_exec_insn (struct regcache *regcache, { case record_full_reg: /* reg */ { - gdb_byte reg[MAX_REGISTER_SIZE]; + std::vector reg (register_size (gdbarch, entry->u.reg.num)); if (record_debug > 1) fprintf_unfiltered (gdb_stdlog, @@ -707,10 +707,10 @@ record_full_exec_insn (struct regcache *regcache, host_address_to_string (entry), entry->u.reg.num); - regcache_cooked_read (regcache, entry->u.reg.num, reg); - regcache_cooked_write (regcache, entry->u.reg.num, + regcache_cooked_read (regcache, entry->u.reg.num, reg.data ()); + regcache_cooked_write (regcache, entry->u.reg.num, record_full_get_loc (entry)); - memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); + memcpy (record_full_get_loc (entry), reg.data (), entry->u.reg.len); } break; @@ -792,15 +792,17 @@ static void record_full_core_open_1 (const char *name, int from_tty) { struct regcache *regcache = get_current_regcache (); - int regnum = gdbarch_num_regs (get_regcache_arch (regcache)); + struct gdbarch *gdbarch = get_regcache_arch (regcache); + int regnum = gdbarch_num_regs (gdbarch); + int regmaxsize = max_register_size (gdbarch); int i; /* Get record_full_core_regbuf. */ target_fetch_registers (regcache, -1); - record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum); + record_full_core_regbuf = (gdb_byte *) xmalloc (regmaxsize * regnum); for (i = 0; i < regnum; i ++) regcache_raw_collect (regcache, i, - record_full_core_regbuf + MAX_REGISTER_SIZE * i); + record_full_core_regbuf + regmaxsize * i); /* Get record_full_core_start and record_full_core_end. */ if (build_section_table (core_bfd, &record_full_core_start, @@ -2038,6 +2040,9 @@ record_full_core_fetch_registers (struct target_ops *ops, struct regcache *regcache, int regno) { + struct gdbarch *gdbarch = get_regcache_arch (regcache); + int regmaxsize = max_register_size (gdbarch); + if (regno < 0) { int num = gdbarch_num_regs (get_regcache_arch (regcache)); @@ -2045,11 +2050,11 @@ record_full_core_fetch_registers (struct target_ops *ops, for (i = 0; i < num; i ++) regcache_raw_supply (regcache, i, - record_full_core_regbuf + MAX_REGISTER_SIZE * i); + record_full_core_regbuf + regmaxsize * i); } else regcache_raw_supply (regcache, regno, - record_full_core_regbuf + MAX_REGISTER_SIZE * regno); + record_full_core_regbuf + regmaxsize * regno); } /* "to_prepare_to_store" method for prec over corefile. */ @@ -2067,9 +2072,12 @@ record_full_core_store_registers (struct target_ops *ops, struct regcache *regcache, int regno) { + struct gdbarch *gdbarch = get_regcache_arch (regcache); + int regmaxsize = max_register_size (gdbarch); + if (record_full_gdb_operation_disable) regcache_raw_collect (regcache, regno, - record_full_core_regbuf + MAX_REGISTER_SIZE * regno); + record_full_core_regbuf + regmaxsize * regno); else error (_("You can't do that without a process to debug.")); } @@ -2253,25 +2261,6 @@ init_record_full_core_ops (void) } /* Record log save-file format - Version 1 (never released) - - Header: - 4 bytes: magic number htonl(0x20090829). - NOTE: be sure to change whenever this file format changes! - - Records: - record_full_end: - 1 byte: record type (record_full_end, see enum record_full_type). - record_full_reg: - 1 byte: record type (record_full_reg, see enum record_full_type). - 8 bytes: register id (network byte order). - MAX_REGISTER_SIZE bytes: register value. - record_full_mem: - 1 byte: record type (record_full_mem, see enum record_full_type). - 8 bytes: memory length (network byte order). - 8 bytes: memory address (network byte order). - n bytes: memory value (n == memory length). - Version 2 4 bytes: magic number netorder32(0x20091016). NOTE: be sure to change whenever this file format changes! diff --git a/gdb/regcache.h b/gdb/regcache.h index e5a7cf553279b8cc0d546ec1b8274cbf97e246d5..f8d8f2f84887cb9bb2541f68367f5269b75deb56 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -50,6 +50,12 @@ extern struct address_space *get_regcache_aspace (const struct regcache *); enum register_status regcache_register_status (const struct regcache *regcache, int regnum); +/* Make certain that the register cache is up-to-date with respect to the + current thread. */ +void regcache_raw_update (struct regcache *regcache, int regnum); + +enum register_status regcache_raw_read (struct regcache *regcache, + int rawnum, gdb_byte *buf); /* Transfer a raw register [0..NUM_REGS) between core-gdb and the regcache. The read variants return the status of the register. */ @@ -202,6 +208,8 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum); extern int register_size (struct gdbarch *gdbarch, int regnum); +/* Return the size of the largest register. */ +extern long max_register_size (struct gdbarch *gdbarch); /* Save/restore a register cache. The set of registers saved / restored into the DST regcache determined by the save_reggroup / @@ -228,4 +236,8 @@ extern void regcache_cpy (struct regcache *dest, struct regcache *src); extern void registers_changed (void); extern void registers_changed_ptid (ptid_t); +extern void regcache_debug_print_register (const char *func, + struct regcache *regcache, + int regno); + #endif /* REGCACHE_H */ diff --git a/gdb/regcache.c b/gdb/regcache.c index 9d28aa2c2114e0f1c52758bb2fbe9669a329c13e..7d693ac616c3c8e692d369d748937ac4662c849a 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -73,6 +73,9 @@ struct regcache_descr /* Cached table containing the type of each register. */ struct type **register_type; + + /* Size of the largest register. */ + long max_register_size; }; static void * @@ -126,6 +129,8 @@ init_regcache_descr (struct gdbarch *gdbarch) descr->register_offset[i] = offset; offset += descr->sizeof_register[i]; gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); + descr->max_register_size = std::max (descr->max_register_size, + descr->sizeof_register[i]); } /* Set the real size of the raw register cache buffer. */ descr->sizeof_raw_registers = offset; @@ -136,6 +141,8 @@ init_regcache_descr (struct gdbarch *gdbarch) descr->register_offset[i] = offset; offset += descr->sizeof_register[i]; gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]); + descr->max_register_size = std::max (descr->max_register_size, + descr->sizeof_register[i]); } /* Set the real size of the readonly register cache buffer. */ descr->sizeof_cooked_registers = offset; @@ -187,6 +194,13 @@ regcache_register_size (const struct regcache *regcache, int n) return register_size (get_regcache_arch (regcache), n); } +long +max_register_size (struct gdbarch *gdbarch) +{ + struct regcache_descr *descr = regcache_descr (gdbarch); + return descr->max_register_size; +} + /* The register cache for storing raw register values. */ struct regcache @@ -327,7 +341,6 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read, void *src) { struct gdbarch *gdbarch = dst->descr->gdbarch; - gdb_byte buf[MAX_REGISTER_SIZE]; int regnum; /* The DST should be `read-only', if it wasn't then the save would @@ -346,12 +359,10 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read, { if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup)) { + gdb_byte *buf = register_buffer (dst, regnum); enum register_status status = cooked_read (src, regnum, buf); - if (status == REG_VALID) - memcpy (register_buffer (dst, regnum), buf, - register_size (gdbarch, regnum)); - else + if (status != REG_VALID) { gdb_assert (status != REG_UNKNOWN); @@ -369,7 +380,7 @@ regcache_restore (struct regcache *dst, void *cooked_read_context) { struct gdbarch *gdbarch = dst->descr->gdbarch; - gdb_byte buf[MAX_REGISTER_SIZE]; + std::vector buf (max_register_size (gdbarch)); int regnum; /* The dst had better not be read-only. If it is, the `restore' @@ -385,9 +396,9 @@ regcache_restore (struct regcache *dst, { enum register_status status; - status = cooked_read (cooked_read_context, regnum, buf); + status = cooked_read (cooked_read_context, regnum, buf.data ()); if (status == REG_VALID) - regcache_cooked_write (dst, regnum, buf); + regcache_cooked_write (dst, regnum, buf.data ()); } } } @@ -642,15 +653,17 @@ registers_changed (void) alloca (0); } -enum register_status -regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf) +void +regcache_raw_update (struct regcache *regcache, int regnum) { - gdb_assert (regcache != NULL && buf != NULL); - gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers); /* Make certain that the register cache is up-to-date with respect to the current thread. This switching shouldn't be necessary only there is still only one target side register cache. Sigh! On the bright side, at least there is a regcache object. */ + + gdb_assert (regcache != NULL); + gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers); + if (!regcache->readonly_p && regcache_register_status (regcache, regnum) == REG_UNKNOWN) { @@ -666,6 +679,13 @@ regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf) if (regcache->register_status[regnum] == REG_UNKNOWN) regcache->register_status[regnum] = REG_UNAVAILABLE; } +} + +enum register_status +regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf) +{ + gdb_assert (buf != NULL); + regcache_raw_update (regcache, regnum); if (regcache->register_status[regnum] != REG_VALID) memset (buf, 0, regcache->descr->sizeof_register[regnum]); @@ -1250,6 +1270,42 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc) reinit_frame_cache (); } +void +regcache_debug_print_register (const char *func, struct regcache *regcache, + int regno) +{ + struct gdbarch *gdbarch = get_regcache_arch (regcache); + + fprintf_unfiltered (gdb_stdlog, "%s ", func); + if (regno >= 0 && regno < gdbarch_num_regs (gdbarch) + && gdbarch_register_name (gdbarch, regno) != NULL + && gdbarch_register_name (gdbarch, regno)[0] != '\0') + fprintf_unfiltered (gdb_stdlog, "(%s)", + gdbarch_register_name (gdbarch, regno)); + else + fprintf_unfiltered (gdb_stdlog, "(%d)", regno); + if (regno >= 0 && regno < gdbarch_num_regs (gdbarch)) + { + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + int i, size = register_size (gdbarch, regno); + gdb_byte *buf = register_buffer (regcache, regno); + + regcache_raw_collect (regcache, regno, buf); + fprintf_unfiltered (gdb_stdlog, " = "); + for (i = 0; i < size; i++) + { + fprintf_unfiltered (gdb_stdlog, "%02x", buf[i]); + } + if (size <= sizeof (LONGEST)) + { + ULONGEST val = extract_unsigned_integer (buf, size, byte_order); + + fprintf_unfiltered (gdb_stdlog, " %s %s", + core_addr_to_string_nz (val), plongest (val)); + } + } + fprintf_unfiltered (gdb_stdlog, "\n"); +} static void reg_flush_command (char *command, int from_tty) @@ -1279,7 +1335,7 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, int footnote_register_offset = 0; int footnote_register_type_name_null = 0; long register_offset = 0; - gdb_byte buf[MAX_REGISTER_SIZE]; + std::vector buf (max_register_size (gdbarch)); #if 0 fprintf_unfiltered (file, "nr_raw_registers %d\n", @@ -1406,8 +1462,8 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, fprintf_unfiltered (file, ""); else { - regcache_raw_read (regcache, regnum, buf); - print_hex_chars (file, buf, + regcache_raw_update (regcache, regnum); + print_hex_chars (file, register_buffer (regcache, regnum), regcache->descr->sizeof_register[regnum], gdbarch_byte_order (gdbarch)); } @@ -1422,13 +1478,13 @@ regcache_dump (struct regcache *regcache, struct ui_file *file, { enum register_status status; - status = regcache_cooked_read (regcache, regnum, buf); + status = regcache_cooked_read (regcache, regnum, buf.data ()); if (status == REG_UNKNOWN) fprintf_unfiltered (file, ""); else if (status == REG_UNAVAILABLE) fprintf_unfiltered (file, ""); else - print_hex_chars (file, buf, + print_hex_chars (file, buf.data (), regcache->descr->sizeof_register[regnum], gdbarch_byte_order (gdbarch)); } diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c index b0c68c617e365c0ea18ac2eca525598b688ffbb5..0dd901377e53eafc27e4fb39a1453380779ae361 100644 --- a/gdb/remote-sim.c +++ b/gdb/remote-sim.c @@ -439,6 +439,9 @@ gdbsim_fetch_register (struct target_ops *ops, return; } + int regsize = register_size (gdbarch, regno); + std::vector buf (regsize); + switch (gdbarch_register_sim_regno (gdbarch, regno)) { case LEGACY_SIM_REGNO_IGNORE: @@ -447,38 +450,32 @@ gdbsim_fetch_register (struct target_ops *ops, { /* For moment treat a `does not exist' register the same way as an ``unavailable'' register. */ - gdb_byte buf[MAX_REGISTER_SIZE]; int nr_bytes; - memset (buf, 0, MAX_REGISTER_SIZE); - regcache_raw_supply (regcache, regno, buf); + regcache_raw_supply (regcache, regno, buf.data ()); break; } default: { static int warn_user = 1; - gdb_byte buf[MAX_REGISTER_SIZE]; int nr_bytes; gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch)); - memset (buf, 0, MAX_REGISTER_SIZE); nr_bytes = sim_fetch_register (sim_data->gdbsim_desc, gdbarch_register_sim_regno (gdbarch, regno), - buf, - register_size (gdbarch, regno)); + buf.data (), regsize); if (nr_bytes > 0 - && nr_bytes != register_size (gdbarch, regno) && warn_user) + && nr_bytes != regsize && warn_user) { fprintf_unfiltered (gdb_stderr, "Size of register %s (%d/%d) " "incorrect (%d instead of %d))", gdbarch_register_name (gdbarch, regno), regno, - gdbarch_register_sim_regno - (gdbarch, regno), - nr_bytes, register_size (gdbarch, regno)); + gdbarch_register_sim_regno (gdbarch, regno), + nr_bytes, regsize); warn_user = 0; } /* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0' @@ -486,13 +483,13 @@ gdbsim_fetch_register (struct target_ops *ops, which registers are fetchable. */ /* Else if (nr_bytes < 0): an old simulator, that doesn't think to return the register size. Just assume all is ok. */ - regcache_raw_supply (regcache, regno, buf); + regcache_raw_supply (regcache, regno, buf.data ()); if (remote_debug) { fprintf_unfiltered (gdb_stdlog, "gdbsim_fetch_register: %d", regno); /* FIXME: We could print something more intelligible. */ - dump_mem (buf, register_size (gdbarch, regno)); + dump_mem (buf.data (), regsize); } break; } @@ -516,15 +513,16 @@ gdbsim_store_register (struct target_ops *ops, } else if (gdbarch_register_sim_regno (gdbarch, regno) >= 0) { - gdb_byte tmp[MAX_REGISTER_SIZE]; + int regsize = register_size (gdbarch, regno); + std::vector tmp (regsize); int nr_bytes; - regcache_cooked_read (regcache, regno, tmp); + regcache_cooked_read (regcache, regno, tmp.data ()); nr_bytes = sim_store_register (sim_data->gdbsim_desc, gdbarch_register_sim_regno (gdbarch, regno), - tmp, register_size (gdbarch, regno)); - if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno)) + tmp.data (), regsize); + if (nr_bytes > 0 && nr_bytes != regsize) internal_error (__FILE__, __LINE__, _("Register size different to expected")); if (nr_bytes < 0) @@ -538,7 +536,7 @@ gdbsim_store_register (struct target_ops *ops, { fprintf_unfiltered (gdb_stdlog, "gdbsim_store_register: %d", regno); /* FIXME: We could print something more intelligible. */ - dump_mem (tmp, register_size (gdbarch, regno)); + dump_mem (tmp.data (), regsize); } } } diff --git a/gdb/remote.c b/gdb/remote.c index c4cec910c44cf91cc7f36b7f2d87cde3f46de41e..0eeba218ea4846466746817e8e79a5bbe84fba95 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -7759,7 +7759,6 @@ remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) { struct remote_arch_state *rsa = get_remote_arch_state (); int i; - gdb_byte buf[MAX_REGISTER_SIZE]; /* Make sure the entire registers array is valid. */ switch (packet_support (PACKET_P)) @@ -7769,7 +7768,7 @@ remote_prepare_to_store (struct target_ops *self, struct regcache *regcache) /* Make sure all the necessary registers are cached. */ for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++) if (rsa->regs[i].in_g_packet) - regcache_raw_read (regcache, rsa->regs[i].regnum, buf); + regcache_raw_update (regcache, rsa->regs[i].regnum); break; case PACKET_ENABLE: break; diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c index a09a3ab9a8bc56f367e3ba1537f5674f0a7f491f..06b146c314a10be0e360a7242bab229ced1c00b1 100644 --- a/gdb/sol-thread.c +++ b/gdb/sol-thread.c @@ -534,12 +534,6 @@ sol_thread_store_registers (struct target_ops *ops, if (regnum != -1) { - /* Not writing all the registers. */ - char old_value[MAX_REGISTER_SIZE]; - - /* Save new register value. */ - regcache_raw_collect (regcache, regnum, old_value); - val = p_td_thr_getgregs (&thandle, gregset); if (val != TD_OK) error (_("sol_thread_store_registers: td_thr_getgregs %s"), @@ -548,9 +542,6 @@ sol_thread_store_registers (struct target_ops *ops, if (val != TD_OK) error (_("sol_thread_store_registers: td_thr_getfpregs %s"), td_err_string (val)); - - /* Restore new register value. */ - regcache_raw_supply (regcache, regnum, old_value); } fill_gregset (regcache, (gdb_gregset_t *) &gregset, regnum); diff --git a/gdb/stack.c b/gdb/stack.c index e00e2972cf20bc63917af19f86bf57f1c6b0b5b0..7ba7d68bde8d83ea1e700faa466c6951979e0f76 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1650,33 +1650,35 @@ frame_info (char *addr_exp, int from_tty) int count; int i; int need_nl = 1; + int sp_regnum = gdbarch_sp_regnum (gdbarch); /* The sp is special; what's displayed isn't the save address, but the value of the previous frame's sp. This is a legacy thing, at one stage the frame cached the previous frame's SP instead of its address, hence it was easiest to just display the cached value. */ - if (gdbarch_sp_regnum (gdbarch) >= 0) + if (sp_regnum >= 0) { /* Find out the location of the saved stack pointer with out actually evaluating it. */ - frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch), - &optimized, &unavailable, &lval, &addr, - &realnum, NULL); + frame_register_unwind (fi, sp_regnum, &optimized, &unavailable, &lval, + &addr, &realnum, NULL); if (!optimized && !unavailable && lval == not_lval) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - int sp_size = register_size (gdbarch, gdbarch_sp_regnum (gdbarch)); - gdb_byte value[MAX_REGISTER_SIZE]; + int sp_size = register_size (gdbarch, sp_regnum); CORE_ADDR sp; + struct value *value = frame_unwind_register_value (fi, sp_regnum); - frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch), - &optimized, &unavailable, &lval, &addr, - &realnum, value); + gdb_assert (value != NULL); /* NOTE: cagney/2003-05-22: This is assuming that the stack pointer was packed as an unsigned integer. That may or may not be valid. */ - sp = extract_unsigned_integer (value, sp_size, byte_order); + sp = extract_unsigned_integer (value_contents_all (value), sp_size, + byte_order); + release_value (value); + value_free (value); + printf_filtered (" Previous frame's sp is "); fputs_filtered (paddress (gdbarch, sp), gdb_stdout); printf_filtered ("\n"); @@ -1702,7 +1704,7 @@ frame_info (char *addr_exp, int from_tty) numregs = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch); for (i = 0; i < numregs; i++) - if (i != gdbarch_sp_regnum (gdbarch) + if (i != sp_regnum && gdbarch_register_reggroup_p (gdbarch, i, all_reggroup)) { /* Find out the location of the saved register without diff --git a/gdb/target.c b/gdb/target.c index 3c409f0f619141205dfdcbbf8e46a277585ed683..ae1321c57983fe3446148090e1d0a4aa823a3b19 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3547,49 +3547,12 @@ target_options_to_string (int target_options) return ret; } -static void -debug_print_register (const char * func, - struct regcache *regcache, int regno) -{ - struct gdbarch *gdbarch = get_regcache_arch (regcache); - - fprintf_unfiltered (gdb_stdlog, "%s ", func); - if (regno >= 0 && regno < gdbarch_num_regs (gdbarch) - && gdbarch_register_name (gdbarch, regno) != NULL - && gdbarch_register_name (gdbarch, regno)[0] != '\0') - fprintf_unfiltered (gdb_stdlog, "(%s)", - gdbarch_register_name (gdbarch, regno)); - else - fprintf_unfiltered (gdb_stdlog, "(%d)", regno); - if (regno >= 0 && regno < gdbarch_num_regs (gdbarch)) - { - enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - int i, size = register_size (gdbarch, regno); - gdb_byte buf[MAX_REGISTER_SIZE]; - - regcache_raw_collect (regcache, regno, buf); - fprintf_unfiltered (gdb_stdlog, " = "); - for (i = 0; i < size; i++) - { - fprintf_unfiltered (gdb_stdlog, "%02x", buf[i]); - } - if (size <= sizeof (LONGEST)) - { - ULONGEST val = extract_unsigned_integer (buf, size, byte_order); - - fprintf_unfiltered (gdb_stdlog, " %s %s", - core_addr_to_string_nz (val), plongest (val)); - } - } - fprintf_unfiltered (gdb_stdlog, "\n"); -} - void target_fetch_registers (struct regcache *regcache, int regno) { current_target.to_fetch_registers (¤t_target, regcache, regno); if (targetdebug) - debug_print_register ("target_fetch_registers", regcache, regno); + regcache_debug_print_register ("target_fetch_registers", regcache, regno); } void @@ -3601,7 +3564,8 @@ target_store_registers (struct regcache *regcache, int regno) current_target.to_store_registers (¤t_target, regcache, regno); if (targetdebug) { - debug_print_register ("target_store_registers", regcache, regno); + regcache_debug_print_register ("target_store_registers", regcache, + regno); } }