[patch] Make a function for block->objfile lookups

[patch] Make a function for block->objfile lookups

[Jan Kratochvil]

Hi,

the patch generalizes the `display' command code to use objfile and not
so_list matching; the modified code was created recently by:
	Re: Fix a crash when displaying variables from shared library.
	http://sourceware.org/ml/gdb-patches/2009-03/msg00090.html

This patch was made so that it can be reused from varobj.c where only objfile
(not so_list) is available.
	[patch] [4/5] Types reference counting [varobj-validation]
	http://sourceware.org/ml/gdb-patches/2009-04/threads.html#00203

I found it as a simplification, one may disagree.  I can use
matching_objfiles/block_objfile just for varobj.c.

No regressions on x86_64-unknown-linux-gnu.


Thanks,
Jan


gdb/
2009-04-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
	* block.c (block_objfile): New function.
	* block.h (block_objfile): New prototype.
	* objfiles.c (matching_objfiles): New function.
	* objfiles.h (matching_objfiles): New prototype.
	* printcmd.c: Remove include solib.h.
	(display_uses_solib_p): Rename to ...
	(display_uses_objfile): ... a new function name.  Change the SOLIB
	parameter to OBJFILE parameter.  Use now a matching_objfiles call.
	(clear_dangling_display_expressions): Update the caller.

--- gdb/block.c	3 Jan 2009 05:57:50 -0000	1.18
+++ gdb/block.c	22 Apr 2009 18:15:44 -0000
@@ -309,3 +309,20 @@ allocate_block (struct obstack *obstack)
 
   return bl;
 }
+
+/* Return OBJFILE in which BLOCK is located or NULL if we cannot find it for
+   whatever reason.  */
+
+struct objfile *block_objfile (const struct block *block)
+{
+  struct symbol *func;
+
+  if (block == NULL)
+    return NULL;
+
+  func = block_linkage_function (block);
+  if (func == NULL)
+    return NULL;
+
+  return SYMBOL_SYMTAB (func)->objfile;
+}
--- gdb/block.h	3 Jan 2009 05:57:50 -0000	1.19
+++ gdb/block.h	22 Apr 2009 18:15:44 -0000
@@ -164,4 +164,6 @@ extern const struct block *block_global_
 
 extern struct block *allocate_block (struct obstack *obstack);
 
+extern struct objfile *block_objfile (const struct block *block);
+
 #endif /* BLOCK_H */
--- gdb/objfiles.c	11 Mar 2009 20:26:02 -0000	1.82
+++ gdb/objfiles.c	22 Apr 2009 18:15:44 -0000
@@ -891,3 +891,21 @@ objfile_data (struct objfile *objfile, c
   gdb_assert (data->index < objfile->num_data);
   return objfile->data[data->index];
 }
+
+/* Return non-zero if A and B point to the same OBJFILE, ignoring any binary
+   vs. debuginfo variants of the pointers.  If either A or B is NULL return
+   zero as not a match.  */
+
+int
+matching_objfiles (struct objfile *a, struct objfile *b)
+{
+  if (a == NULL || b == NULL)
+    return 0;
+
+  if (a->separate_debug_objfile_backlink)
+    a = a->separate_debug_objfile_backlink;
+  if (b->separate_debug_objfile_backlink)
+    b = b->separate_debug_objfile_backlink;
+
+  return a == b;
+}
--- gdb/objfiles.h	15 Jan 2009 16:35:22 -0000	1.59
+++ gdb/objfiles.h	22 Apr 2009 18:15:45 -0000
@@ -508,6 +508,7 @@ extern void set_objfile_data (struct obj
 			      const struct objfile_data *data, void *value);
 extern void *objfile_data (struct objfile *objfile,
 			   const struct objfile_data *data);
+extern int matching_objfiles (struct objfile *a, struct objfile *b);
 \f
 
 /* Traverse all object files.  ALL_OBJFILES_SAFE works even if you delete
--- gdb/printcmd.c	25 Mar 2009 22:38:46 -0000	1.150
+++ gdb/printcmd.c	22 Apr 2009 18:15:45 -0000
@@ -46,7 +46,6 @@
 #include "exceptions.h"
 #include "observer.h"
 #include "solist.h"
-#include "solib.h"
 #include "parser-defs.h"
 #include "charset.h"
 
@@ -1760,19 +1759,17 @@ disable_display_command (char *args, int
       }
 }
 
-/* Return 1 if D uses SOLIB (and will become dangling when SOLIB
+/* Return 1 if D uses OBJFILE (and will become dangling when OBJFILE
    is unloaded), otherwise return 0.  */
 
 static int
-display_uses_solib_p (const struct display *d,
-		      const struct so_list *solib)
+display_uses_objfile (const struct display *d, struct objfile *objfile)
 {
   int endpos;
   struct expression *const exp = d->exp;
   const union exp_element *const elts = exp->elts;
 
-  if (d->block != NULL
-      && solib_contains_address_p (solib, d->block->startaddr))
+  if (matching_objfiles (block_objfile (d->block), objfile))
     return 1;
 
   for (endpos = exp->nelts; endpos > 0; )
@@ -1791,11 +1788,10 @@ display_uses_solib_p (const struct displ
 	  const struct obj_section *const section =
 	    SYMBOL_OBJ_SECTION (symbol);
 
-	  if (block != NULL
-	      && solib_contains_address_p (solib, block->startaddr))
+	  if (matching_objfiles (block_objfile (block), objfile))
 	    return 1;
 
-	  if (section && section->objfile == solib->objfile)
+	  if (section && section->objfile == objfile)
 	    return 1;
 	}
       endpos -= oplen;
@@ -1820,7 +1816,7 @@ clear_dangling_display_expressions (stru
 
   for (d = display_chain; d; d = d->next)
     {
-      if (d->exp && display_uses_solib_p (d, solib))
+      if (d->exp && display_uses_objfile (d, solib->objfile))
 	{
 	  xfree (d->exp);
 	  d->exp = NULL;

Re: [patch] Make a function for block->objfile lookups

[Tom Tromey]

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

Jan> 2009-04-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* block.c (block_objfile): New function.
Jan> 	* block.h (block_objfile): New prototype.
Jan> 	* objfiles.c (matching_objfiles): New function.
Jan> 	* objfiles.h (matching_objfiles): New prototype.
Jan> 	* printcmd.c: Remove include solib.h.
Jan> 	(display_uses_solib_p): Rename to ...
Jan> 	(display_uses_objfile): ... a new function name.  Change the SOLIB
Jan> 	parameter to OBJFILE parameter.  Use now a matching_objfiles call.
Jan> 	(clear_dangling_display_expressions): Update the caller.

Jan> +struct objfile *block_objfile (const struct block *block)

Newline after the "*".

Jan> +  struct symbol *func;
Jan> +
Jan> +  if (block == NULL)
Jan> +    return NULL;
Jan> +
Jan> +  func = block_linkage_function (block);
Jan> +  if (func == NULL)
Jan> +    return NULL;

Just to be sure I understand... in the loop in display_uses_solib_p,
there are two checks:

	  if (block != NULL
	      && solib_contains_address_p (solib, block->startaddr))
	    return 1;

	  if (section && section->objfile == solib->objfile)
	    return 1;

So I gather from this change that the first check is checking for
functions and the second one will handle variables coming from the
solib?  And that is why it is ok to replace the first change with
block_objfile?  My concern here is that the patch not affect the
semantics of the existing check; and I don't really know the block
stuff very well.

Jan> --- gdb/objfiles.h	15 Jan 2009 16:35:22 -0000	1.59
Jan> +++ gdb/objfiles.h	22 Apr 2009 18:15:45 -0000
Jan> @@ -508,6 +508,7 @@ extern void set_objfile_data (struct obj
Jan>  			      const struct objfile_data *data, void *value);
Jan>  extern void *objfile_data (struct objfile *objfile,
Jan>  			   const struct objfile_data *data);
Jan> +extern int matching_objfiles (struct objfile *a, struct objfile *b);

I think this declaration should not be in this section, which is all
related to the objfile_data stuff.  Instead just move it above the
comment introducing objfile_data.

Assuming my understanding of the semantics is correct, then this is ok
with the above fixlets.  Thanks.

Tom

Re: [patch] Make a function for block->objfile lookups

[Jan Kratochvil]

On Wed, 22 Apr 2009 21:17:12 +0200, Tom Tromey wrote:
> Just to be sure I understand... in the loop in display_uses_solib_p,
> there are two checks:
> 
> 	  if (block != NULL
> 	      && solib_contains_address_p (solib, block->startaddr))
> 	    return 1;
> 
> 	  if (section && section->objfile == solib->objfile)
> 	    return 1;
> 
> So I gather from this change that the first check is checking for
> functions and the second one will handle variables coming from the
> solib?

Yes.  This patch does not change anything wrt it.  Made a comment there now.

BTW I do not see expression->block to be set anywhere, only display->block is
AFAICS in charge by the current outer GDB (C) code.


> And that is why it is ok to replace the first change with block_objfile?  My
> concern here is that the patch not affect the semantics of the existing
> check; and I don't really know the block stuff very well.

-         if (block != NULL
-             && solib_contains_address_p (solib, block->startaddr))
+         if (matching_objfiles (block_objfile (block), objfile))

This is a different implementation but IMO with the same result.

solib_contains_address_p existence makes sense as sometimes it is being used
when only CORE_ADDR is available.

But at this point we have full `struct block' which should be even cheaper to
resolve by getting the (non-inlined) function name -> symbol table -> objfile.
If there would not be full DWARF available there would be no `struct block'.

This change is required for the later use in varobj.c patch where so_list does
not have to be available.


> Assuming my understanding of the semantics is correct, then this is ok
> with the above fixlets.  Thanks.

Sorry for that indentation/placement.


Thanks,
Jan


gdb/
2009-04-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
	* block.c (block_objfile): New function.
	* block.h (block_objfile): New prototype.
	* objfiles.c (matching_objfiles): New function.
	* objfiles.h (matching_objfiles): New prototype.
	* printcmd.c: Remove include solib.h.
	(display_uses_solib_p): Rename to ...
	(display_uses_objfile): ... a new function name.  Change the SOLIB
	parameter to OBJFILE parameter.  Use now a matching_objfiles call.
	(clear_dangling_display_expressions): Update the caller.

--- gdb/block.c	3 Jan 2009 05:57:50 -0000	1.18
+++ gdb/block.c	22 Apr 2009 19:51:40 -0000
@@ -309,3 +309,21 @@ allocate_block (struct obstack *obstack)
 
   return bl;
 }
+
+/* Return OBJFILE in which BLOCK is located or NULL if we cannot find it for
+   whatever reason.  */
+
+struct objfile *
+block_objfile (const struct block *block)
+{
+  struct symbol *func;
+
+  if (block == NULL)
+    return NULL;
+
+  func = block_linkage_function (block);
+  if (func == NULL)
+    return NULL;
+
+  return SYMBOL_SYMTAB (func)->objfile;
+}
--- gdb/block.h	3 Jan 2009 05:57:50 -0000	1.19
+++ gdb/block.h	22 Apr 2009 19:51:40 -0000
@@ -164,4 +164,6 @@ extern const struct block *block_global_
 
 extern struct block *allocate_block (struct obstack *obstack);
 
+extern struct objfile *block_objfile (const struct block *block);
+
 #endif /* BLOCK_H */
--- gdb/objfiles.c	11 Mar 2009 20:26:02 -0000	1.82
+++ gdb/objfiles.c	22 Apr 2009 19:51:40 -0000
@@ -891,3 +891,21 @@ objfile_data (struct objfile *objfile, c
   gdb_assert (data->index < objfile->num_data);
   return objfile->data[data->index];
 }
+
+/* Return non-zero if A and B point to the same OBJFILE, ignoring any binary
+   vs. debuginfo variants of the pointers.  If either A or B is NULL return
+   zero as not a match.  */
+
+int
+matching_objfiles (struct objfile *a, struct objfile *b)
+{
+  if (a == NULL || b == NULL)
+    return 0;
+
+  if (a->separate_debug_objfile_backlink)
+    a = a->separate_debug_objfile_backlink;
+  if (b->separate_debug_objfile_backlink)
+    b = b->separate_debug_objfile_backlink;
+
+  return a == b;
+}
--- gdb/objfiles.h	15 Jan 2009 16:35:22 -0000	1.59
+++ gdb/objfiles.h	22 Apr 2009 19:51:40 -0000
@@ -497,6 +497,8 @@ extern struct obj_section *find_pc_secti
 
 extern int in_plt_section (CORE_ADDR, char *);
 
+extern int matching_objfiles (struct objfile *a, struct objfile *b);
+
 /* Keep a registry of per-objfile data-pointers required by other GDB
    modules.  */
 
--- gdb/printcmd.c	25 Mar 2009 22:38:46 -0000	1.150
+++ gdb/printcmd.c	22 Apr 2009 19:51:40 -0000
@@ -46,7 +46,6 @@
 #include "exceptions.h"
 #include "observer.h"
 #include "solist.h"
-#include "solib.h"
 #include "parser-defs.h"
 #include "charset.h"
 
@@ -1760,19 +1759,17 @@ disable_display_command (char *args, int
       }
 }
 
-/* Return 1 if D uses SOLIB (and will become dangling when SOLIB
+/* Return 1 if D uses OBJFILE (and will become dangling when OBJFILE
    is unloaded), otherwise return 0.  */
 
 static int
-display_uses_solib_p (const struct display *d,
-		      const struct so_list *solib)
+display_uses_objfile (const struct display *d, struct objfile *objfile)
 {
   int endpos;
   struct expression *const exp = d->exp;
   const union exp_element *const elts = exp->elts;
 
-  if (d->block != NULL
-      && solib_contains_address_p (solib, d->block->startaddr))
+  if (matching_objfiles (block_objfile (d->block), objfile))
     return 1;
 
   for (endpos = exp->nelts; endpos > 0; )
@@ -1791,11 +1788,12 @@ display_uses_solib_p (const struct displ
 	  const struct obj_section *const section =
 	    SYMBOL_OBJ_SECTION (symbol);
 
-	  if (block != NULL
-	      && solib_contains_address_p (solib, block->startaddr))
+	  /* Check objfile where is placed the code touching the variable.  */
+	  if (matching_objfiles (block_objfile (block), objfile))
 	    return 1;
 
-	  if (section && section->objfile == solib->objfile)
+	  /* Check objfile where the variable itself is placed.  */
+	  if (section && section->objfile == objfile)
 	    return 1;
 	}
       endpos -= oplen;
@@ -1820,7 +1818,7 @@ clear_dangling_display_expressions (stru
 
   for (d = display_chain; d; d = d->next)
     {
-      if (d->exp && display_uses_solib_p (d, solib))
+      if (d->exp && display_uses_objfile (d, solib->objfile))
 	{
 	  xfree (d->exp);
 	  d->exp = NULL;

obsolete: Re: [patch] Make a function for block->objfile lookups

[Jan Kratochvil]

This patch (not checked in) got now obsoleted by:
	[patch 3/8] Types GC [display_uses_solib_p to exp_iterate]
	http://sourceware.org/ml/gdb-patches/2009-05/msg00546.html