From patchwork Tue Feb 14 11:24:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 19254 Received: (qmail 27142 invoked by alias); 14 Feb 2017 11:24:29 -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 27086 invoked by uid 89); 14 Feb 2017 11:24:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, KAM_LOTSOFHASH, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=no version=3.3.2 spammy=*cmd, ptid_t, field_int, sk:value_c X-HELO: EUR03-DB5-obe.outbound.protection.outlook.com Received: from mail-eopbgr40070.outbound.protection.outlook.com (HELO EUR03-DB5-obe.outbound.protection.outlook.com) (40.107.4.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Feb 2017 11:24:15 +0000 Received: from VI1PR0801MB1822.eurprd08.prod.outlook.com (10.168.68.7) by VI1PR0801MB1822.eurprd08.prod.outlook.com (10.168.68.7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.888.16; Tue, 14 Feb 2017 11:24:03 +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.030; Tue, 14 Feb 2017 11:24:03 +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, 14 Feb 2017 11:24:02 +0000 Message-ID: <43DE0D94-591B-48F9-9F98-4902A4C91F44@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> <5F3D30AE-9A53-493A-B6DC-DF594C2FAB18@arm.com> <8660kkpq1l.fsf@gmail.com> In-Reply-To: <8660kkpq1l.fsf@gmail.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-exchange-messagesentrepresentingtype: 1 x-ms-office365-filtering-correlation-id: 73be4fa0-e414-42cb-0ef2-08d454cbfb2e x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081); SRVR:VI1PR0801MB1822; x-microsoft-exchange-diagnostics: 1; VI1PR0801MB1822; 7:gwOXlDM9SUGkww9wUvdgDELTLExpMx8qR8eR69hnkZEHJg87V7qs8wEX4SYzvmIv1ugEoGUBgLoXXHE+Fb3O2ZNm/zCUV/rZuDOjQ1TZRihQWZ8DwG1JIdeyMO/owZhAJWTPsDssNcJF2jFBcIVp/NXy996hgf4Jxlp5WyYjerjyyK4jCy4wpCgRyJavwFYmHVbWGm662ijKoYbbmG084XuWnhcTYi9kHZijYIfrehlK8TaMdrlPiVIvjjhpnp5PWMqdU+3OKeGblDcAPTMqInsc+IVfFMctb0PoLVDLNbi9N6br8rCwLkNpOtVX0vTycSywq0+FEBDK40iFA7zCNWnr1r0sSiUCuh6FsV7U46HQtQPqQmA7XOM8kVadoCgMbCQOMMGRbDsrnjcF9Y30eBA5HfY0FWfaazZP2Jb+bblv7C2RcDHPYO5tg8h3IBnRxYUsMAN7V8qgzeCFyUa2+Ofl7fH3zyohrKvLSaThX/YEiInQLHBudbuhhETwVrRczpwC5DeY3u/Xaspa6qQGkA== nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6041248)(20161123564025)(20161123562025)(20161123558025)(20161123555025)(20161123560025)(6072148); SRVR:VI1PR0801MB1822; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0801MB1822; x-forefront-prvs: 0218A015FA x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(39850400002)(39410400002)(39450400003)(39860400002)(39840400002)(189002)(377424004)(199003)(24454002)(81003)(39060400002)(66066001)(3280700002)(189998001)(6506006)(6436002)(92566002)(25786008)(110136004)(38730400002)(6116002)(2906002)(106356001)(3846002)(102836003)(86362001)(575784001)(6486002)(97736004)(4326007)(36756003)(77096006)(53936002)(105586002)(2900100001)(68736007)(229853002)(106116001)(5660300001)(82746002)(81156014)(7736002)(3660700001)(50986999)(6916009)(122556002)(33656002)(83716003)(101416001)(53546003)(76176999)(6246003)(99286003)(54906002)(6512007)(1411001)(8676002)(305945005)(2950100002)(81166006)(8936002)(93886004)(54356999)(104396002); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0801MB1822; 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: MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Feb 2017 11:24:02.8320 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0801MB1822 > On 8 Feb 2017, at 17:09, Yao Qi wrote: > > Alan Hayward writes: > >> @@ -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; >> > > This function should be moved to regcache.c, because it is about > comparing bytes of a certain register in both regcaches. Then, wen can > compare raw registers from register_buffer, and pseudo registers from > the values. > >> @@ -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; > > -- > Yao (齐尧) Ignore my previous reply to this comment. I've since noticed you can just use regcache_cooked_read_value. New patch below. I removed the error case from mi-main becuase it was impossible for the code to get here - register_changed_p would always return 0 or 1. Tested on a build of all targets using make check with target boards unix and native-gdbserver. This testing also included the two patches: * M68K_MAX_REGISTER_SIZE and I386_MAX_REGISTER_SIZE changes * stack.c changes Ok? Alan. 2017-02-13 Alan Hayward * gdb/mi/mi-main.c (mi_cmd_data_list_changed_registers): Use regcache_register_changed_p (register_changed_p): Remove. * gdb/regcache.c (regcache_register_changed_p): New function. * gdb/regcache.h (regcache_register_changed_p): New declaration. diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 57c23ebf5d6b2d3b398aa40ebd9b3cb70c56125c..d22cb9b59d3c5d65609bf3762457bba90401fd8f 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -91,8 +91,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p, const char *args); static void mi_execute_async_cli_command (char *cli_command, char **argv, int argc); -static int register_changed_p (int regnum, struct regcache *, - struct regcache *); static void output_register (struct frame_info *, int regnum, int format, int skip_unavailable); @@ -1098,11 +1096,7 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc) if (gdbarch_register_name (gdbarch, regnum) == NULL || *(gdbarch_register_name (gdbarch, regnum)) == '\0') continue; - changed = register_changed_p (regnum, prev_regs, this_regs); - if (changed < 0) - error (_("-data-list-changed-registers: " - "Unable to read register contents.")); - else if (changed) + if (regcache_register_changed_p (regnum, prev_regs, this_regs)) uiout->field_int (NULL, regnum); } } @@ -1117,11 +1111,7 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc) && gdbarch_register_name (gdbarch, regnum) != NULL && *gdbarch_register_name (gdbarch, regnum) != '\000') { - changed = register_changed_p (regnum, prev_regs, this_regs); - if (changed < 0) - error (_("-data-list-changed-registers: " - "Unable to read register contents.")); - else if (changed) + if (regcache_register_changed_p (regnum, prev_regs, this_regs)) uiout->field_int (NULL, regnum); } else @@ -1130,34 +1120,6 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc) do_cleanups (cleanup); } -static int -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]; - enum register_status prev_status; - enum register_status this_status; - - /* First time through or after gdbarch change consider all registers - as changed. */ - if (!prev_regs || get_regcache_arch (prev_regs) != gdbarch) - 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); - - if (this_status != prev_status) - return 1; - else if (this_status == REG_VALID) - return memcmp (prev_buffer, this_buffer, - register_size (gdbarch, regnum)) != 0; - else - return 0; -} - /* Return a list of register number and value pairs. The valid arguments expected are: a letter indicating the format in which to display the registers contents. This can be one of: x diff --git a/gdb/regcache.h b/gdb/regcache.h index e5a7cf553279b8cc0d546ec1b8274cbf97e246d5..174aa0166f419b40b0509b1be5c6dce3c2373fd0 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -228,4 +228,11 @@ extern void regcache_cpy (struct regcache *dest, struct regcache *src); extern void registers_changed (void); extern void registers_changed_ptid (ptid_t); +/* Return true if the register value of regnum differs between prev_regs and + this_regs. prev_regs can be null, but this_regs must be set. */ + +extern bool regcache_register_changed_p (int regnum, + struct regcache *prev_regs, + struct regcache *this_regs); + #endif /* REGCACHE_H */ diff --git a/gdb/regcache.c b/gdb/regcache.c index 9d28aa2c2114e0f1c52758bb2fbe9669a329c13e..96cc94ee7cfeb60d3b6541ca0b49871e6087aa8a 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -1251,6 +1251,74 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc) } +/* Return true if the register value of regnum differs between prev_regs and + this_regs. prev_regs can be null, but this_regs must be set. */ + +bool +regcache_register_changed_p (int regnum, struct regcache *prev_regs, + struct regcache *this_regs) +{ + struct gdbarch *gdbarch; + gdb_assert (regnum >= 0); + gdb_assert (this_regs); + + /* if there are no previous registers consider all registers as changed. */ + if (!prev_regs) + return true; + + gdb_assert (regnum < prev_regs->descr->nr_cooked_registers); + gdb_assert (regnum < this_regs->descr->nr_cooked_registers); + gdbarch = get_regcache_arch (this_regs); + + /* If arches don't match then consider all registers as changed. */ + if (gdbarch != get_regcache_arch (prev_regs)) + return true; + + if (regnum < this_regs->descr->nr_raw_registers) + { + enum register_status prev_status, this_status; + + prev_status = (enum register_status) prev_regs->register_status[regnum]; + this_status = (enum register_status) this_regs->register_status[regnum]; + + if (this_status != prev_status) + return true; + else if (this_status == REG_VALID) + return memcmp (register_buffer (prev_regs, regnum), + register_buffer (this_regs, regnum), + register_size (gdbarch, regnum)) != 0; + else + return false; + } + else + { + struct value *prev_value, *this_value; + bool ret; + + prev_value = regcache_cooked_read_value (prev_regs, regnum); + this_value = regcache_cooked_read_value (this_regs, regnum); + + if (value_optimized_out (prev_value) + || value_optimized_out (this_value)) + ret = value_optimized_out (prev_value) + != value_optimized_out (this_value); + else if (!value_entirely_available (prev_value) + || !value_entirely_available (this_value)) + ret = value_entirely_available (prev_value) + != value_entirely_available (this_value); + else + ret = memcmp (value_contents_raw (prev_value), + value_contents_raw (this_value), + register_size (gdbarch, regnum)) != 0; + + release_value (prev_value); + value_free (prev_value); + release_value (this_value); + value_free (this_value); + return ret; + } +} + static void reg_flush_command (char *command, int from_tty) {