[PATCH 2/7] Move some integer operations to common.

Antoine Tremblay antoine.tremblay@ericsson.com
Fri Sep 11 17:16:00 GMT 2015



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 <http://www.gnu.org/licenses/>.  */
>> +
>> +#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 <http://www.gnu.org/licenses/>.  */
>> +
>> +
>> +#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
>



More information about the Gdb-patches mailing list