RFC: fix bug when debugging prelink'd library

RFC: fix bug when debugging prelink'd library

[Tom Tromey]

I plan to check this in, but I wanted to ask for comments first, because
I think at least Jan disagrees with this direction.  Once the discussion
is over I plan to commit this, or its replacement, to both the trunk and
the 7.2 branch.

When debugging a program linked with -lpthread I found that I couldn't
print a variable in one of the pthread functions.  However, I have full
debuginfo installed, and the debuginfo is correct for this particular
variable.

Jan pointed out a Fedora patch that showed the problem --
dwarf2_per_cu_objfile was returning the master objfile, not the
debuginfo objfile.  But, at least in the prelink case, the master
objfile does not have the same section offsets as the debuginfo objfile.

This fixes the problem by changing dwarf2_per_cu_objfile to return the
debuginfo objfile.  This appears to be correct for all callers except
one, which I also updated.

Built and regtested on x86-64 (compile farm).
I also tried it locally on my prelink test case.

Tom

2010-07-12  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_per_cu_objfile): Return the objfile, not
	the master objfile.
	* dwarf2loc.c (dwarf_expr_tls_address): Use the master objfile.

Index: dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.94
diff -u -r1.94 dwarf2loc.c
--- dwarf2loc.c	7 Jul 2010 17:26:38 -0000	1.94
+++ dwarf2loc.c	12 Jul 2010 19:25:11 -0000
@@ -234,6 +234,10 @@
   struct dwarf_expr_baton *debaton = (struct dwarf_expr_baton *) baton;
   struct objfile *objfile = dwarf2_per_cu_objfile (debaton->per_cu);
 
+  /* Use the master objfile.  */
+  if (objfile->separate_debug_objfile_backlink)
+    objfile = objfile->separate_debug_objfile_backlink;
+
   return target_translate_tls_address (objfile, offset);
 }
 
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.411
diff -u -r1.411 dwarf2read.c
--- dwarf2read.c	12 Jul 2010 17:07:11 -0000	1.411
+++ dwarf2read.c	12 Jul 2010 19:25:12 -0000
@@ -12054,14 +12054,7 @@
 struct objfile *
 dwarf2_per_cu_objfile (struct dwarf2_per_cu_data *per_cu)
 {
-  struct objfile *objfile = per_cu->psymtab->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 per_cu->psymtab->objfile;
 }
 
 /* Return the address size given in the compilation unit header for CU.  */

Re: RFC: fix bug when debugging prelink'd library

[Jan Kratochvil]

On Mon, 12 Jul 2010 21:34:20 +0200, Tom Tromey wrote:
> This fixes the problem by changing dwarf2_per_cu_objfile to return the
> debuginfo objfile.  This appears to be correct for all callers except
> one, which I also updated.

As I find the API difficult to figure out which variant of `struct objfile *'
there is I tried to unify it - choosing the variant (A) described at:
	[patch 12/15] PIE: Support loading PIEs from core files
	http://sourceware.org/ml/gdb-patches/2009-11/msg00179.html

The patch below follows none of the (A)/(B)/(C) rules.  In such case I miss at
least some fat comment at dwarf2_per_cu_objfile stating it returns non-primary
objfile variant.

dwarf2_per_cu_objfile result is in most cases used only for get_objfile_arch
(should not matter) and
	CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
so there could be rather function dwarf2_per_cu_base_offset.


Regards,
Jan

Re: RFC: fix bug when debugging prelink'd library

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Tom> This fixes the problem by changing dwarf2_per_cu_objfile to return the
Tom> debuginfo objfile.  This appears to be correct for all callers except
Tom> one, which I also updated.

Jan> As I find the API difficult to figure out which variant of `struct
Jan> objfile *' there is I tried to unify it - choosing the variant (A)
Jan> described at:
Jan> 	[patch 12/15] PIE: Support loading PIEs from core files
Jan> 	http://sourceware.org/ml/gdb-patches/2009-11/msg00179.html

I agree that one of these approaches is good for generic code in gdb.
(Actually -- I would much prefer a different design, where there is no
ambiguity possible.  But that seems like a big refactoring without a big
user-visible benefit.)

Jan> In such case I miss at least some fat comment at
Jan> dwarf2_per_cu_objfile stating it returns non-primary objfile
Jan> variant.

I will do that.

Jan> dwarf2_per_cu_objfile result is in most cases used only for
Jan> get_objfile_arch (should not matter) and
Jan> 	CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
Jan> so there could be rather function dwarf2_per_cu_base_offset.

Actually, I think I will do this.  It does mean we need 3 new accessors,
which is ugly.  OTOH, I found that we use the objfile's name in
locexpr_describe_location_piece, and there I think we also want the
master objfile.

Tom

Re: RFC: fix bug when debugging prelink'd library

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> It does mean we need 3 new accessors, which is ugly.  OTOH, I found
Tom> that we use the objfile's name in locexpr_describe_location_piece,
Tom> and there I think we also want the master objfile.

We only need 1 new accessor -- using the master objfile is ok for the
other uses.

So, this is what I am going to commit.

Built & regtested on x86-64 (compile farm).
I also tried it on my test case.

Tom

2010-07-12  Tom Tromey  <tromey@redhat.com>

	* dwarf2loc.h (dwarf2_per_cu_text_offset): Declare.
	* dwarf2loc.c (find_location_expression): Use
	dwarf2_per_cu_text_offset.
	(dwarf2_evaluate_loc_desc): Likewise.
	(dwarf2_loc_desc_needs_frame): Likewise.
	(compile_dwarf_to_ax): Likewise.
	(loclist_describe_location): Likewise.
	* dwarf2read.c (dwarf2_per_cu_text_offset): New function.

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 4106b7f..2a8e557 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -70,8 +70,7 @@ find_location_expression (struct dwarf2_loclist_baton *baton,
   int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd);
   CORE_ADDR base_mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
   /* Adjust base_address for relocatable objects.  */
-  CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets,
-				    SECT_OFF_TEXT (objfile));
+  CORE_ADDR base_offset = dwarf2_per_cu_text_offset (baton->per_cu);
   CORE_ADDR base_address = baton->base_address + base_offset;
 
   loc_ptr = baton->data;
@@ -909,7 +908,7 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
 
   ctx->gdbarch = get_objfile_arch (objfile);
   ctx->addr_size = dwarf2_per_cu_addr_size (per_cu);
-  ctx->offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  ctx->offset = dwarf2_per_cu_text_offset (per_cu);
   ctx->baton = &baton;
   ctx->read_reg = dwarf_expr_read_reg;
   ctx->read_mem = dwarf_expr_read_mem;
@@ -1095,7 +1094,7 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, unsigned short size,
 
   ctx->gdbarch = get_objfile_arch (objfile);
   ctx->addr_size = dwarf2_per_cu_addr_size (per_cu);
-  ctx->offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  ctx->offset = dwarf2_per_cu_text_offset (per_cu);
   ctx->baton = &baton;
   ctx->read_reg = needs_frame_read_reg;
   ctx->read_mem = needs_frame_read_mem;
@@ -1304,12 +1303,7 @@ compile_dwarf_to_ax (struct agent_expr *expr, struct axs_value *loc,
 	     index, not an address.  We don't support things like
 	     branching between the address and the TLS op.  */
 	  if (op_ptr >= op_end || *op_ptr != DW_OP_GNU_push_tls_address)
-	    {
-	      struct objfile *objfile = dwarf2_per_cu_objfile (per_cu);
-
-	      uoffset += ANOFFSET (objfile->section_offsets,
-				   SECT_OFF_TEXT (objfile));
-	    }
+	    uoffset += dwarf2_per_cu_text_offset (per_cu);
 	  ax_const_l (expr, uoffset);
 	  break;
 
@@ -2511,8 +2505,7 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
   int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd);
   CORE_ADDR base_mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
   /* Adjust base_address for relocatable objects.  */
-  CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets,
-				    SECT_OFF_TEXT (objfile));
+  CORE_ADDR base_offset = dwarf2_per_cu_text_offset (dlbaton->per_cu);
   CORE_ADDR base_address = dlbaton->base_address + base_offset;
 
   loc_ptr = dlbaton->data;
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index 3ff595e..540e1d6 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -28,7 +28,9 @@ struct dwarf2_per_cu_data;
 /* 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.  */
+/* Return the OBJFILE associated with the compilation unit CU.  If CU
+   came from a separate debuginfo file, then the master objfile is
+   returned.  */
 struct objfile *dwarf2_per_cu_objfile (struct dwarf2_per_cu_data *cu);
 
 /* Return the address size given in the compilation unit header for CU.  */
@@ -37,6 +39,9 @@ CORE_ADDR dwarf2_per_cu_addr_size (struct dwarf2_per_cu_data *cu);
 /* Return the offset size given in the compilation unit header for CU.  */
 int dwarf2_per_cu_offset_size (struct dwarf2_per_cu_data *cu);
 
+/* Return the text offset for CU.  */
+CORE_ADDR dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *cu);
+
 struct dwarf2_locexpr_baton dwarf2_fetch_die_location_block
   (unsigned int offset, struct dwarf2_per_cu_data *per_cu);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e4ab034..653866c 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12106,6 +12106,16 @@ dwarf2_per_cu_offset_size (struct dwarf2_per_cu_data *per_cu)
     }
 }
 
+/* Return the text offset of the CU.  */
+
+CORE_ADDR
+dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)
+{
+  struct objfile *objfile = per_cu->psymtab->objfile;
+
+  return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+}
+
 /* Locate the .debug_info compilation unit from CU's objfile which contains
    the DIE at OFFSET.  Raises an error on failure.  */
 

Re: RFC: fix bug when debugging prelink'd library

[Jan Kratochvil]

On Mon, 12 Jul 2010 23:06:21 +0200, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
> 
> Tom> It does mean we need 3 new accessors, which is ugly.  OTOH, I found
> Tom> that we use the objfile's name in locexpr_describe_location_piece,
> Tom> and there I think we also want the master objfile.
> 
> We only need 1 new accessor -- using the master objfile is ok for the
> other uses.

Yes, I like the patch this way.


Thanks,
Jan



> --- a/gdb/dwarf2loc.h
> +++ b/gdb/dwarf2loc.h
> -/* Return the OBJFILE associated with the compilation unit CU.  */
> +/* Return the OBJFILE associated with the compilation unit CU.  If CU
> +   came from a separate debuginfo file, then the master objfile is
> +   returned.  */
>  struct objfile *dwarf2_per_cu_objfile (struct dwarf2_per_cu_data *cu);

The same comment needs to be updated at the function definition in
dwarf2read.c.


> +/* Return the text offset for CU.  */
> +CORE_ADDR dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *cu);
+
> +/* Return the text offset of the CU.  */

Suggesting to extend the comment:

This returned offset is specific to this CU's debug info objfile and it may
be different from the SEPARATE_DEBUG_OBJFILE_BACKLINK objfile offsets.


> +CORE_ADDR
> +dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)
> +{
> +  struct objfile *objfile = per_cu->psymtab->objfile;
> +
> +  return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
> +}

Re: RFC: fix bug when debugging prelink'd library

[Tom Tromey]

Jan> Yes, I like the patch this way.

Great.

Jan> The same comment needs to be updated at the function definition in
Jan> dwarf2read.c.
[...]
Jan> Suggesting to extend the comment:
[...]

Here is what I am checking in.

Tom

2010-07-13  Tom Tromey  <tromey@redhat.com>

	* dwarf2loc.h (dwarf2_per_cu_text_offset): Declare.
	* dwarf2loc.c (find_location_expression): Use
	dwarf2_per_cu_text_offset.
	(dwarf2_evaluate_loc_desc): Likewise.
	(dwarf2_loc_desc_needs_frame): Likewise.
	(compile_dwarf_to_ax): Likewise.
	(loclist_describe_location): Likewise.
	* dwarf2read.c (dwarf2_per_cu_text_offset): New function.
	(dwarf2_per_cu_objfile): Update comment.

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 4106b7f..2a8e557 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -70,8 +70,7 @@ find_location_expression (struct dwarf2_loclist_baton *baton,
   int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd);
   CORE_ADDR base_mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
   /* Adjust base_address for relocatable objects.  */
-  CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets,
-				    SECT_OFF_TEXT (objfile));
+  CORE_ADDR base_offset = dwarf2_per_cu_text_offset (baton->per_cu);
   CORE_ADDR base_address = baton->base_address + base_offset;
 
   loc_ptr = baton->data;
@@ -909,7 +908,7 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
 
   ctx->gdbarch = get_objfile_arch (objfile);
   ctx->addr_size = dwarf2_per_cu_addr_size (per_cu);
-  ctx->offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  ctx->offset = dwarf2_per_cu_text_offset (per_cu);
   ctx->baton = &baton;
   ctx->read_reg = dwarf_expr_read_reg;
   ctx->read_mem = dwarf_expr_read_mem;
@@ -1095,7 +1094,7 @@ dwarf2_loc_desc_needs_frame (const gdb_byte *data, unsigned short size,
 
   ctx->gdbarch = get_objfile_arch (objfile);
   ctx->addr_size = dwarf2_per_cu_addr_size (per_cu);
-  ctx->offset = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  ctx->offset = dwarf2_per_cu_text_offset (per_cu);
   ctx->baton = &baton;
   ctx->read_reg = needs_frame_read_reg;
   ctx->read_mem = needs_frame_read_mem;
@@ -1304,12 +1303,7 @@ compile_dwarf_to_ax (struct agent_expr *expr, struct axs_value *loc,
 	     index, not an address.  We don't support things like
 	     branching between the address and the TLS op.  */
 	  if (op_ptr >= op_end || *op_ptr != DW_OP_GNU_push_tls_address)
-	    {
-	      struct objfile *objfile = dwarf2_per_cu_objfile (per_cu);
-
-	      uoffset += ANOFFSET (objfile->section_offsets,
-				   SECT_OFF_TEXT (objfile));
-	    }
+	    uoffset += dwarf2_per_cu_text_offset (per_cu);
 	  ax_const_l (expr, uoffset);
 	  break;
 
@@ -2511,8 +2505,7 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
   int signed_addr_p = bfd_get_sign_extend_vma (objfile->obfd);
   CORE_ADDR base_mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
   /* Adjust base_address for relocatable objects.  */
-  CORE_ADDR base_offset = ANOFFSET (objfile->section_offsets,
-				    SECT_OFF_TEXT (objfile));
+  CORE_ADDR base_offset = dwarf2_per_cu_text_offset (dlbaton->per_cu);
   CORE_ADDR base_address = dlbaton->base_address + base_offset;
 
   loc_ptr = dlbaton->data;
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index 3ff595e..826bc45 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -28,7 +28,9 @@ struct dwarf2_per_cu_data;
 /* 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.  */
+/* Return the OBJFILE associated with the compilation unit CU.  If CU
+   came from a separate debuginfo file, then the master objfile is
+   returned.  */
 struct objfile *dwarf2_per_cu_objfile (struct dwarf2_per_cu_data *cu);
 
 /* Return the address size given in the compilation unit header for CU.  */
@@ -37,6 +39,12 @@ CORE_ADDR dwarf2_per_cu_addr_size (struct dwarf2_per_cu_data *cu);
 /* Return the offset size given in the compilation unit header for CU.  */
 int dwarf2_per_cu_offset_size (struct dwarf2_per_cu_data *cu);
 
+/* Return the text offset of the CU.  The returned offset comes from
+   this CU's objfile.  If this objfile came from a separate debuginfo
+   file, then the offset may be different from the corresponding
+   offset in the parent objfile.  */
+CORE_ADDR dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *cu);
+
 struct dwarf2_locexpr_baton dwarf2_fetch_die_location_block
   (unsigned int offset, struct dwarf2_per_cu_data *per_cu);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index e4ab034..16381de 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12047,7 +12047,9 @@ dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
     }
 }
 
-/* Return the OBJFILE associated with the compilation unit CU.  */
+/* Return the OBJFILE associated with the compilation unit CU.  If CU
+   came from a separate debuginfo file, then the master objfile is
+   returned.  */
 
 struct objfile *
 dwarf2_per_cu_objfile (struct dwarf2_per_cu_data *per_cu)
@@ -12106,6 +12108,19 @@ dwarf2_per_cu_offset_size (struct dwarf2_per_cu_data *per_cu)
     }
 }
 
+/* Return the text offset of the CU.  The returned offset comes from
+   this CU's objfile.  If this objfile came from a separate debuginfo
+   file, then the offset may be different from the corresponding
+   offset in the parent objfile.  */
+
+CORE_ADDR
+dwarf2_per_cu_text_offset (struct dwarf2_per_cu_data *per_cu)
+{
+  struct objfile *objfile = per_cu->psymtab->objfile;
+
+  return ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+}
+
 /* Locate the .debug_info compilation unit from CU's objfile which contains
    the DIE at OFFSET.  Raises an error on failure.  */