[rfc] Make DWARF-2 "address size" explicit

[rfc] Make DWARF-2 "address size" explicit

[Ulrich Weigand]

Hello,

the DWARF-2 standard in some places refers to the "size of an address
on the target machine", and uses this parameter implicitly to define
the semantics of several opcodes (e.g. DW_OP_deref).

GDB currently implements this as
  gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT
buried deep inside the call chain, which makes it difficult to
eliminate those uses of the current_gdbarch global variable.

This patch changes this by making the parameter explicit as member
of the dwarf_expr_context structure.  The core DWARF expresssion
engine thus no longer needs to make this assumption.

At those places where new expression contexts are initialized, the
addr_size member can now be set according to a frame architecture
or (by a follow-on patch) an objfile architecture.

This patch does not change the behaviour of the code, with one
exception: it appears the DW_OP_deref_size operator was implemented
incorrectly, as it always converts a full address-sized buffer even
if only a smaller amount of data was read from the target.  This is
fixed now by passed the proper size to dwarf2_read_address.

Tested on amd64-linux.

Bye,
Ulrich


ChangeLog:

	* dwarf2expr.h (struct dwarf_expr_context): Add ADDR_SIZE member.
	(dwarf2_read_address): Update prototype.

	* dwarf2expr.c (unsigned_address_type): Add ADDR_SIZE parameter.
	(signed_address_type): Likewise.
	(dwarf2_read_address): Replace BYTES_READ parameter with ADDR_SIZE.
	(execute_stack_op): Update calls to unsigned_address_type,
	signed_address_type and dwarf2_read_address.  Fix implementation
	of DW_OP_deref_size.

	* dwarf2-frame.c (execute_stack_op): Set ctx->addr_size.
	(execute_cfa_program): Update calls to dwarf2_read_address.

	* dwarf2loc.c (find_location_expression): Update calls to
	dwarf2_read_address.
	(locexpr_describe_location): Likewise.
	(dwarf2_evaluate_loc_desc): Set ctx->addr_size.
	(dwarf2_loc_desc_needs_frame): Likewise.


diff -urNp gdb-orig/gdb/dwarf2expr.c gdb-head/gdb/dwarf2expr.c
--- gdb-orig/gdb/dwarf2expr.c	2007-12-08 18:47:58.000000000 +0100
+++ gdb-head/gdb/dwarf2expr.c	2007-12-08 18:48:58.000000000 +0100
@@ -31,7 +31,7 @@
 
 static void execute_stack_op (struct dwarf_expr_context *,
 			      gdb_byte *, gdb_byte *);
-static struct type *unsigned_address_type (void);
+static struct type *unsigned_address_type (int);
 
 /* Create a new context for the expression evaluator.  */
 
@@ -191,20 +191,17 @@ read_sleb128 (gdb_byte *buf, gdb_byte *b
   return buf;
 }
 
-/* Read an address from BUF, and verify that it doesn't extend past
-   BUF_END.  The address is returned, and *BYTES_READ is set to the
-   number of bytes read from BUF.  */
+/* Read an address of size ADDR_SIZE from BUF, and verify that it
+   doesn't extend past BUF_END.  */
 
 CORE_ADDR
-dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end, int *bytes_read)
+dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end, int addr_size)
 {
   CORE_ADDR result;
 
-  if (buf_end - buf < gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  if (buf_end - buf < addr_size)
     error (_("dwarf2_read_address: Corrupted DWARF expression."));
 
-  *bytes_read = gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT;
-
   /* For most architectures, calling extract_unsigned_integer() alone
      is sufficient for extracting an address.  However, some
      architectures (e.g. MIPS) use signed addresses and using
@@ -228,21 +225,18 @@ dwarf2_read_address (gdb_byte *buf, gdb_
      address being returned.  */
 
   result = value_as_address (value_from_longest 
-			      (unsigned_address_type (),
-			       extract_unsigned_integer 
-				 (buf,
-				  gdbarch_addr_bit (current_gdbarch)
-				    / TARGET_CHAR_BIT)));
-
+			      (unsigned_address_type (addr_size),
+			       extract_unsigned_integer (buf, addr_size)));
   return result;
 }
 
-/* Return the type of an address, for unsigned arithmetic.  */
+/* Return the type of an address of size ADDR_SIZE,
+   for unsigned arithmetic.  */
 
 static struct type *
-unsigned_address_type (void)
+unsigned_address_type (int addr_size)
 {
-  switch (gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  switch (addr_size)
     {
     case 2:
       return builtin_type_uint16;
@@ -256,12 +250,13 @@ unsigned_address_type (void)
     }
 }
 
-/* Return the type of an address, for signed arithmetic.  */
+/* Return the type of an address of size ADDR_SIZE,
+   for signed arithmetic.  */
 
 static struct type *
-signed_address_type (void)
+signed_address_type (int addr_size)
 {
-  switch (gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  switch (addr_size)
     {
     case 2:
       return builtin_type_int16;
@@ -291,7 +286,6 @@ execute_stack_op (struct dwarf_expr_cont
       CORE_ADDR result;
       ULONGEST uoffset, reg;
       LONGEST offset;
-      int bytes_read;
 
       switch (op)
 	{
@@ -331,8 +325,8 @@ execute_stack_op (struct dwarf_expr_cont
 	  break;
 
 	case DW_OP_addr:
-	  result = dwarf2_read_address (op_ptr, op_end, &bytes_read);
-	  op_ptr += bytes_read;
+	  result = dwarf2_read_address (op_ptr, op_end, ctx->addr_size);
+	  op_ptr += ctx->addr_size;
 	  break;
 
 	case DW_OP_const1u:
@@ -549,34 +543,20 @@ execute_stack_op (struct dwarf_expr_cont
 	    {
 	    case DW_OP_deref:
 	      {
-		gdb_byte *buf = alloca (gdbarch_addr_bit (current_gdbarch)
-					  / TARGET_CHAR_BIT);
-		int bytes_read;
-
-		(ctx->read_mem) (ctx->baton, buf, result,
-				 gdbarch_addr_bit (current_gdbarch)
-				   / TARGET_CHAR_BIT);
-		result = dwarf2_read_address (buf,
-					      buf + (gdbarch_addr_bit
-						       (current_gdbarch)
-						     / TARGET_CHAR_BIT),
-					      &bytes_read);
+		gdb_byte *buf = alloca (ctx->addr_size);
+		(ctx->read_mem) (ctx->baton, buf, result, ctx->addr_size);
+		result = dwarf2_read_address (buf, buf + ctx->addr_size,
+					      ctx->addr_size);
 	      }
 	      break;
 
 	    case DW_OP_deref_size:
 	      {
-		gdb_byte *buf
-		   = alloca (gdbarch_addr_bit (current_gdbarch)
-			      / TARGET_CHAR_BIT);
-		int bytes_read;
-
-		(ctx->read_mem) (ctx->baton, buf, result, *op_ptr++);
-		result = dwarf2_read_address (buf,
-					      buf + (gdbarch_addr_bit
-						      (current_gdbarch)
-						     / TARGET_CHAR_BIT),
-					      &bytes_read);
+		int addr_size = *op_ptr++;
+		gdb_byte *buf = alloca (addr_size);
+		(ctx->read_mem) (ctx->baton, buf, result, addr_size);
+		result = dwarf2_read_address (buf, buf + addr_size,
+					      addr_size);
 	      }
 	      break;
 
@@ -627,8 +607,10 @@ execute_stack_op (struct dwarf_expr_cont
 	    first = dwarf_expr_fetch (ctx, 0);
 	    dwarf_expr_pop (ctx);
 
-	    val1 = value_from_longest (unsigned_address_type (), first);
-	    val2 = value_from_longest (unsigned_address_type (), second);
+	    val1 = value_from_longest
+		     (unsigned_address_type (ctx->addr_size), first);
+	    val2 = value_from_longest
+		     (unsigned_address_type (ctx->addr_size), second);
 
 	    switch (op)
 	      {
@@ -661,7 +643,8 @@ execute_stack_op (struct dwarf_expr_cont
                 break;
 	      case DW_OP_shra:
 		binop = BINOP_RSH;
-		val1 = value_from_longest (signed_address_type (), first);
+		val1 = value_from_longest
+			 (signed_address_type (ctx->addr_size), first);
 		break;
 	      case DW_OP_xor:
 		binop = BINOP_BITWISE_XOR;
diff -urNp gdb-orig/gdb/dwarf2expr.h gdb-head/gdb/dwarf2expr.h
--- gdb-orig/gdb/dwarf2expr.h	2007-12-08 18:47:58.000000000 +0100
+++ gdb-head/gdb/dwarf2expr.h	2007-12-08 18:47:53.000000000 +0100
@@ -33,6 +33,9 @@ struct dwarf_expr_context
      number of elements allocated to the stack.  */
   int stack_len, stack_allocated;
 
+  /* Target address size in bytes.  */
+  int addr_size;
+
   /* An opaque argument provided by the caller, which will be passed
      to all of the callback functions.  */
   void *baton;
@@ -135,6 +138,6 @@ CORE_ADDR dwarf_expr_fetch (struct dwarf
 gdb_byte *read_uleb128 (gdb_byte *buf, gdb_byte *buf_end, ULONGEST * r);
 gdb_byte *read_sleb128 (gdb_byte *buf, gdb_byte *buf_end, LONGEST * r);
 CORE_ADDR dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end,
-			       int *bytes_read);
+			       int addr_size);
 
 #endif /* dwarf2expr.h */
diff -urNp gdb-orig/gdb/dwarf2-frame.c gdb-head/gdb/dwarf2-frame.c
--- gdb-orig/gdb/dwarf2-frame.c	2007-12-08 18:47:58.000000000 +0100
+++ gdb-head/gdb/dwarf2-frame.c	2007-12-08 18:47:53.000000000 +0100
@@ -272,10 +272,12 @@ static CORE_ADDR
 execute_stack_op (gdb_byte *exp, ULONGEST len,
 		  struct frame_info *next_frame, CORE_ADDR initial)
 {
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
   struct dwarf_expr_context *ctx;
   CORE_ADDR result;
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
   ctx->baton = next_frame;
   ctx->read_reg = read_reg;
   ctx->read_mem = read_mem;
@@ -301,8 +303,8 @@ execute_cfa_program (gdb_byte *insn_ptr,
 		     struct dwarf2_frame_state *fs, int eh_frame_p)
 {
   CORE_ADDR pc = frame_pc_unwind (next_frame);
-  int bytes_read;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  int addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
 
   while (insn_ptr < insn_end && fs->pc <= pc)
     {
@@ -347,8 +349,8 @@ register %s (#%d) at 0x%s"),
 	  switch (insn)
 	    {
 	    case DW_CFA_set_loc:
-	      fs->pc = dwarf2_read_address (insn_ptr, insn_end, &bytes_read);
-	      insn_ptr += bytes_read;
+	      fs->pc = dwarf2_read_address (insn_ptr, insn_end, addr_size);
+	      insn_ptr += addr_size;
 	      break;
 
 	    case DW_CFA_advance_loc1:
diff -urNp gdb-orig/gdb/dwarf2loc.c gdb-head/gdb/dwarf2loc.c
--- gdb-orig/gdb/dwarf2loc.c	2007-12-08 18:47:58.000000000 +0100
+++ gdb-head/gdb/dwarf2loc.c	2007-12-08 18:47:53.000000000 +0100
@@ -66,10 +66,10 @@ find_location_expression (struct dwarf2_
 
   while (1)
     {
-      low = dwarf2_read_address (loc_ptr, buf_end, &length);
-      loc_ptr += length;
-      high = dwarf2_read_address (loc_ptr, buf_end, &length);
-      loc_ptr += length;
+      low = dwarf2_read_address (loc_ptr, buf_end, addr_size);
+      loc_ptr += addr_size;
+      high = dwarf2_read_address (loc_ptr, buf_end, addr_size);
+      loc_ptr += addr_size;
 
       /* An end-of-list entry.  */
       if (low == 0 && high == 0)
@@ -208,6 +208,7 @@ dwarf2_evaluate_loc_desc (struct symbol 
   baton.objfile = objfile;
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = gdbarch_addr_bit (arch) / TARGET_CHAR_BIT;
   ctx->baton = &baton;
   ctx->read_reg = dwarf_expr_read_reg;
   ctx->read_mem = dwarf_expr_read_mem;
@@ -327,6 +328,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *d
   baton.needs_frame = 0;
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT;
   ctx->baton = &baton;
   ctx->read_reg = needs_frame_read_reg;
   ctx->read_mem = needs_frame_read_mem;
@@ -448,6 +450,7 @@ locexpr_describe_location (struct symbol
 {
   /* FIXME: be more extensive.  */
   struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (symbol);
+  int addr_size = gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT;
 
   if (dlbaton->size == 1
       && dlbaton->data[0] >= DW_OP_reg0
@@ -477,10 +480,9 @@ locexpr_describe_location (struct symbol
       && dlbaton->data[dlbaton->size - 1] == DW_OP_GNU_push_tls_address)
     if (dlbaton->data[0] == DW_OP_addr)
       {
-	int bytes_read;
 	CORE_ADDR offset = dwarf2_read_address (&dlbaton->data[1],
 						&dlbaton->data[dlbaton->size - 1],
-						&bytes_read);
+						addr_size);
 	fprintf_filtered (stream, 
 			  "a thread-local variable at offset %s in the "
 			  "thread-local storage for `%s'",
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Make DWARF-2 "address size" explicit

[Daniel Jacobowitz]

On Sun, Dec 09, 2007 at 08:39:55PM +0100, Ulrich Weigand wrote:
> This patch changes this by making the parameter explicit as member
> of the dwarf_expr_context structure.  The core DWARF expresssion
> engine thus no longer needs to make this assumption.

Seems right to me!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Make DWARF-2 "address size" explicit

[Jim Blandy]

Daniel Jacobowitz <drow at false.org> writes:
> On Sun, Dec 09, 2007 at 08:39:55PM +0100, Ulrich Weigand wrote:
>> This patch changes this by making the parameter explicit as member
>> of the dwarf_expr_context structure.  The core DWARF expresssion
>> engine thus no longer needs to make this assumption.
>
> Seems right to me!

This is certainly a step in the right direction.  However, the proper
value to use for addr_size is the one that comes from the header of
the compilation unit containing the expression.  (The fact that
there's no compilation unit associated with the frame information is
an existing problem.)

I had a patch to carry that through from a long while back.  A fellow
from Intel has been working on updating it; I've encouraged him to
post it here.

http://sourceware.org/ml/gdb-patches/2006-05/msg00226.html

Re: [rfc] Make DWARF-2 "address size" explicit

[Ulrich Weigand]

Hi Jim,

> This is certainly a step in the right direction.  However, the proper
> value to use for addr_size is the one that comes from the header of
> the compilation unit containing the expression.  (The fact that
> there's no compilation unit associated with the frame information is
> an existing problem.)
> 
> I had a patch to carry that through from a long while back.  A fellow
> from Intel has been working on updating it; I've encouraged him to
> post it here.
> 
> http://sourceware.org/ml/gdb-patches/2006-05/msg00226.html

Thanks for pointing this out!  I've updated my patch to retrieve the
address size from the CU header, based on your patch.  However, the
implementation details are a bit different (e.g. because there is no 
dwarf2_pinfo any more):  I use an (opaque) struct dwarf2_cu * in the
location list / location expression batons, and provide a number of
accessor functions to retrieve CU-associated information:
  dwarf2_cu_objfile
  dwarf2_cu_base_address
  dwarf2_cu_addr_size

Apart from that, the behaviour should be identical, with one
exception: what address size to use for CFI.  Your patch uses
  size_of_encoded_value (DW_EH_PE_absptr)
which basically boils down to:
  gdbarch_ptr_bit (current_gdbarch) / TARGET_CHAR_BIT

This would be an effective change in behaviour to what we have
now, which is:
  gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT

My patch uses in effect
  gdbarch_addr_bit (get_frame_arch (frame)) / TARGET_CHAR_BIT
which does not change behaviour.

However, it might well be that the original code is simply wrong
and we *should* be using ...ptr_bit instead of ...addr_bit.
What do you think?

Tested on i386-linux with no regressions.

Bye,
Ulrich


ChangeLog:

	* dwarf2expr.h (struct dwarf_expr_context): Add ADDR_SIZE member.
	(dwarf2_read_address): Update prototype.

	* dwarf2expr.c (unsigned_address_type): Add ADDR_SIZE parameter.
	(signed_address_type): Likewise.
	(dwarf2_read_address): Replace BYTES_READ parameter with ADDR_SIZE.
	(execute_stack_op): Update calls to unsigned_address_type,
	signed_address_type and dwarf2_read_address.  Fix implementation
	of DW_OP_deref_size.

	* dwarf2-frame.c (execute_stack_op): Set ctx->addr_size.
	(execute_cfa_program): Update calls to dwarf2_read_address.

	* dwarfloc.h (dwarf2_cu_objfile): Add prototype.
	(dwarf2_cu_addr_size, dwarf2_cu_base_address): Likewise.
	(struct dwarf2_locexpr_baton): Replace OBJFILE with CU.
	(struct dwarf2_loclist_baton): Likewise.  Remove BASE_ADDRESS.

	* dwarf2loc.c (find_location_expression): Update calls to
	dwarf2_read_address.  Use dwarf2_cu_objfile, dwarf2_cu_addr_size,
	and dwarf2_cu_base_address to retrieve CU parameters.
	(locexpr_describe_location): Likewise.
	(dwarf2_evaluate_loc_desc): Replace OBJFILE with CU parameter.
	Set ctx->addr_size to dwarf2_cu_addr_size (cu).
	(dwarf2_loc_desc_needs_frame): Add CU parameter.  Set ctx->addr_size
	to dwarf2_cu_addr_size (cu).
	(locexpr_read_variable): Update dwarf2_evaluate_loc_desc call.
	(loclist_read_variable): Likewise.
	(locexpr_read_needs_frame): Update dwarf2_loc_desc_needs_frame call.

	* dwarf2read.c (dwarf2_symbol_mark_computed): Set baton->cu
	instead of baton->objfile.  Do not set baton->base_address.
	(dwarf2_cu_obfile): New function.
	(dwarf2_cu_addr_size, dwarf2_cu_base_address): Likewise.


diff -urNp gdb-orig/gdb/dwarf2expr.c gdb-head/gdb/dwarf2expr.c
--- gdb-orig/gdb/dwarf2expr.c	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2expr.c	2008-01-14 15:15:06.000000000 +0100
@@ -32,7 +32,7 @@
 
 static void execute_stack_op (struct dwarf_expr_context *,
 			      gdb_byte *, gdb_byte *);
-static struct type *unsigned_address_type (void);
+static struct type *unsigned_address_type (int);
 
 /* Create a new context for the expression evaluator.  */
 
@@ -192,20 +192,17 @@ read_sleb128 (gdb_byte *buf, gdb_byte *b
   return buf;
 }
 
-/* Read an address from BUF, and verify that it doesn't extend past
-   BUF_END.  The address is returned, and *BYTES_READ is set to the
-   number of bytes read from BUF.  */
+/* Read an address of size ADDR_SIZE from BUF, and verify that it
+   doesn't extend past BUF_END.  */
 
 CORE_ADDR
-dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end, int *bytes_read)
+dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end, int addr_size)
 {
   CORE_ADDR result;
 
-  if (buf_end - buf < gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  if (buf_end - buf < addr_size)
     error (_("dwarf2_read_address: Corrupted DWARF expression."));
 
-  *bytes_read = gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT;
-
   /* For most architectures, calling extract_unsigned_integer() alone
      is sufficient for extracting an address.  However, some
      architectures (e.g. MIPS) use signed addresses and using
@@ -229,21 +226,18 @@ dwarf2_read_address (gdb_byte *buf, gdb_
      address being returned.  */
 
   result = value_as_address (value_from_longest 
-			      (unsigned_address_type (),
-			       extract_unsigned_integer 
-				 (buf,
-				  gdbarch_addr_bit (current_gdbarch)
-				    / TARGET_CHAR_BIT)));
-
+			      (unsigned_address_type (addr_size),
+			       extract_unsigned_integer (buf, addr_size)));
   return result;
 }
 
-/* Return the type of an address, for unsigned arithmetic.  */
+/* Return the type of an address of size ADDR_SIZE,
+   for unsigned arithmetic.  */
 
 static struct type *
-unsigned_address_type (void)
+unsigned_address_type (int addr_size)
 {
-  switch (gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  switch (addr_size)
     {
     case 2:
       return builtin_type_uint16;
@@ -257,12 +251,13 @@ unsigned_address_type (void)
     }
 }
 
-/* Return the type of an address, for signed arithmetic.  */
+/* Return the type of an address of size ADDR_SIZE,
+   for signed arithmetic.  */
 
 static struct type *
-signed_address_type (void)
+signed_address_type (int addr_size)
 {
-  switch (gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT)
+  switch (addr_size)
     {
     case 2:
       return builtin_type_int16;
@@ -292,7 +287,6 @@ execute_stack_op (struct dwarf_expr_cont
       CORE_ADDR result;
       ULONGEST uoffset, reg;
       LONGEST offset;
-      int bytes_read;
 
       switch (op)
 	{
@@ -332,8 +326,8 @@ execute_stack_op (struct dwarf_expr_cont
 	  break;
 
 	case DW_OP_addr:
-	  result = dwarf2_read_address (op_ptr, op_end, &bytes_read);
-	  op_ptr += bytes_read;
+	  result = dwarf2_read_address (op_ptr, op_end, ctx->addr_size);
+	  op_ptr += ctx->addr_size;
 	  break;
 
 	case DW_OP_const1u:
@@ -550,34 +544,20 @@ execute_stack_op (struct dwarf_expr_cont
 	    {
 	    case DW_OP_deref:
 	      {
-		gdb_byte *buf = alloca (gdbarch_addr_bit (current_gdbarch)
-					  / TARGET_CHAR_BIT);
-		int bytes_read;
-
-		(ctx->read_mem) (ctx->baton, buf, result,
-				 gdbarch_addr_bit (current_gdbarch)
-				   / TARGET_CHAR_BIT);
-		result = dwarf2_read_address (buf,
-					      buf + (gdbarch_addr_bit
-						       (current_gdbarch)
-						     / TARGET_CHAR_BIT),
-					      &bytes_read);
+		gdb_byte *buf = alloca (ctx->addr_size);
+		(ctx->read_mem) (ctx->baton, buf, result, ctx->addr_size);
+		result = dwarf2_read_address (buf, buf + ctx->addr_size,
+					      ctx->addr_size);
 	      }
 	      break;
 
 	    case DW_OP_deref_size:
 	      {
-		gdb_byte *buf
-		   = alloca (gdbarch_addr_bit (current_gdbarch)
-			      / TARGET_CHAR_BIT);
-		int bytes_read;
-
-		(ctx->read_mem) (ctx->baton, buf, result, *op_ptr++);
-		result = dwarf2_read_address (buf,
-					      buf + (gdbarch_addr_bit
-						      (current_gdbarch)
-						     / TARGET_CHAR_BIT),
-					      &bytes_read);
+		int addr_size = *op_ptr++;
+		gdb_byte *buf = alloca (addr_size);
+		(ctx->read_mem) (ctx->baton, buf, result, addr_size);
+		result = dwarf2_read_address (buf, buf + addr_size,
+					      addr_size);
 	      }
 	      break;
 
@@ -628,8 +608,10 @@ execute_stack_op (struct dwarf_expr_cont
 	    first = dwarf_expr_fetch (ctx, 0);
 	    dwarf_expr_pop (ctx);
 
-	    val1 = value_from_longest (unsigned_address_type (), first);
-	    val2 = value_from_longest (unsigned_address_type (), second);
+	    val1 = value_from_longest
+		     (unsigned_address_type (ctx->addr_size), first);
+	    val2 = value_from_longest
+		     (unsigned_address_type (ctx->addr_size), second);
 
 	    switch (op)
 	      {
@@ -662,7 +644,8 @@ execute_stack_op (struct dwarf_expr_cont
                 break;
 	      case DW_OP_shra:
 		binop = BINOP_RSH;
-		val1 = value_from_longest (signed_address_type (), first);
+		val1 = value_from_longest
+			 (signed_address_type (ctx->addr_size), first);
 		break;
 	      case DW_OP_xor:
 		binop = BINOP_BITWISE_XOR;
diff -urNp gdb-orig/gdb/dwarf2expr.h gdb-head/gdb/dwarf2expr.h
--- gdb-orig/gdb/dwarf2expr.h	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2expr.h	2008-01-14 15:15:06.000000000 +0100
@@ -34,6 +34,9 @@ struct dwarf_expr_context
      number of elements allocated to the stack.  */
   int stack_len, stack_allocated;
 
+  /* Target address size in bytes.  */
+  int addr_size;
+
   /* An opaque argument provided by the caller, which will be passed
      to all of the callback functions.  */
   void *baton;
@@ -136,6 +139,6 @@ CORE_ADDR dwarf_expr_fetch (struct dwarf
 gdb_byte *read_uleb128 (gdb_byte *buf, gdb_byte *buf_end, ULONGEST * r);
 gdb_byte *read_sleb128 (gdb_byte *buf, gdb_byte *buf_end, LONGEST * r);
 CORE_ADDR dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end,
-			       int *bytes_read);
+			       int addr_size);
 
 #endif /* dwarf2expr.h */
diff -urNp gdb-orig/gdb/dwarf2-frame.c gdb-head/gdb/dwarf2-frame.c
--- gdb-orig/gdb/dwarf2-frame.c	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2-frame.c	2008-01-14 15:15:06.000000000 +0100
@@ -302,10 +302,12 @@ static CORE_ADDR
 execute_stack_op (gdb_byte *exp, ULONGEST len,
 		  struct frame_info *next_frame, CORE_ADDR initial)
 {
+  struct gdbarch *gdbarch = get_frame_arch (next_frame);
   struct dwarf_expr_context *ctx;
   CORE_ADDR result;
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
   ctx->baton = next_frame;
   ctx->read_reg = read_reg;
   ctx->read_mem = read_mem;
@@ -331,8 +333,8 @@ execute_cfa_program (gdb_byte *insn_ptr,
 		     struct dwarf2_frame_state *fs, int eh_frame_p)
 {
   CORE_ADDR pc = frame_pc_unwind (next_frame);
-  int bytes_read;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
+  int addr_size = gdbarch_addr_bit (gdbarch) / TARGET_CHAR_BIT;
 
   while (insn_ptr < insn_end && fs->pc <= pc)
     {
@@ -362,8 +364,8 @@ execute_cfa_program (gdb_byte *insn_ptr,
 	  switch (insn)
 	    {
 	    case DW_CFA_set_loc:
-	      fs->pc = dwarf2_read_address (insn_ptr, insn_end, &bytes_read);
-	      insn_ptr += bytes_read;
+	      fs->pc = dwarf2_read_address (insn_ptr, insn_end, addr_size);
+	      insn_ptr += addr_size;
 	      break;
 
 	    case DW_CFA_advance_loc1:
diff -urNp gdb-orig/gdb/dwarf2loc.c gdb-head/gdb/dwarf2loc.c
--- gdb-orig/gdb/dwarf2loc.c	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2loc.c	2008-01-14 15:59:19.000000000 +0100
@@ -54,22 +54,23 @@ find_location_expression (struct dwarf2_
   CORE_ADDR low, high;
   gdb_byte *loc_ptr, *buf_end;
   int length;
-  unsigned int addr_size = gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT;
+  struct objfile *objfile = dwarf2_cu_objfile (baton->cu);
+  unsigned int addr_size = dwarf2_cu_addr_size (baton->cu);
   CORE_ADDR base_mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
   /* Adjust base_address for relocatable objects.  */
-  CORE_ADDR base_offset = ANOFFSET (baton->objfile->section_offsets,
-				    SECT_OFF_TEXT (baton->objfile));
-  CORE_ADDR base_address = baton->base_address + base_offset;
+  CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets,
+				    SECT_OFF_TEXT (objfile));
+  CORE_ADDR base_address = dwarf2_cu_base_address (baton->cu) + base_offset;
 
   loc_ptr = baton->data;
   buf_end = baton->data + baton->size;
 
   while (1)
     {
-      low = dwarf2_read_address (loc_ptr, buf_end, &length);
-      loc_ptr += length;
-      high = dwarf2_read_address (loc_ptr, buf_end, &length);
-      loc_ptr += length;
+      low = dwarf2_read_address (loc_ptr, buf_end, addr_size);
+      loc_ptr += addr_size;
+      high = dwarf2_read_address (loc_ptr, buf_end, addr_size);
+      loc_ptr += addr_size;
 
       /* An end-of-list entry.  */
       if (low == 0 && high == 0)
@@ -189,7 +190,7 @@ dwarf_expr_tls_address (void *baton, COR
 static struct value *
 dwarf2_evaluate_loc_desc (struct symbol *var, struct frame_info *frame,
 			  gdb_byte *data, unsigned short size,
-			  struct objfile *objfile)
+			  struct dwarf2_cu *cu)
 {
   struct gdbarch *arch = get_frame_arch (frame);
   struct value *retval;
@@ -205,9 +206,10 @@ dwarf2_evaluate_loc_desc (struct symbol 
     }
 
   baton.frame = frame;
-  baton.objfile = objfile;
+  baton.objfile = dwarf2_cu_objfile (cu);
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = dwarf2_cu_addr_size (cu);
   ctx->baton = &baton;
   ctx->read_reg = dwarf_expr_read_reg;
   ctx->read_mem = dwarf_expr_read_mem;
@@ -318,7 +320,8 @@ needs_frame_tls_address (void *baton, CO
    requires a frame to evaluate.  */
 
 static int
-dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned short size)
+dwarf2_loc_desc_needs_frame (gdb_byte *data, unsigned short size,
+			     struct dwarf2_cu *cu)
 {
   struct needs_frame_baton baton;
   struct dwarf_expr_context *ctx;
@@ -327,6 +330,7 @@ dwarf2_loc_desc_needs_frame (gdb_byte *d
   baton.needs_frame = 0;
 
   ctx = new_dwarf_expr_context ();
+  ctx->addr_size = dwarf2_cu_addr_size (cu);
   ctx->baton = &baton;
   ctx->read_reg = needs_frame_read_reg;
   ctx->read_mem = needs_frame_read_mem;
@@ -429,7 +433,7 @@ locexpr_read_variable (struct symbol *sy
   struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (symbol);
   struct value *val;
   val = dwarf2_evaluate_loc_desc (symbol, frame, dlbaton->data, dlbaton->size,
-				  dlbaton->objfile);
+				  dlbaton->cu);
 
   return val;
 }
@@ -439,7 +443,8 @@ static int
 locexpr_read_needs_frame (struct symbol *symbol)
 {
   struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (symbol);
-  return dwarf2_loc_desc_needs_frame (dlbaton->data, dlbaton->size);
+  return dwarf2_loc_desc_needs_frame (dlbaton->data, dlbaton->size,
+				      dlbaton->cu);
 }
 
 /* Print a natural-language description of SYMBOL to STREAM.  */
@@ -448,6 +453,7 @@ locexpr_describe_location (struct symbol
 {
   /* FIXME: be more extensive.  */
   struct dwarf2_locexpr_baton *dlbaton = SYMBOL_LOCATION_BATON (symbol);
+  int addr_size = dwarf2_cu_addr_size (dlbaton->cu);
 
   if (dlbaton->size == 1
       && dlbaton->data[0] >= DW_OP_reg0
@@ -477,14 +483,14 @@ locexpr_describe_location (struct symbol
       && dlbaton->data[dlbaton->size - 1] == DW_OP_GNU_push_tls_address)
     if (dlbaton->data[0] == DW_OP_addr)
       {
-	int bytes_read;
+	struct objfile *objfile = dwarf2_cu_objfile (dlbaton->cu);
 	CORE_ADDR offset = dwarf2_read_address (&dlbaton->data[1],
 						&dlbaton->data[dlbaton->size - 1],
-						&bytes_read);
+						addr_size);
 	fprintf_filtered (stream, 
 			  "a thread-local variable at offset %s in the "
 			  "thread-local storage for `%s'",
-			  paddr_nz (offset), dlbaton->objfile->name);
+			  paddr_nz (offset), objfile->name);
 	return 1;
       }
   
@@ -545,8 +551,7 @@ loclist_read_variable (struct symbol *sy
       set_value_optimized_out (val, 1);
     }
   else
-    val = dwarf2_evaluate_loc_desc (symbol, frame, data, size,
-				    dlbaton->objfile);
+    val = dwarf2_evaluate_loc_desc (symbol, frame, data, size, dlbaton->cu);
 
   return val;
 }
diff -urNp gdb-orig/gdb/dwarf2loc.h gdb-head/gdb/dwarf2loc.h
--- gdb-orig/gdb/dwarf2loc.h	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2loc.h	2008-01-14 15:44:51.000000000 +0100
@@ -21,10 +21,21 @@
 #define DWARF2LOC_H
 
 struct symbol_ops;
+struct objfile;
+struct dwarf2_cu;
 
 /* This header is private to the DWARF-2 reader.  It is shared between
    dwarf2read.c and dwarf2loc.c.  */
 
+/* Return the OBJFILE associated with the compilation unit CU.  */
+struct objfile *dwarf2_cu_objfile (struct dwarf2_cu *cu);
+
+/* Return the address size given in the compilation unit header for CU.  */
+CORE_ADDR dwarf2_cu_addr_size (struct dwarf2_cu *cu);
+
+/* Return the base address given in the compilation unit header for CU.  */
+CORE_ADDR dwarf2_cu_base_address (struct dwarf2_cu *cu);
+
 /* The symbol location baton types used by the DWARF-2 reader (i.e.
    SYMBOL_LOCATION_BATON for a LOC_COMPUTED symbol).  "struct
    dwarf2_locexpr_baton" is for a symbol with a single location
@@ -39,28 +50,22 @@ struct dwarf2_locexpr_baton
   /* Length of the location expression.  */
   unsigned long size;
 
-  /* The objfile containing the symbol whose location we're computing.  */
-  struct objfile *objfile;
+  /* The compilation unit containing the symbol whose location
+     we're computing.  */
+  struct dwarf2_cu *cu;
 };
 
 struct dwarf2_loclist_baton
 {
-  /* The initial base address for the location list, based on the compilation
-     unit.  */
-  CORE_ADDR base_address;
-
   /* Pointer to the start of the location list.  */
   gdb_byte *data;
 
   /* Length of the location list.  */
   unsigned long size;
 
-  /* The objfile containing the symbol whose location we're computing.  */
-  /* Used (only???) by thread local variables.  The objfile in which
-     this symbol is defined.  To find a thread-local variable (e.g., a
-     variable declared with the `__thread' storage class), we may need
-     to know which object file it's in.  */
-  struct objfile *objfile;
+  /* The compilation unit containing the symbol whose location
+     we're computing.  */
+  struct dwarf2_cu *cu;
 };
 
 extern const struct symbol_ops dwarf2_locexpr_funcs;
diff -urNp gdb-orig/gdb/dwarf2read.c gdb-head/gdb/dwarf2read.c
--- gdb-orig/gdb/dwarf2read.c	2008-01-14 15:01:05.000000000 +0100
+++ gdb-head/gdb/dwarf2read.c	2008-01-14 16:07:52.000000000 +0100
@@ -9804,13 +9804,6 @@ static void
 dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
 			     struct dwarf2_cu *cu)
 {
-  struct objfile *objfile = cu->objfile;
-
-  /* Save the master objfile, so that we can report and look up the
-     correct file containing this variable.  */
-  if (objfile->separate_debug_objfile_backlink)
-    objfile = objfile->separate_debug_objfile_backlink;
-
   if (attr_form_is_section_offset (attr)
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
@@ -9821,13 +9814,12 @@ dwarf2_symbol_mark_computed (struct attr
 
       baton = obstack_alloc (&cu->objfile->objfile_obstack,
 			     sizeof (struct dwarf2_loclist_baton));
-      baton->objfile = objfile;
+      baton->cu = cu;
 
       /* We don't know how long the location list is, but make sure we
 	 don't run off the edge of the section.  */
       baton->size = dwarf2_per_objfile->loc_size - DW_UNSND (attr);
       baton->data = dwarf2_per_objfile->loc_buffer + DW_UNSND (attr);
-      baton->base_address = cu->header.base_address;
       if (cu->header.base_known == 0)
 	complaint (&symfile_complaints,
 		   _("Location list used without specifying the CU base address."));
@@ -9841,7 +9833,7 @@ dwarf2_symbol_mark_computed (struct attr
 
       baton = obstack_alloc (&cu->objfile->objfile_obstack,
 			     sizeof (struct dwarf2_locexpr_baton));
-      baton->objfile = objfile;
+      baton->cu = cu;
 
       if (attr_form_is_block (attr))
 	{
@@ -9866,6 +9858,37 @@ dwarf2_symbol_mark_computed (struct attr
     }
 }
 
+/* Return the OBJFILE associated with the compilation unit CU.  */
+
+struct objfile *
+dwarf2_cu_objfile (struct dwarf2_cu *cu)
+{
+  struct objfile *objfile = cu->objfile;
+
+  /* Return the master objfile, so that we can report and look up the
+     correct file containing this variable.  */
+  if (objfile->separate_debug_objfile_backlink)
+    objfile = objfile->separate_debug_objfile_backlink;
+
+  return objfile;
+}
+
+/* Return the address size given in the compilation unit header for CU.  */
+
+CORE_ADDR
+dwarf2_cu_addr_size (struct dwarf2_cu *cu)
+{
+  return cu->header.addr_size;
+}
+
+/* Return the base address given in the compilation unit header for CU.  */
+
+CORE_ADDR
+dwarf2_cu_base_address (struct dwarf2_cu *cu)
+{
+  return cu->header.base_address;
+}
+
 /* Locate the compilation unit from CU's objfile which contains the
    DIE at OFFSET.  Raises an error on failure.  */
 


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Make DWARF-2 "address size" explicit

[Ulrich Weigand]

I wrote:
> Apart from that, the behaviour should be identical, with one
> exception: what address size to use for CFI.  Your patch uses
>   size_of_encoded_value (DW_EH_PE_absptr)
> which basically boils down to:
>   gdbarch_ptr_bit (current_gdbarch) / TARGET_CHAR_BIT
> 
> This would be an effective change in behaviour to what we have
> now, which is:
>   gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT
> 
> My patch uses in effect
>   gdbarch_addr_bit (get_frame_arch (frame)) / TARGET_CHAR_BIT
> which does not change behaviour.
> 
> However, it might well be that the original code is simply wrong
> and we *should* be using ...ptr_bit instead of ...addr_bit.
> What do you think?

The notion of "target address size" is relevant in several places of Dwarf
Call Frame Information: target addresses are used explicitly in the 
"initial location" and "address range" fields of the FDE header, as well
as as immediate operand of the DW_CFA_set_loc call frame instruction.

In addition, the DW_CFA_def_cfa_expression, DW_CFA_expression and
DW_CFA_val_expression call frame instructions refer to DWARF expressions
as operand, which implicitly depend on the target address size: first,
the whole process of expression evaluation implicitly depends on the
target address size in that each element of the stack used is considered
to be of target address size (this affects e.g. rounding/truncation of
arithmetic operations).  In addition, the DW_OP_addr operation explicitly
takes an immediate target address as operand, and the DW_OP_deref and
DW_OP_xderef operations implicitly use the target address size.


I've now looked both at generation of CFI by GCC (dwarf2out.c) and
use of CFI by the GCC exception handling runtime (unwind-dw.c) to try
to determine what GCC uses and/or expects as "target address size"
in all of those cases.  Note that the CFI provided in the (standard)
.debug_frame section and the CFI in the (GCC / C++ ABI extension)
.eh_frame section *differ* in critical aspects here.

For the explicit uses of target addresses in FDE header fields and
the DW_CFA_set_loc instruction the situation is pretty clear.  For
.debug_frame, GCC uses its notion of "DWARF2_ADDR_SIZE" to define
the target address size.  This is a value that defaults to the size
of a "void *" on the target, but can be overridden by the platform
back-end.  This currently appears to be done by xstormy16, m68hc11,
m32c, and avr (which use a 4-byte DWARF address size even while
using 2-byte pointers), as well as certain ABI modes of mips64
(where the DWARF address size is defined by -mabi= while the pointer
size is defined by -mlong32 vs. -mlong64).  Note that this value is
used throughout the .debug_ sections, and could be retrieved e.g.
from the .debug_info address size header field.

For .eh_frame, GCC uses a completely different way to represent
target addresses: the "FDE encoding" field of the CIE augmentation
record defined which of a variety of encoding methods is used to
encode the FDE header fields as well as the DW_CFA_set_loc operand.
Most of these encodings explicitly specify a size; those that rely
on an implicit address size (DW_EH_PE_aligned, DW_EH_PE_absptr and
DW_EH_PE_indirect) always employ the size of a "void *" (*not* the
"DWARF2 address size as used in .debug_frame!).  As should be 
expected, the GCC code generator and the GCC exception runtime
are in complete agreement on this.  (This applies to .eh_frame only,
there is no *user* of .debug_frame anywhere in GCC.)

For the address sizes used in DWARF *expressions*, the picture gets
a lot less clear.  (Un?)fortunately, GCC currently generates only
a small subset of DWARF expressions as part of call frame information.
It never generates DW_CFA_expression or DW_CFA_val_expression, and
while it does generate DW_CFA_def_cfa_expression instructions, they
only contain a subset of all possible expressions.  In particular,
they never contain DW_OP_addr; GCC simply assumes arithmetic operations
never wrap so the exact size used for the value stack does not matter;
and DW_OP_deref is only generated for some platforms (where apparently
the address size issue doesn't really come up).  On the other hand,
in some places GCC seems to assume an address size as specified by
DWARF2_ADDR_SIZE (both for .eh_frame and .dwarf_frame -- this certainly
appears to be broken ...).

On the user side, the exception handling infrastructure assumes
the size of "void *" for DW_OP_addr and DW_OP_deref, but uses
"_Unwind_Word" (which represents the size of a target *register*,
not necessarily a target pointer) as size of the value stack
elements.  (Again this applies to .eh_frame only, there is no
user of .debug_frame in GCC.)


When comparing all this with what GDB currently does, there is
one obvious error: GDB does not take the FDE encoding into
account *at all* when accessing the operand of DW_CFA_set_loc
in the .eh_frame section.  It looks like this was already noticed
by Dan some time ago, but the associated patch:

http://www.cygwin.com/ml/gdb-patches/2006-10/msg00063.html

apparently was never applied.  Dan, are you still planning on
applying this patch?


Apart from that, it would appear that the most logical size to
use for target addresses in DWARF expression evaluation would
be the target "void *" size for .eh_frame FDEs, and the value
of the associated compilation unit's .debug_info address size
header field value for .debug_frame FDEs (however, I'm not sure
how to best determine that).


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Make DWARF-2 "address size" explicit

[Daniel Jacobowitz]

On Mon, Jan 28, 2008 at 09:52:32PM +0100, Ulrich Weigand wrote:
> When comparing all this with what GDB currently does, there is
> one obvious error: GDB does not take the FDE encoding into
> account *at all* when accessing the operand of DW_CFA_set_loc
> in the .eh_frame section.  It looks like this was already noticed
> by Dan some time ago, but the associated patch:
> 
> http://www.cygwin.com/ml/gdb-patches/2006-10/msg00063.html
> 
> apparently was never applied.  Dan, are you still planning on
> applying this patch?

If you could look over that patch and tell me if it looks right, I'll
apply it.  I had no way to test it with valid debug information.

> Apart from that, it would appear that the most logical size to
> use for target addresses in DWARF expression evaluation would
> be the target "void *" size for .eh_frame FDEs, and the value
> of the associated compilation unit's .debug_info address size
> header field value for .debug_frame FDEs (however, I'm not sure
> how to best determine that).

DJ Delorie ran into this same mess on the GCC list a few days ago and
Ian had the helpful suggestion to just avoid the bits that depend on
the ambiguous "address size".  If enough people do that, maybe it
won't matter what we pick... I think your choices sound correct.  It's
hard for .debug_info, though, as there is no direct correlation or
dependency between the sections.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Make DWARF-2 "address size" explicit

[Ulrich Weigand]

Dan Jacobowitz wrote:
> On Mon, Jan 28, 2008 at 09:52:32PM +0100, Ulrich Weigand wrote:
> > When comparing all this with what GDB currently does, there is
> > one obvious error: GDB does not take the FDE encoding into
> > account *at all* when accessing the operand of DW_CFA_set_loc
> > in the .eh_frame section.  It looks like this was already noticed
> > by Dan some time ago, but the associated patch:
> > 
> > http://www.cygwin.com/ml/gdb-patches/2006-10/msg00063.html
> > 
> > apparently was never applied.  Dan, are you still planning on
> > applying this patch?
> 
> If you could look over that patch and tell me if it looks right, I'll
> apply it.  I had no way to test it with valid debug information.

Your patch certainly looks right to me.  I'm wondering about this
comment, however:

+	      /* Always apply the objfile offset.  We can do this because
+		 if the code used DW_EH_PE_absptr, it probably wasn't
+		 relocatable, so the offset should be zero.  */
+	      fs->pc += ANOFFSET (unit->objfile->section_offsets,
+				  SECT_OFF_TEXT (unit->objfile));

I agree that we should always apply the objfile offset.  In fact,
even in the case of a DW_EH_PE_absptr in a relocatable object,
it *still* should be right to apply the offset: in this case,
there will be a relocation on the .eh_frame section that gets
applied when loading it into memory.  So if we'd read the value
from the in-memory copy, it would be wrong to add any offset.
However, we actually read the section contents via the
symfile_relocate_debug_section routine, which -while it does
apply relocations- assumes every section is located at its 
default offset.  So to get actual addresses we still need to
apply the section offset, right?

Apart from that, just a cosmetic comment:  I was wondering
why you didn't simply add a back-link from struct dwarf2_cie
to its containing struct comp_unit.  Then you'd automatically
have the CU available whereever you already have a CIE or FDE
pointer, without having to add "unit" arguments to all those
functions ...

> > Apart from that, it would appear that the most logical size to
> > use for target addresses in DWARF expression evaluation would
> > be the target "void *" size for .eh_frame FDEs, and the value
> > of the associated compilation unit's .debug_info address size
> > header field value for .debug_frame FDEs (however, I'm not sure
> > how to best determine that).
> 
> DJ Delorie ran into this same mess on the GCC list a few days ago and
> Ian had the helpful suggestion to just avoid the bits that depend on
> the ambiguous "address size".  If enough people do that, maybe it
> won't matter what we pick... I think your choices sound correct.  It's
> hard for .debug_info, though, as there is no direct correlation or
> dependency between the sections.

Yes, unfortunately you cannot look up the CU that corresponds to the
address range covered by the FDE, because to read that address range
you already need to know the address size :-/

You could make a pass over all .debug_info CU headers in the whole
objfile, and check whether they all agree on their address size
value -- I think this should just about always be true, as even on
those targets where GCC supports multiple address sizes (i.e. mips),
they can only be changed via ABI-breaking options, so those should
never be linked together into a single objfile.  If they all *do*
agree, then we have a number we can use ...  Does this make sense?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Make DWARF-2 "address size" explicit

[Jim Blandy]

On Jan 14, 2008 7:54 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Apart from that, the behaviour should be identical, with one
> exception: what address size to use for CFI.  Your patch uses
>   size_of_encoded_value (DW_EH_PE_absptr)
> which basically boils down to:
>   gdbarch_ptr_bit (current_gdbarch) / TARGET_CHAR_BIT
>
> This would be an effective change in behaviour to what we have
> now, which is:
>   gdbarch_addr_bit (current_gdbarch) / TARGET_CHAR_BIT
>
> My patch uses in effect
>   gdbarch_addr_bit (get_frame_arch (frame)) / TARGET_CHAR_BIT
> which does not change behaviour.
>
> However, it might well be that the original code is simply wrong
> and we *should* be using ...ptr_bit instead of ...addr_bit.
> What do you think?

(Sorry --- I thought I had replied to this.)

I don't think any change my patch made to the value used there was
intentional.  Whatever you have determined is the best value to use
there is more likely to be right than whatever is in my patch.  I
don't think this difference is a problem.