From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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:00 -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 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-03/txt/msg00003.txt.bz2 Alan Hayward writes: > @@ -1252,7 +1252,11 @@ frame_unwind_register_signed (struct frame_info *f= rame, int regnum) > struct gdbarch *gdbarch =3D frame_unwind_arch (frame); > enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > int size =3D 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 *fram= e, 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 =3D frame_unwind_register_value (frame->next, > + regnum); > + gdb_assert (value !=3D 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); --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom 2a76b39ffa87ed015f3637ee4a9d083f682863a0 Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Wed, 1 Mar 2017 10:43:29 +0000 Subject: [PATCH] Use content provider in extract_unsigned_integer --- gdb/ada-lang.c | 9 ++++++--- gdb/defs.h | 4 ++++ gdb/findvar.c | 38 ++++++++++++++++++++++++++++++-------- gdb/frame.c | 7 ++++--- gdb/ia64-tdep.c | 19 +++++++++++++++---- 5 files changed, 59 insertions(+), 18 deletions(-) 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 *ty= pe) { struct gdbarch *gdbarch =3D get_type_arch (type); unsigned len =3D TYPE_LENGTH (type); - gdb_byte *buf =3D (gdb_byte *) alloca (len); CORE_ADDR addr; =20 addr =3D value_address (value); - gdbarch_address_to_pointer (gdbarch, type, buf, addr); - addr =3D extract_unsigned_integer (buf, len, gdbarch_byte_order (gdbarch= )); + addr =3D extract_unsigned_integer ([&] (gdb_byte *buf, size_t size) + { + gdbarch_address_to_pointer (gdbarch, + type, buf, + addr); + }, len, gdbarch_byte_order (gdbarch)); return addr; } =20 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); =20 +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 *); =20 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 tha= n %d bytes."), return retval; } =20 -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 =3D addr; const unsigned char *endaddr =3D startaddr + len; =20 - 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 =3D 0; @@ -111,6 +106,33 @@ That operation is not available on integers of more th= an %d bytes."), return retval; } =20 +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 =3D frame_unwind_arch (frame); enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); int size =3D register_size (gdbarch, regnum); - gdb_byte buf[MAX_REGISTER_SIZE]; =20 - 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); } =20 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 =3D=3D 0 && rN =3D=3D 2=20 && ((rM =3D=3D fp_reg && fp_reg !=3D 0) || rM =3D=3D 12)) { - gdb_byte buf[MAX_REGISTER_SIZE]; CORE_ADDR saved_sp =3D 0; /* adds r2, spilloffset, rFramePointer=20 or @@ -1534,8 +1533,15 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, { struct gdbarch *gdbarch =3D get_frame_arch (this_frame); enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); - get_frame_register (this_frame, sp_regnum, buf); - saved_sp =3D extract_unsigned_integer (buf, 8, byte_order); + + saved_sp + =3D extract_unsigned_integer ([&] (gdb_byte * buf, + size_t size) + { + get_frame_register (this_frame, + sp_regnum, + buf); + }, 8, byte_order); } spill_addr =3D saved_sp + (rM =3D=3D 12 ? 0 : mem_stack_frame_size)=20 @@ -1782,7 +1788,12 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, else if (cfm_reg !=3D 0) { get_frame_register (this_frame, cfm_reg, buf); - cfm =3D extract_unsigned_integer (buf, 8, byte_order); + cfm =3D extract_unsigned_integer ([&] (gdb_byte * buf, size_t size) + { + get_frame_register (this_frame, + cfm_reg, + buf); + }, 8, byte_order); } cache->prev_cfm =3D cfm; =20=20=20=20=20=20=20 --=20 1.9.1