[PATCH] Make SIZE element for dwarf_block as size_t

[PATCH] Make SIZE element for dwarf_block as size_t

[Siddhesh Poyarekar]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

Hi,

dwarf_block.SIZE should be size_t to accept larger data blocks in case
of DW_FORM_block, since the size of the length of a DW_FORM_block is
not specified and can be anything encoded in a uleb128. The
dwarf2_loclist_baton and dwarf2_locexpr_baton SIZE members also need to
be made size_t just to eliminate splint warnings.

I have run the testsuite on x86_64 to ensure that there are no
regressions. This was part of the bitpos-expand[1] change that I am
currently reviewing and I realized that this could go in as a
separate independent change. Does this look OK to commit?

Thanks,
Siddhesh

gdb/ChangeLog:

2012-07-22  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* dwarf2loc.h (struct dwarf2_locexpr_baton): Make SIZE as
	size_t.
	(struct dwarf2_loclist_baton): Likewise.
	* dwarf2read.c (struct dwarf_block): Likewise.
	(dump_die_shallow): Use pulongest to print dwarf_block.size.

[1] http://sourceware.org/ml/gdb-patches/2012-06/msg00851.html

[-- Attachment #2: dwblock-size.patch --]
[-- Type: text/x-patch, Size: 1652 bytes --]

diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index e9d06a3..0f2af3c 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -97,7 +97,7 @@ struct dwarf2_locexpr_baton
 
   /* Length of the location expression.  For optimized out expressions it is
      zero.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
@@ -114,7 +114,7 @@ struct dwarf2_loclist_baton
   const gdb_byte *data;
 
   /* Length of the location list.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 187d1e8..6f671c7 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -887,7 +887,7 @@ struct die_info
 /* Blocks are a bunch of untyped bytes.  */
 struct dwarf_block
   {
-    unsigned int size;
+    size_t size;
 
     /* Valid only if SIZE is not zero.  */
     gdb_byte *data;
@@ -15074,12 +15074,12 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_block4:
 	case DW_FORM_block:
 	case DW_FORM_block1:
-	  fprintf_unfiltered (f, "block: size %d",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "block: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_exprloc:
-	  fprintf_unfiltered (f, "expression: size %u",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "expression: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_ref_addr:
 	  fprintf_unfiltered (f, "ref address: ");

Re: [PATCH] Make SIZE element for dwarf_block as size_t

[Siddhesh Poyarekar]

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

On Sun, 22 Jul 2012 13:16:58 +0530, Siddhesh wrote:
> 2012-07-22  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* dwarf2loc.h (struct dwarf2_locexpr_baton): Make SIZE as
> 	size_t.
> 	(struct dwarf2_loclist_baton): Likewise.
> 	* dwarf2read.c (struct dwarf_block): Likewise.
> 	(dump_die_shallow): Use pulongest to print dwarf_block.size.
> 
> [1] http://sourceware.org/ml/gdb-patches/2012-06/msg00851.html

A small adjustment to this patch to adjust for the expansion of
dwarf_block.size -- a couple of local variables in decode_locdesc
needed expansion as well.

Regards,
Siddhesh

 2012-07-22  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
 	* dwarf2loc.h (struct dwarf2_locexpr_baton): Make SIZE as
 	size_t.
 	(struct dwarf2_loclist_baton): Likewise.
 	* dwarf2read.c (struct dwarf_block): Likewise.
 	(dump_die_shallow): Use pulongest to print dwarf_block.size.
	(decode_locdesc): Expand SIZE and I to size_t.

[-- Attachment #2: dwblock-size.patch --]
[-- Type: text/x-patch, Size: 1948 bytes --]

diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index e9d06a3..0f2af3c 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -97,7 +97,7 @@ struct dwarf2_locexpr_baton
 
   /* Length of the location expression.  For optimized out expressions it is
      zero.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
@@ -114,7 +114,7 @@ struct dwarf2_loclist_baton
   const gdb_byte *data;
 
   /* Length of the location list.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 187d1e8..1d499ce 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -887,7 +887,7 @@ struct die_info
 /* Blocks are a bunch of untyped bytes.  */
 struct dwarf_block
   {
-    unsigned int size;
+    size_t size;
 
     /* Valid only if SIZE is not zero.  */
     gdb_byte *data;
@@ -15074,12 +15074,12 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_block4:
 	case DW_FORM_block:
 	case DW_FORM_block1:
-	  fprintf_unfiltered (f, "block: size %d",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "block: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_exprloc:
-	  fprintf_unfiltered (f, "expression: size %u",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "expression: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_ref_addr:
 	  fprintf_unfiltered (f, "ref address: ");
@@ -15616,8 +15616,8 @@ static CORE_ADDR
 decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->objfile;
-  int i;
-  int size = blk->size;
+  size_t i;
+  size_t size = blk->size;
   gdb_byte *data = blk->data;
   CORE_ADDR stack[64];
   int stacki;

Re: [PATCH] Make SIZE element for dwarf_block as size_t

[Tom Tromey]

>>>>> "Siddhesh" == Siddhesh Poyarekar <siddhesh@redhat.com> writes:

Siddhesh> A small adjustment to this patch to adjust for the expansion of
Siddhesh> dwarf_block.size -- a couple of local variables in decode_locdesc
Siddhesh> needed expansion as well.

What about dwarf2_evaluate_loc_desc_full?
It has 'unsigned short size' as an argument but is called from
indirect_pieced_value:

  struct dwarf2_locexpr_baton baton;
[...]
  return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
					baton.data, baton.size, baton.per_cu,
					piece->v.ptr.offset + byte_offset);


It would have been useful to me if you had written a bit about how you
wrote this patch and verified it.  As it is, with no information on
that, I went grepping through the source to see if the patch was
complete.

Tom

Re: [PATCH] Make SIZE element for dwarf_block as size_t

[Siddhesh Poyarekar]

On Mon, 23 Jul 2012 08:41:27 -0600, Tom wrote:
> What about dwarf2_evaluate_loc_desc_full?
> It has 'unsigned short size' as an argument but is called from
> indirect_pieced_value:
> 
>   struct dwarf2_locexpr_baton baton;
> [...]
>   return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type),
> frame, baton.data, baton.size, baton.per_cu,
> 					piece->v.ptr.offset +
> byte_offset);

Ah yes, I missed that, thanks. I'll look through once more.

> It would have been useful to me if you had written a bit about how you
> wrote this patch and verified it.  As it is, with no information on
> that, I went grepping through the source to see if the patch was
> complete.

The fix was partly ripped out of the bitpos-expand patch[1] that I am
working on, in am attempt to make that patch a little smaller (not that
I've succeeded much at all). My current method for verification is just
to ensure that there are no regressions in the testsuite and grepping
through the code to try and ensure that I haven't missed anything
(which I did this time). I don't have a reproducer that demonstrates any
breakage due to this, but I can try to come up with one if it is
necessary.

Regards,
Siddhesh

[1] http://sourceware.org/ml/gdb-patches/2012-06/msg00851.html

Re: [PATCH] Make SIZE element for dwarf_block as size_t

[Tom Tromey]

>>>>> "Siddhesh" == Siddhesh Poyarekar <siddhesh@redhat.com> writes:

Siddhesh> My current method for verification is just to ensure that
Siddhesh> there are no regressions in the testsuite and grepping through
Siddhesh> the code to try and ensure that I haven't missed anything
Siddhesh> (which I did this time).

Thanks.

Siddhesh> I don't have a reproducer that demonstrates any breakage due
Siddhesh> to this, but I can try to come up with one if it is necessary.

I suspect it would be more trouble than it is worth.

Tom

[PATCH][v2] Make SIZE element for dwarf_block as size_t

[Siddhesh Poyarekar]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

Hi,

This is version 2 of the patch I posted in the following thread:

http://sourceware.org/ml/gdb-patches/2012-07/msg00422.html

dwarf_block.SIZE should be size_t to accept larger data blocks in case
of DW_FORM_block, since the size of the length of a DW_FORM_block is
not specified and can be anything encoded in a uleb128. The
dwarf2_loclist_baton and dwarf2_locexpr_baton SIZE members also need to
be made size_t just to eliminate splint warnings.

I haven't been able to figure out a test case to reproduce this
problem, so I have stuck to ensuring that it does not cause a
regression in the testsuite. I have also done another pass through the
code to make sure that there are no other places that need fixing as a
result of this.

Regards,
Siddhesh

gdb/ChangeLog:

2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Expand parameter
	SIZE to size_t.
	(dwarf2_evaluate_loc_desc): Likewise.
	(dwarf2_loc_desc_needs_frame): Likewise.
	(locexpr_describe_location_1): Likewise.
	* dwarf2loc.h (struct dwarf2_locexpr_baton): Make SIZE as
	size_t.
	(struct dwarf2_loclist_baton): Likewise.
	* dwarf2read.c (struct dwarf_block): Likewise.
	(dump_die_shallow): Use pulongest to print dwarf_block.size.
	(decode_locdesc): Expand SIZE and I to size_t.


[-- Attachment #2: locexpr-baton.patch --]
[-- Type: text/x-patch, Size: 4425 bytes --]

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 9b87dad..3eadc55 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -54,8 +54,8 @@ static const struct dwarf_expr_context_funcs dwarf_expr_ctx_funcs;
 static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
 						    struct frame_info *frame,
 						    const gdb_byte *data,
-						    unsigned short size,
-					      struct dwarf2_per_cu_data *per_cu,
+						    size_t size,
+						    struct dwarf2_per_cu_data *per_cu,
 						    LONGEST byte_offset);
 
 /* Until these have formal names, we define these here.
@@ -2112,7 +2112,7 @@ static const struct dwarf_expr_context_funcs dwarf_expr_ctx_funcs =
 
 static struct value *
 dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
-			       const gdb_byte *data, unsigned short size,
+			       const gdb_byte *data, size_t size,
 			       struct dwarf2_per_cu_data *per_cu,
 			       LONGEST byte_offset)
 {
@@ -2313,7 +2313,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 
 struct value *
 dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
-			  const gdb_byte *data, unsigned short size,
+			  const gdb_byte *data, size_t size,
 			  struct dwarf2_per_cu_data *per_cu)
 {
   return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
@@ -2434,7 +2434,7 @@ static const struct dwarf_expr_context_funcs needs_frame_ctx_funcs =
    requires a frame to evaluate.  */
 
 static int
-dwarf2_loc_desc_needs_frame (const gdb_byte *data, unsigned short size,
+dwarf2_loc_desc_needs_frame (const gdb_byte *data, size_t size,
 			     struct dwarf2_per_cu_data *per_cu)
 {
   struct needs_frame_baton baton;
@@ -3828,7 +3828,7 @@ disassemble_dwarf_expression (struct ui_file *stream,
 static void
 locexpr_describe_location_1 (struct symbol *symbol, CORE_ADDR addr,
 			     struct ui_file *stream,
-			     const gdb_byte *data, int size,
+			     const gdb_byte *data, size_t size,
 			     struct objfile *objfile, unsigned int addr_size,
 			     int offset_size, struct dwarf2_per_cu_data *per_cu)
 {
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index e9d06a3..326838f 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -77,7 +77,7 @@ struct type *dwarf2_get_die_type (cu_offset die_offset,
 struct value *dwarf2_evaluate_loc_desc (struct type *type,
 					struct frame_info *frame,
 					const gdb_byte *data,
-					unsigned short size,
+					size_t size,
 					struct dwarf2_per_cu_data *per_cu);
 
 CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu,
@@ -97,7 +97,7 @@ struct dwarf2_locexpr_baton
 
   /* Length of the location expression.  For optimized out expressions it is
      zero.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
@@ -114,7 +114,7 @@ struct dwarf2_loclist_baton
   const gdb_byte *data;
 
   /* Length of the location list.  */
-  unsigned long size;
+  size_t size;
 
   /* The compilation unit containing the symbol whose location
      we're computing.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4b20098..99fef46 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -990,7 +990,7 @@ struct die_info
 /* Blocks are a bunch of untyped bytes.  */
 struct dwarf_block
   {
-    unsigned int size;
+    size_t size;
 
     /* Valid only if SIZE is not zero.  */
     gdb_byte *data;
@@ -16197,12 +16197,12 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_block4:
 	case DW_FORM_block:
 	case DW_FORM_block1:
-	  fprintf_unfiltered (f, "block: size %d",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "block: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_exprloc:
-	  fprintf_unfiltered (f, "expression: size %u",
-			      DW_BLOCK (&die->attrs[i])->size);
+	  fprintf_unfiltered (f, "expression: size %s",
+			      pulongest (DW_BLOCK (&die->attrs[i])->size));
 	  break;
 	case DW_FORM_ref_addr:
 	  fprintf_unfiltered (f, "ref address: ");
@@ -16746,8 +16746,8 @@ static CORE_ADDR
 decode_locdesc (struct dwarf_block *blk, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->objfile;
-  int i;
-  int size = blk->size;
+  size_t i;
+  size_t size = blk->size;
   gdb_byte *data = blk->data;
   CORE_ADDR stack[64];
   int stacki;

Re: [PATCH][v2] Make SIZE element for dwarf_block as size_t

[Tom Tromey]

>>>>> "Siddhesh" == Siddhesh Poyarekar <siddhesh@redhat.com> writes:

Siddhesh> I haven't been able to figure out a test case to reproduce this
Siddhesh> problem, so I have stuck to ensuring that it does not cause a
Siddhesh> regression in the testsuite.

I think that is good enough.

Siddhesh> 2012-07-25  Siddhesh Poyarekar  <siddhesh@redhat.com>
Siddhesh> 	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Expand parameter
Siddhesh> 	SIZE to size_t.
Siddhesh> 	(dwarf2_evaluate_loc_desc): Likewise.
Siddhesh> 	(dwarf2_loc_desc_needs_frame): Likewise.
Siddhesh> 	(locexpr_describe_location_1): Likewise.
Siddhesh> 	* dwarf2loc.h (struct dwarf2_locexpr_baton): Make SIZE as
Siddhesh> 	size_t.
Siddhesh> 	(struct dwarf2_loclist_baton): Likewise.
Siddhesh> 	* dwarf2read.c (struct dwarf_block): Likewise.
Siddhesh> 	(dump_die_shallow): Use pulongest to print dwarf_block.size.
Siddhesh> 	(decode_locdesc): Expand SIZE and I to size_t.

Ok.

Tom