From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109953 invoked by alias); 11 Sep 2015 17:16:12 -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 109944 invoked by uid 89); 11 Sep 2015 17:16:12 -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: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 11 Sep 2015 17:16:09 +0000 Received: from EUSAAHC001.ericsson.se (Unknown_Domain [147.117.188.75]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 6B.DE.26730.AA1A2F55; Fri, 11 Sep 2015 11:40:58 +0200 (CEST) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.77) with Microsoft SMTP Server id 14.3.248.2; Fri, 11 Sep 2015 13:16:05 -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> CC: From: Antoine Tremblay Message-ID: <55F30C55.3080507@ericsson.com> Date: Fri, 11 Sep 2015 17:16: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: <20150911142442.GA23515@blade.nx> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00238.txt.bz2 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 ? > 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 >