From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5356 invoked by alias); 11 Sep 2015 17:32:16 -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 5346 invoked by uid 89); 11 Sep 2015 17:32:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 11 Sep 2015 17:32:12 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id 34.A2.32596.F81B2F55; Fri, 11 Sep 2015 12:48:47 +0200 (CEST) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.80) with Microsoft SMTP Server id 14.3.248.2; Fri, 11 Sep 2015 13:32:09 -0400 Subject: Re: [PATCH 2/7] Move some integer operations to common. To: Gary Benson References: <1441973603-15247-1-git-send-email-antoine.tremblay@ericsson.com> <1441973603-15247-3-git-send-email-antoine.tremblay@ericsson.com> <20150911142442.GA23515@blade.nx> <55F30C55.3080507@ericsson.com> CC: From: Antoine Tremblay Message-ID: <55F31019.1080607@ericsson.com> Date: Fri, 11 Sep 2015 17:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <55F30C55.3080507@ericsson.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00242.txt.bz2 On 09/11/2015 01:16 PM, Antoine Tremblay wrote: > > > On 09/11/2015 10:24 AM, Gary Benson wrote: >> Hi Antoine, >> >> Please don't introduce "#ifdef GDBSERVER" conditionals into common >> code, I spent some time removing them. I know I didn't get them >> all, but the remaining two are on my hit list. >> > > Humm what is the issue that makes this a bad idea if I may ? > > >> To work around this... do you really need to deal with endianness in >> gdbserver? It's running on the target so if you're pulling numbers >> from target memory in target endianness then could you use or >> generalize target_read_uint32 for your needs? >> > > Yes unfortunately I do need the endianness since the program can be in a > different endianness then GDBServer the code section can even be > different than the data section... > > I could however : > > - Remove int-utils.h from common-defs.h and move to defs.h for GDB > - Move the int-utils.h in server.h for GDBServer which is kinda the > GDBServer's equivalent of defs.h with the BFD defines before the include... > > Does that sound ok ? > Actually this doesn't work since there's an int-utils.c that needs to include again either bfh.h or define the enum with the same #ifdef GSBSERVER ... it would just be moved to the .c ... I would need to move the code to a .h only... At this point I need to understand more the argument about the #ifndef GDBSERVER...? Or any other suggestions are welcome... >> Antoine Tremblay wrote: >>> This patch is in preparation for sharing code between GDB and >>> GDBServer to >>> enable software single stepping on ARM aarch32-linux. >>> >>> It moves multiple functions related to extracting or storing a value >>> based on >>> its endianness from findvar.c in gdb to a new file called int-utils.c in >>> common. >>> >>> Definitions of these functions are also moved to defs.h to >>> common-defs.h. >>> >>> gdb/ChangeLog: >>> * Makefile.in: Add int-utils.o. >>> * common/common-defs.h: New functions defs from defs.h. >>> * common/int-utils.c: New file. >>> * common/int-utils.h: New file. >>> * defs.h: Move functions defs to common-defs.h. >>> * findvar.c (extract_signed_integer): Move to int-utils.c. >>> (extract_unsigned_integer): Likewise. >>> (extract_long_unsigned_integer): Likewise. >>> (store_signed_integer): Likewise. >>> (store_unsigned_integer): Likewise. >>> --- >>> gdb/Makefile.in | 9 ++- >>> gdb/common/common-defs.h | 1 + >>> gdb/common/int-utils.c | 199 >>> +++++++++++++++++++++++++++++++++++++++++++++++ >>> gdb/common/int-utils.h | 45 +++++++++++ >>> gdb/defs.h | 16 ---- >>> gdb/findvar.c | 176 >>> ----------------------------------------- >>> 6 files changed, 252 insertions(+), 194 deletions(-) >>> create mode 100644 gdb/common/int-utils.c >>> create mode 100644 gdb/common/int-utils.h >>> >>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in >>> index 0d7cf97..e20c5a6 100644 >>> --- a/gdb/Makefile.in >>> +++ b/gdb/Makefile.in >>> @@ -854,6 +854,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c >>> ada-valprint.c ada-tasks.c \ >>> infcall.c \ >>> infcmd.c inflow.c infrun.c \ >>> inline-frame.c \ >>> + common/int-utils.c \ >>> interps.c \ >>> jv-exp.y jv-lang.c jv-valprint.c jv-typeprint.c jv-varobj.c \ >>> language.c linespec.c location.c minidebug.c \ >>> @@ -985,7 +986,7 @@ i386-linux-nat.h common/common-defs.h >>> common/errors.h common/common-types.h \ >>> common/common-debug.h common/cleanups.h common/gdb_setjmp.h \ >>> common/common-exceptions.h target/target.h common/symbol.h \ >>> common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \ >>> -common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h \ >>> +common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h >>> common/int-utils.h \ >>> nat/linux-namespaces.h arch/arm.h common/gdb_sys_time.h >>> >>> # Header files that already have srcdir in them, or which are in >>> objdir. >>> @@ -1084,7 +1085,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ >>> common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \ >>> format.o registry.o btrace.o record-btrace.o waitstatus.o \ >>> print-utils.o rsp-low.o errors.o common-debug.o debug.o \ >>> - common-exceptions.o btrace-common.o fileio.o \ >>> + common-exceptions.o btrace-common.o fileio.o int-utils.o \ >>> $(SUBDIR_GCC_COMPILE_OBS) >>> >>> TSOBS = inflow.o >>> @@ -2265,6 +2266,10 @@ btrace-common.o: ${srcdir}/common/btrace-common.c >>> fileio.o: ${srcdir}/common/fileio.c >>> $(COMPILE) $(srcdir)/common/fileio.c >>> $(POSTCOMPILE) >>> +int-utils.o: ${srcdir}/common/int-utils.c >>> + $(COMPILE) $(srcdir)/common/int-utils.c >>> + $(POSTCOMPILE) >>> + >>> # >>> # gdb/target/ dependencies >>> # >>> diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h >>> index 2be0d7d..cb79234 100644 >>> --- a/gdb/common/common-defs.h >>> +++ b/gdb/common/common-defs.h >>> @@ -49,6 +49,7 @@ >>> #include "common-debug.h" >>> #include "cleanups.h" >>> #include "common-exceptions.h" >>> +#include "int-utils.h" >>> >>> #ifdef __cplusplus >>> # define EXTERN_C extern "C" >>> diff --git a/gdb/common/int-utils.c b/gdb/common/int-utils.c >>> new file mode 100644 >>> index 0000000..57d0dba >>> --- /dev/null >>> +++ b/gdb/common/int-utils.c >>> @@ -0,0 +1,199 @@ >>> +/* Shared utility routines for integer endianness manipulations. >>> + Copyright (C) 2015 Free Software Foundation, Inc. >>> + >>> + This file is part of GDB. >>> + >>> + This program is free software; you can redistribute it and/or modify >>> + it under the terms of the GNU General Public License as published by >>> + the Free Software Foundation; either version 3 of the License, or >>> + (at your option) any later version. >>> + >>> + This program is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + GNU General Public License for more details. >>> + >>> + You should have received a copy of the GNU General Public License >>> + along with this program. If not, see >>> . */ >>> + >>> +#include "common-defs.h" >>> + >>> +/* Basic byte-swapping routines. All 'extract' functions return a >>> + host-format integer from a target-format integer at ADDR which is >>> + LEN bytes long. */ >>> + >>> +LONGEST >>> +extract_signed_integer (const gdb_byte *addr, int len, >>> + enum bfd_endian byte_order) >>> +{ >>> + LONGEST retval; >>> + const unsigned char *p; >>> + const unsigned char *startaddr = addr; >>> + const unsigned char *endaddr = startaddr + len; >>> + >>> + if (len > (int) sizeof (LONGEST)) >>> + error (_("\ >>> +That operation is not available on integers of more than %d bytes."), >>> + (int) sizeof (LONGEST)); >>> + >>> + /* Start at the most significant end of the integer, and work towards >>> + the least significant. */ >>> + if (byte_order == BFD_ENDIAN_BIG) >>> + { >>> + p = startaddr; >>> + /* Do the sign extension once at the start. */ >>> + retval = ((LONGEST) * p ^ 0x80) - 0x80; >>> + for (++p; p < endaddr; ++p) >>> + retval = (retval << 8) | *p; >>> + } >>> + else >>> + { >>> + p = endaddr - 1; >>> + /* Do the sign extension once at the start. */ >>> + retval = ((LONGEST) * p ^ 0x80) - 0x80; >>> + for (--p; p >= startaddr; --p) >>> + retval = (retval << 8) | *p; >>> + } >>> + return retval; >>> +} >>> + >>> +ULONGEST >>> +extract_unsigned_integer (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; >>> + if (byte_order == BFD_ENDIAN_BIG) >>> + { >>> + for (p = startaddr; p < endaddr; ++p) >>> + retval = (retval << 8) | *p; >>> + } >>> + else >>> + { >>> + for (p = endaddr - 1; p >= startaddr; --p) >>> + retval = (retval << 8) | *p; >>> + } >>> + return retval; >>> +} >>> + >>> +/* 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 >>> + function returns 1 and sets *PVAL. Otherwise it returns 0. */ >>> + >>> +int >>> +extract_long_unsigned_integer (const gdb_byte *addr, int orig_len, >>> + enum bfd_endian byte_order, LONGEST *pval) >>> +{ >>> + const gdb_byte *p; >>> + const gdb_byte *first_addr; >>> + int len; >>> + >>> + len = orig_len; >>> + if (byte_order == BFD_ENDIAN_BIG) >>> + { >>> + for (p = addr; >>> + len > (int) sizeof (LONGEST) && p < addr + orig_len; >>> + p++) >>> + { >>> + if (*p == 0) >>> + len--; >>> + else >>> + break; >>> + } >>> + first_addr = p; >>> + } >>> + else >>> + { >>> + first_addr = addr; >>> + for (p = addr + orig_len - 1; >>> + len > (int) sizeof (LONGEST) && p >= addr; >>> + p--) >>> + { >>> + if (*p == 0) >>> + len--; >>> + else >>> + break; >>> + } >>> + } >>> + >>> + if (len <= (int) sizeof (LONGEST)) >>> + { >>> + *pval = (LONGEST) extract_unsigned_integer (first_addr, >>> + sizeof (LONGEST), >>> + byte_order); >>> + return 1; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +/* All 'store' functions accept a host-format integer and store a >>> + target-format integer at ADDR which is LEN bytes long. */ >>> + >>> +void >>> +store_signed_integer (gdb_byte *addr, int len, >>> + enum bfd_endian byte_order, LONGEST val) >>> +{ >>> + gdb_byte *p; >>> + gdb_byte *startaddr = addr; >>> + gdb_byte *endaddr = startaddr + len; >>> + >>> + /* Start at the least significant end of the integer, and work >>> towards >>> + the most significant. */ >>> + if (byte_order == BFD_ENDIAN_BIG) >>> + { >>> + for (p = endaddr - 1; p >= startaddr; --p) >>> + { >>> + *p = val & 0xff; >>> + val >>= 8; >>> + } >>> + } >>> + else >>> + { >>> + for (p = startaddr; p < endaddr; ++p) >>> + { >>> + *p = val & 0xff; >>> + val >>= 8; >>> + } >>> + } >>> +} >>> + >>> +void >>> +store_unsigned_integer (gdb_byte *addr, int len, >>> + enum bfd_endian byte_order, ULONGEST val) >>> +{ >>> + unsigned char *p; >>> + unsigned char *startaddr = (unsigned char *) addr; >>> + unsigned char *endaddr = startaddr + len; >>> + >>> + /* Start at the least significant end of the integer, and work >>> towards >>> + the most significant. */ >>> + if (byte_order == BFD_ENDIAN_BIG) >>> + { >>> + for (p = endaddr - 1; p >= startaddr; --p) >>> + { >>> + *p = val & 0xff; >>> + val >>= 8; >>> + } >>> + } >>> + else >>> + { >>> + for (p = startaddr; p < endaddr; ++p) >>> + { >>> + *p = val & 0xff; >>> + val >>= 8; >>> + } >>> + } >>> +} >>> diff --git a/gdb/common/int-utils.h b/gdb/common/int-utils.h >>> new file mode 100644 >>> index 0000000..c170348 >>> --- /dev/null >>> +++ b/gdb/common/int-utils.h >>> @@ -0,0 +1,45 @@ >>> +/* Shared utility routines for integer endianness manipulations. >>> + Copyright (C) 2015 Free Software Foundation, Inc. >>> + >>> + This file is part of GDB. >>> + >>> + This program is free software; you can redistribute it and/or modify >>> + it under the terms of the GNU General Public License as published by >>> + the Free Software Foundation; either version 3 of the License, or >>> + (at your option) any later version. >>> + >>> + This program is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + GNU General Public License for more details. >>> + >>> + You should have received a copy of the GNU General Public License >>> + along with this program. If not, see >>> . */ >>> + >>> + >>> +#ifndef INT_UTILS_H >>> +#define INT_UTILS_H 1 >>> + >>> +#ifdef GDBSERVER >>> +/* Allow this enum without requiring bfd in gdbserver. */ >>> +enum bfd_endian { BFD_ENDIAN_BIG, BFD_ENDIAN_LITTLE, >>> BFD_ENDIAN_UNKNOWN }; >>> +#else >>> +#include "bfd.h" >>> +#endif >>> + >>> +extern LONGEST extract_signed_integer (const gdb_byte *, int, >>> + enum bfd_endian); >>> + >>> +extern ULONGEST extract_unsigned_integer (const gdb_byte *, int, >>> + enum bfd_endian); >>> + >>> +extern int extract_long_unsigned_integer (const gdb_byte *, int, >>> + enum bfd_endian, LONGEST *); >>> + >>> +extern void store_signed_integer (gdb_byte *, int, >>> + enum bfd_endian, LONGEST); >>> + >>> +extern void store_unsigned_integer (gdb_byte *, int, >>> + enum bfd_endian, ULONGEST); >>> + >>> +#endif /* INT_UTILS_H */ >>> diff --git a/gdb/defs.h b/gdb/defs.h >>> index 03f7e8a..e292977 100644 >>> --- a/gdb/defs.h >>> +++ b/gdb/defs.h >>> @@ -596,28 +596,12 @@ enum { MAX_REGISTER_SIZE = 64 }; >>> >>> /* In findvar.c. */ >>> >>> -extern LONGEST extract_signed_integer (const gdb_byte *, int, >>> - enum bfd_endian); >>> - >>> -extern ULONGEST extract_unsigned_integer (const gdb_byte *, int, >>> - enum bfd_endian); >>> - >>> -extern int extract_long_unsigned_integer (const gdb_byte *, int, >>> - enum bfd_endian, LONGEST *); >>> - >>> extern CORE_ADDR extract_typed_address (const gdb_byte *buf, >>> struct type *type); >>> >>> -extern void store_signed_integer (gdb_byte *, int, >>> - enum bfd_endian, LONGEST); >>> - >>> -extern void store_unsigned_integer (gdb_byte *, int, >>> - enum bfd_endian, ULONGEST); >>> - >>> extern void store_typed_address (gdb_byte *buf, struct type *type, >>> CORE_ADDR addr); >>> >>> - >>> /* From valops.c */ >>> >>> extern int watchdog; >>> diff --git a/gdb/findvar.c b/gdb/findvar.c >>> index 1c077f7..2299ca4 100644 >>> --- a/gdb/findvar.c >>> +++ b/gdb/findvar.c >>> @@ -46,124 +46,6 @@ >>> you lose >>> #endif >>> >>> -LONGEST >>> -extract_signed_integer (const gdb_byte *addr, int len, >>> - enum bfd_endian byte_order) >>> -{ >>> - LONGEST retval; >>> - const unsigned char *p; >>> - const unsigned char *startaddr = addr; >>> - const unsigned char *endaddr = startaddr + len; >>> - >>> - if (len > (int) sizeof (LONGEST)) >>> - error (_("\ >>> -That operation is not available on integers of more than %d bytes."), >>> - (int) sizeof (LONGEST)); >>> - >>> - /* Start at the most significant end of the integer, and work towards >>> - the least significant. */ >>> - if (byte_order == BFD_ENDIAN_BIG) >>> - { >>> - p = startaddr; >>> - /* Do the sign extension once at the start. */ >>> - retval = ((LONGEST) * p ^ 0x80) - 0x80; >>> - for (++p; p < endaddr; ++p) >>> - retval = (retval << 8) | *p; >>> - } >>> - else >>> - { >>> - p = endaddr - 1; >>> - /* Do the sign extension once at the start. */ >>> - retval = ((LONGEST) * p ^ 0x80) - 0x80; >>> - for (--p; p >= startaddr; --p) >>> - retval = (retval << 8) | *p; >>> - } >>> - return retval; >>> -} >>> - >>> -ULONGEST >>> -extract_unsigned_integer (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; >>> - if (byte_order == BFD_ENDIAN_BIG) >>> - { >>> - for (p = startaddr; p < endaddr; ++p) >>> - retval = (retval << 8) | *p; >>> - } >>> - else >>> - { >>> - for (p = endaddr - 1; p >= startaddr; --p) >>> - retval = (retval << 8) | *p; >>> - } >>> - return retval; >>> -} >>> - >>> -/* 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 >>> - function returns 1 and sets *PVAL. Otherwise it returns 0. */ >>> - >>> -int >>> -extract_long_unsigned_integer (const gdb_byte *addr, int orig_len, >>> - enum bfd_endian byte_order, LONGEST *pval) >>> -{ >>> - const gdb_byte *p; >>> - const gdb_byte *first_addr; >>> - int len; >>> - >>> - len = orig_len; >>> - if (byte_order == BFD_ENDIAN_BIG) >>> - { >>> - for (p = addr; >>> - len > (int) sizeof (LONGEST) && p < addr + orig_len; >>> - p++) >>> - { >>> - if (*p == 0) >>> - len--; >>> - else >>> - break; >>> - } >>> - first_addr = p; >>> - } >>> - else >>> - { >>> - first_addr = addr; >>> - for (p = addr + orig_len - 1; >>> - len > (int) sizeof (LONGEST) && p >= addr; >>> - p--) >>> - { >>> - if (*p == 0) >>> - len--; >>> - else >>> - break; >>> - } >>> - } >>> - >>> - if (len <= (int) sizeof (LONGEST)) >>> - { >>> - *pval = (LONGEST) extract_unsigned_integer (first_addr, >>> - sizeof (LONGEST), >>> - byte_order); >>> - return 1; >>> - } >>> - >>> - return 0; >>> -} >>> - >>> - >>> /* Treat the bytes at BUF as a pointer of type TYPE, and return the >>> address it represents. */ >>> CORE_ADDR >>> @@ -178,64 +60,6 @@ extract_typed_address (const gdb_byte *buf, >>> struct type *type) >>> return gdbarch_pointer_to_address (get_type_arch (type), type, buf); >>> } >>> >>> -/* All 'store' functions accept a host-format integer and store a >>> - target-format integer at ADDR which is LEN bytes long. */ >>> - >>> -void >>> -store_signed_integer (gdb_byte *addr, int len, >>> - enum bfd_endian byte_order, LONGEST val) >>> -{ >>> - gdb_byte *p; >>> - gdb_byte *startaddr = addr; >>> - gdb_byte *endaddr = startaddr + len; >>> - >>> - /* Start at the least significant end of the integer, and work >>> towards >>> - the most significant. */ >>> - if (byte_order == BFD_ENDIAN_BIG) >>> - { >>> - for (p = endaddr - 1; p >= startaddr; --p) >>> - { >>> - *p = val & 0xff; >>> - val >>= 8; >>> - } >>> - } >>> - else >>> - { >>> - for (p = startaddr; p < endaddr; ++p) >>> - { >>> - *p = val & 0xff; >>> - val >>= 8; >>> - } >>> - } >>> -} >>> - >>> -void >>> -store_unsigned_integer (gdb_byte *addr, int len, >>> - enum bfd_endian byte_order, ULONGEST val) >>> -{ >>> - unsigned char *p; >>> - unsigned char *startaddr = (unsigned char *) addr; >>> - unsigned char *endaddr = startaddr + len; >>> - >>> - /* Start at the least significant end of the integer, and work >>> towards >>> - the most significant. */ >>> - if (byte_order == BFD_ENDIAN_BIG) >>> - { >>> - for (p = endaddr - 1; p >= startaddr; --p) >>> - { >>> - *p = val & 0xff; >>> - val >>= 8; >>> - } >>> - } >>> - else >>> - { >>> - for (p = startaddr; p < endaddr; ++p) >>> - { >>> - *p = val & 0xff; >>> - val >>= 8; >>> - } >>> - } >>> -} >>> >>> /* Store the address ADDR as a pointer of type TYPE at BUF, in target >>> form. */ >>> -- >>> 1.9.1 >>