From patchwork Wed Feb 8 14:44:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 19177 Received: (qmail 126776 invoked by alias); 8 Feb 2017 14:44:55 -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 126690 invoked by uid 89); 8 Feb 2017 14:44:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=qiyaoltc@gmail.com, qiyaoltcgmailcom, U*qiyaoltc, sk:qiyaolt X-HELO: EUR01-DB5-obe.outbound.protection.outlook.com Received: from mail-db5eur01on0048.outbound.protection.outlook.com (HELO EUR01-DB5-obe.outbound.protection.outlook.com) (104.47.2.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 08 Feb 2017 14:44:42 +0000 Received: from VI1PR0801MB1822.eurprd08.prod.outlook.com (10.168.68.7) by VI1PR0801MB1821.eurprd08.prod.outlook.com (10.168.67.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.888.16; Wed, 8 Feb 2017 14:44:38 +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; Wed, 8 Feb 2017 14:44:38 +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: Wed, 8 Feb 2017 14:44:38 +0000 Message-ID: <9DD2B6B8-9FC3-4339-996F-F58B387190EC@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> <86inokq39w.fsf@gmail.com> In-Reply-To: <86inokq39w.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: 982e73d0-88c2-47e6-5b27-08d450310250 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081); SRVR:VI1PR0801MB1821; x-microsoft-exchange-diagnostics: 1; VI1PR0801MB1821; 7:ZpyMqE7xbO+dnzrMxbkC4HvmeGtx3NEAqP/uAAypO8Xu+mioXQ5e/5EM0GjqJTykKxRirzI15EYyS5tYmQD17yrk0F0sHE2lgoUZCM9jQFh+Bd5HX4tvPOrwGSaIPZnOS9JYBm1bw9l4XSWfWOTWJLxLAS5zhpUqV/ZSeU8/hFrY30xbpPpysEQUOTzl5BlftS3LBsNsPH/W9lPAmjaNsZ/Bt48r7I2O3pBX4lqR+YWM5WhbemI1ac8/pgvH2wDGEXpUPjbgh981K7qWhcfg2gc6Vj9GLe/tBg630FbNM2s8jk90l8FURhjQw9lwtZNvhnMbNPZBedwAqdRB0SshyoUBCo9FgSSffTIw6TMekWPDl5Qg+UcvNix6x9R44fI/TKXvHgpP7cx9xQFX504r9AZln6AnE/ZlroR1qeGkjfmFMeusa5oAnCZpjJOxeLHtazwIEuyM+7TAxMpLlJNhNFWd1MASCY1CRixAwaKDpOgK2cdNuJFGUHc7FW/vs2yWLapR+seg8S0tF+oOWinnUQ== 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)(5005006)(8121501046)(2017020702029)(20170203043)(10201501046)(3002001)(6055026)(6041248)(20161123555025)(20161123562025)(20161123564025)(20161123558025)(20161123560025)(6072148); SRVR:VI1PR0801MB1821; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0801MB1821; x-forefront-prvs: 0212BDE3BE x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(39850400002)(39860400002)(39450400003)(39840400002)(39410400002)(189002)(24454002)(199003)(377424004)(8936002)(99286003)(54906002)(110136004)(81166006)(81156014)(6486002)(8676002)(6512007)(25786008)(6506006)(6436002)(39060400001)(77096006)(33656002)(68736007)(3660700001)(3280700002)(83716003)(38730400002)(305945005)(229853002)(82746002)(575784001)(86362001)(2906002)(4326007)(7736002)(106356001)(6116002)(101416001)(3846002)(102836003)(93886004)(54356999)(53546003)(6246003)(189998001)(97736004)(76176999)(50986999)(122556002)(92566002)(2900100001)(6916009)(1411001)(2950100002)(53936002)(106116001)(66066001)(5660300001)(36756003)(105586002)(104396002); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0801MB1821; 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: <98BE0C752A1DB0458529216B11B3E159@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2017 14:44:38.4759 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0801MB1821 > On 8 Feb 2017, at 12:24, Yao Qi wrote: > > Alan Hayward writes: > >> 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); > > Why don't you hoist frame_unwind_register_value above?, so the > frame_register_unwind call is no longer needed, > > > struct value *value = frame_unwind_register_value (fi, sp_regnum); > > gdb_assert (value != NULL); > > if (!value_optimized_out (value) && value_entirely_available (value)) > { > if (VALUE_LVAL (value) == not_lval) > { > sp = extract_unsigned_integer (value_contents_all (value), > sp_size, byte_order); > } > else if (VALUE_LVAL (value) == lval_memory) > { > // use value_address (value); > } > else if (VALUE_LVAL (value) == lval_register) > { > // use VALUE_REGNUM (value); > } > } > /* else keep quiet. */ > > release_value (value); > value_free (value); > >> /* 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 > > -- > Yao (齐尧) Ok, changed as requested. Alan. 2017-02-08 Alan Hayward * stack.c (frame_info): Use frame_unwind_register_value to avoid buf. diff --git a/gdb/stack.c b/gdb/stack.c index e00e2972cf20bc63917af19f86bf57f1c6b0b5b0..97b600b6b7bb6b450e54c947dc6178be03f31e6b 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -1642,57 +1642,52 @@ frame_info (char *addr_exp, int from_tty) /* Print as much information as possible on the location of all the registers. */ { - enum lval_type lval; - int optimized; - int unavailable; - CORE_ADDR addr; - int realnum; 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); - 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]; - CORE_ADDR sp; - - frame_register_unwind (fi, gdbarch_sp_regnum (gdbarch), - &optimized, &unavailable, &lval, &addr, - &realnum, value); - /* 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); - printf_filtered (" Previous frame's sp is "); - fputs_filtered (paddress (gdbarch, sp), gdb_stdout); - printf_filtered ("\n"); - need_nl = 0; - } - else if (!optimized && !unavailable && lval == lval_memory) - { - printf_filtered (" Previous frame's sp at "); - fputs_filtered (paddress (gdbarch, addr), gdb_stdout); - printf_filtered ("\n"); - need_nl = 0; - } - else if (!optimized && !unavailable && lval == lval_register) + struct value *value = frame_unwind_register_value (fi, sp_regnum); + gdb_assert (value != NULL); + + if (!value_optimized_out (value) && value_entirely_available (value)) { - printf_filtered (" Previous frame's sp in %s\n", - gdbarch_register_name (gdbarch, realnum)); + if (VALUE_LVAL (value) == not_lval) + { + CORE_ADDR sp; + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); + int sp_size = register_size (gdbarch, sp_regnum); + + sp = extract_unsigned_integer (value_contents_all (value), + sp_size, byte_order); + + printf_filtered (" Previous frame's sp is "); + fputs_filtered (paddress (gdbarch, sp), gdb_stdout); + printf_filtered ("\n"); + } + else if (VALUE_LVAL (value) == lval_memory) + { + printf_filtered (" Previous frame's sp at "); + fputs_filtered (paddress (gdbarch, value_address (value)), + gdb_stdout); + printf_filtered ("\n"); + } + else if (VALUE_LVAL (value) == lval_register) + { + printf_filtered (" Previous frame's sp in %s\n", + gdbarch_register_name (gdbarch, + VALUE_REGNUM (value))); + } + + release_value (value); + value_free (value); need_nl = 0; } /* else keep quiet. */ @@ -1702,9 +1697,15 @@ 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)) { + enum lval_type lval; + int optimized; + int unavailable; + CORE_ADDR addr; + int realnum; + /* Find out the location of the saved register without fetching the corresponding value. */ frame_register_unwind (fi, i, &optimized, &unavailable,