From patchwork Wed Mar 1 12:32:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 19414 Received: (qmail 83209 invoked by alias); 1 Mar 2017 12:32:43 -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 83200 invoked by uid 89); 1 Mar 2017 12:32:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=reaction, cfm, 8115, she X-HELO: mail-wr0-f194.google.com Received: from mail-wr0-f194.google.com (HELO mail-wr0-f194.google.com) (209.85.128.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 01 Mar 2017 12:32:41 +0000 Received: by mail-wr0-f194.google.com with SMTP id l37so5324206wrc.3 for ; Wed, 01 Mar 2017 04:32:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=FadGSY+hY4DS3ht6QdCe1bBlFdE52lUkDyTcfbklHRU=; b=gGd7HCMzoubX9/SB90St84xhHoJH/KukGGz4VbPdd7MqhZJPjkhb24be8PCN3EHTaq gA+sFY7GzthYUXhit5swgGsQgvK5oOv+kwJms1yy8hG5rD+4LVD2GQNOuN0BQxlwd6MJ mNDVR7soT2++gu3RDyhOkPWOVS0TwED+V4Ukn2fJJW6jnf2TMcPEuxTAJaZ4/hBHrjxG rXb2xz/s5Ufvs7hAcj6g44jnJtfrRqhaGPlNCnHwWGjM4u3fEPtAjsXt3n273PtT/XX6 v2hmknt74gYFaT0EJpi3LOu89C+JYLIHED8Xfq8g0e7cxrUVhgoPh3pHqaTJsuVHj4Aw XqFw== X-Gm-Message-State: AMke39mmxqFAUJfDAHphnQQH0LccyCJEMkyKLIVWzGPUP9wCr9i3eG/ct/8dQliRWBxSew== X-Received: by 10.223.128.5 with SMTP id 5mr6822829wrk.163.1488371559288; Wed, 01 Mar 2017 04:32:39 -0800 (PST) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id d6sm22673378wmd.6.2017.03.01.04.32.37 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 01 Mar 2017 04:32:38 -0800 (PST) From: Yao Qi To: Alan Hayward Cc: "gdb-patches\@sourceware.org" , nd Subject: Re: [PATCH] Remove MAX_REGISTER_SIZE from frame.c References: Date: Wed, 01 Mar 2017 12:32:27 +0000 In-Reply-To: (Alan Hayward's message of "Fri, 24 Feb 2017 10:01:15 +0000") Message-ID: <86lgspqisk.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Alan Hayward writes: > @@ -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)); > We apply the restriction of extract_signed_integer to its caller here. People will wonder why do we have such check until he/she digs into extract_signed_integer. My first reaction is to add some comments to explain why do we do so, but the recent gdb::function_view reminds me that we can do something differently (and better, IMO). Current pattern of using extract_unsigned_integer is 1) allocate an array on stack, 2) read data from regcache or frame into the array, 3) pass the array to extract_unsigned_integer we can pass a callable function_view as a content provider to extract_unsigned_integer, so that we don't need step 1). The code becomes, return extract_unsigned_integer ([&] (gdb_byte *buf, size_t size) { frame_unwind_register (frame, regnum, buf); }, size, byte_order); We can remove some uses of MAX_REGISTER_SIZE in this way. Do you (Alan and others) like the patch below? > @@ -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); Use value_contents_writeable, > + put_frame_register (frame, regnum, value_contents_all (value)); s/value_contents_all/value_contents_raw/ because value_contents_all requires value is available but deprecated_frame_register_read doesn't. > + release_value (value); > + value_free (value); diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 753409c..07ce04a 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -4555,12 +4555,15 @@ value_pointer (struct value *value, struct type *type) { struct gdbarch *gdbarch = get_type_arch (type); unsigned len = TYPE_LENGTH (type); - gdb_byte *buf = (gdb_byte *) alloca (len); CORE_ADDR addr; addr = value_address (value); - gdbarch_address_to_pointer (gdbarch, type, buf, addr); - addr = extract_unsigned_integer (buf, len, gdbarch_byte_order (gdbarch)); + addr = extract_unsigned_integer ([&] (gdb_byte *buf, size_t size) + { + gdbarch_address_to_pointer (gdbarch, + type, buf, + addr); + }, len, gdbarch_byte_order (gdbarch)); return addr; } diff --git a/gdb/defs.h b/gdb/defs.h index aa58605..a2f8fce 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -628,6 +628,10 @@ extern LONGEST extract_signed_integer (const gdb_byte *, int, extern ULONGEST extract_unsigned_integer (const gdb_byte *, int, enum bfd_endian); +extern ULONGEST + extract_unsigned_integer (gdb::function_view content_provider, + int len, enum bfd_endian byte_order); + extern int extract_long_unsigned_integer (const gdb_byte *, int, enum bfd_endian, LONGEST *); diff --git a/gdb/findvar.c b/gdb/findvar.c index 80c709a..6d7d0de 100644 --- a/gdb/findvar.c +++ b/gdb/findvar.c @@ -81,20 +81,15 @@ That operation is not available on integers of more than %d bytes."), return retval; } -ULONGEST -extract_unsigned_integer (const gdb_byte *addr, int len, - enum bfd_endian byte_order) +static ULONGEST +extract_unsigned_integer_1 (const gdb_byte *addr, int len, + enum bfd_endian byte_order) { ULONGEST retval; const unsigned char *p; const unsigned char *startaddr = addr; const unsigned char *endaddr = startaddr + len; - if (len > (int) sizeof (ULONGEST)) - error (_("\ -That operation is not available on integers of more than %d bytes."), - (int) sizeof (ULONGEST)); - /* Start at the most significant end of the integer, and work towards the least significant. */ retval = 0; @@ -111,6 +106,33 @@ That operation is not available on integers of more than %d bytes."), return retval; } +ULONGEST +extract_unsigned_integer (const gdb_byte *addr, int len, + enum bfd_endian byte_order) +{ + if (len > (int) sizeof (ULONGEST)) + error (_("\ +That operation is not available on integers of more than %d bytes."), + (int) sizeof (ULONGEST)); + + return extract_unsigned_integer_1 (addr, len, byte_order); +} + +ULONGEST +extract_unsigned_integer (gdb::function_view content_provider, + int len, enum bfd_endian byte_order) +{ + if (len > (int) sizeof (ULONGEST)) + error (_("\ +That operation is not available on integers of more than %d bytes."), + (int) sizeof (ULONGEST)); + + gdb_byte buf[sizeof (ULONGEST)]; + + content_provider (buf, len); + return extract_unsigned_integer_1 (buf, len, byte_order); +} + /* Sometimes a long long unsigned integer can be extracted as a LONGEST value. This is done so that we can print these values better. If this integer can be converted to a LONGEST, this diff --git a/gdb/frame.c b/gdb/frame.c index d98003d..57f82f1 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1270,10 +1270,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]; - frame_unwind_register (frame, regnum, buf); - return extract_unsigned_integer (buf, size, byte_order); + return extract_unsigned_integer ([&] (gdb_byte *buf, size_t size) + { + frame_unwind_register (frame, regnum, buf); + }, size, byte_order); } ULONGEST diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c index 4c53bc6..2748dac 100644 --- a/gdb/ia64-tdep.c +++ b/gdb/ia64-tdep.c @@ -1516,7 +1516,6 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, else if (qp == 0 && rN == 2 && ((rM == fp_reg && fp_reg != 0) || rM == 12)) { - gdb_byte buf[MAX_REGISTER_SIZE]; CORE_ADDR saved_sp = 0; /* adds r2, spilloffset, rFramePointer or @@ -1534,8 +1533,15 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, { struct gdbarch *gdbarch = get_frame_arch (this_frame); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - get_frame_register (this_frame, sp_regnum, buf); - saved_sp = extract_unsigned_integer (buf, 8, byte_order); + + saved_sp + = extract_unsigned_integer ([&] (gdb_byte * buf, + size_t size) + { + get_frame_register (this_frame, + sp_regnum, + buf); + }, 8, byte_order); } spill_addr = saved_sp + (rM == 12 ? 0 : mem_stack_frame_size) @@ -1782,7 +1788,12 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, else if (cfm_reg != 0) { get_frame_register (this_frame, cfm_reg, buf); - cfm = extract_unsigned_integer (buf, 8, byte_order); + cfm = extract_unsigned_integer ([&] (gdb_byte * buf, size_t size) + { + get_frame_register (this_frame, + cfm_reg, + buf); + }, 8, byte_order); } cache->prev_cfm = cfm;