[RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[Doug Evans]

Hi.

While debugging a gdb segv due to a SIGINT arriving while expanding
symtabs, I found it useful to have a command that did symtab consistency/
sanity checks.

There is already "maint check-symtabs" but it really is "maint check-psymtabs"
(e.g., it only loops over all psymtabs).

Plus, when debugging gdb I'd like to be able to do "maint check-foo" *without*
it changing gdb's state (e.g., resulting in more symtab expansion).

So this patch does three things:

1) Rename "maint check-symtabs" to "maint check-psymtabs".
2) Add -n option to "maint check-psymtabs" to prevent symtab expansion.
3) Add "maint check-symtabs".

The new "maint check-symtabs" command is pretty minimal at the moment,
it only contains what I need at present.
We can always add whatever other consistency/sanity checks we want to
it in the future.

Ok to check in?

2013-05-07  Doug Evans  <dje@google.com>

	* NEWS: Mention "maint check-symtabs", "maint check-psymtabs".
	* psymtab.c (maintenance_check_psymtabs): Renamed from
	maintenance_check_symtabs.  Handle -n argument: avoid side effects
	like symtab expansion.
	(_initialize_psymtab): Update.
	* symmisc.c (maintenance_check_symtabs): New function.
	(_initialize_symmisc): Add "mt check-symtabs" command.

	doc/
	* gdb.texinfo (Maintenance Commands): Update doc for
	"maint check-psymtabs".  Add doc for "maint check-symtabs".

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.593
diff -u -p -r1.593 NEWS
--- NEWS	16 Apr 2013 14:36:53 -0000	1.593
+++ NEWS	8 May 2013 00:50:11 -0000
@@ -6,6 +6,10 @@
 * New commands:
 catch rethrow
   Like "catch throw", but catches a re-thrown exception.
+maint check-psymtabs
+  Renamed from old "maint check-symtabs".
+maint check-symtabs
+  Perform consistency checks on symtabs.
 
 show configuration
   Display the details of GDB configure-time options.
Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.74
diff -u -p -r1.74 psymtab.c
--- psymtab.c	6 May 2013 19:15:17 -0000	1.74
+++ psymtab.c	8 May 2013 00:39:56 -0000
@@ -2001,10 +2006,10 @@ maintenance_info_psymtabs (char *regexp,
     }
 }
 
-/* Check consistency of psymtabs and symtabs.  */
+/* Check consistency of psymtabs vs symtabs.  */
 
 static void
-maintenance_check_symtabs (char *ignore, int from_tty)
+maintenance_check_psymtabs (char *args, int from_tty)
 {
   struct symbol *sym;
   struct partial_symbol **psym;
@@ -2014,12 +2019,24 @@ maintenance_check_symtabs (char *ignore,
   struct objfile *objfile;
   struct block *b;
   int length;
+  int allow_side_effects = 1;
+
+  if (args)
+    {
+      if (strcmp (args, "-n") == 0)
+	allow_side_effects = 0;
+      else
+	error (_("Invalid option."));
+    }
 
   ALL_PSYMTABS (objfile, ps)
   {
     struct gdbarch *gdbarch = get_objfile_arch (objfile);
 
-    s = psymtab_to_symtab (objfile, ps);
+    if (allow_side_effects)
+      s = psymtab_to_symtab (objfile, ps);
+    else
+      s = ps->symtab;
     if (s == NULL)
       continue;
     bv = BLOCKVECTOR (s);
@@ -2134,7 +2151,11 @@ This does not include information about 
 just the symbol table structures themselves."),
 	   &maintenanceinfolist);
 
-  add_cmd ("check-symtabs", class_maintenance, maintenance_check_symtabs,
-	   _("Check consistency of psymtabs and symtabs."),
+  add_cmd ("check-psymtabs", class_maintenance, maintenance_check_psymtabs,
+	   _("\
+Check consistency of psymtabs versus symtabs.\n\
+Usage: maint check-psymtabs [-n]\n\
+Options:\n\
+  -n    Do not cause any side effects such as symtab expansion."),
 	   &maintenancelist);
 }
Index: symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.96
diff -u -p -r1.96 symmisc.c
--- symmisc.c	9 Apr 2013 02:17:17 -0000	1.96
+++ symmisc.c	8 May 2013 00:39:57 -0000
@@ -771,6 +771,62 @@ maintenance_info_symtabs (char *regexp, 
         printf_filtered ("}\n");
     }
 }
+
+/* Check consistency of symtabs.
+   An example of what this checks for is NULL blockvectors.
+   They can happen if there's a bug during debug info reading.
+   GDB assumes they are always non-NULL.
+
+   Note: This does not check for psymtab vs symtab consistency.
+   Use "maint check-psymtabs" for that.  */
+
+static void
+maintenance_check_symtabs (char *ignore, int from_tty)
+{
+  struct program_space *pspace;
+  struct objfile *objfile;
+
+  ALL_PSPACES (pspace)
+    ALL_PSPACE_OBJFILES (pspace, objfile)
+    {
+      struct symtab *symtab;
+
+      /* We don't want to print anything for this objfile until we
+         actually find something worth printing.  */
+      int printed_objfile_start = 0;
+
+      ALL_OBJFILE_SYMTABS (objfile, symtab)
+	{
+	  int found_something = 0;
+
+	  QUIT;
+
+	  if (symtab->blockvector == NULL)
+	    found_something = 1;
+	  /* Add more checks here.  */
+
+	  if (found_something)
+	    {
+	      if (! printed_objfile_start)
+		{
+		  printf_filtered ("{ objfile %s ", objfile->name);
+		  wrap_here ("  ");
+		  printf_filtered ("((struct objfile *) %s)\n", 
+				   host_address_to_string (objfile));
+		  printed_objfile_start = 1;
+		}
+	      printf_filtered ("  { symtab %s\n",
+			       symtab_to_filename_for_display (symtab));
+	      if (symtab->blockvector == NULL)
+		printf_filtered ("    NULL blockvector\n");
+	      printf_filtered ("  }\n");
+	    }
+	}
+
+      if (printed_objfile_start)
+        printf_filtered ("}\n");
+    }
+}
 \f
 
 /* Return the nexting depth of a block within other blocks in its symtab.  */
@@ -819,4 +875,10 @@ This does not include information about 
 linetables --- just the symbol table structures themselves.\n\
 With an argument REGEXP, list the symbol tables whose names that match that."),
 	   &maintenanceinfolist);
+
+  add_cmd ("check-symtabs", class_maintenance, maintenance_check_symtabs,
+	   _("\
+Check consistency of symtabs.\n\
+Usage: maint check-symtabs"),
+	   &maintenancelist);
 }
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1084
diff -u -p -r1.1084 gdb.texinfo
--- doc/gdb.texinfo	29 Apr 2013 17:32:43 -0000	1.1084
+++ doc/gdb.texinfo	8 May 2013 00:39:57 -0000
@@ -35487,9 +35487,16 @@ only if non-stop mode is active (@pxref{
 architecture supports displaced stepping.
 @end table
 
+@kindex maint check-psymtabs
+@item maint check-psymtabs [-n]
+Check the consistency of psymtabs versus symtabs.
+Use this to check, for example, whether a symbol is one but not the other.
+If optional parameter @code{-n} is specified, do not do anything
+that will have side effects such as expanding symbol tables.
+
 @kindex maint check-symtabs
 @item maint check-symtabs
-Check the consistency of psymtabs and symtabs.
+Check the consistency of symtabs.
 
 @kindex maint cplus first_component
 @item maint cplus first_component @var{name}

Re: [RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[Doug Evans]

On Tue, May 7, 2013 at 6:02 PM, Doug Evans <dje@google.com> wrote:
> Plus, when debugging gdb I'd like to be able to do "maint check-foo" *without*
> it changing gdb's state (e.g., resulting in more symtab expansion).
>
> So this patch does three things:
>
> 1) Rename "maint check-symtabs" to "maint check-psymtabs".
> 2) Add -n option to "maint check-psymtabs" to prevent symtab expansion.
> 3) Add "maint check-symtabs".

Another way to go, and I like this better,
is to add a command that expands symtabs (perhaps based on a regexp),
and then have "maint check-psymtabs" not do symtab expansion at all.
[I'd prefer it if "maint check-foo" always had a default behaviour of
no side effects.]

Re: [RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

>> 1) Rename "maint check-symtabs" to "maint check-psymtabs".
>> 2) Add -n option to "maint check-psymtabs" to prevent symtab expansion.
>> 3) Add "maint check-symtabs".

Doug> Another way to go, and I like this better,
Doug> is to add a command that expands symtabs (perhaps based on a regexp),
Doug> and then have "maint check-psymtabs" not do symtab expansion at all.
Doug> [I'd prefer it if "maint check-foo" always had a default behaviour of
Doug> no side effects.]

I also like this idea.
I thought the patch was fine, too.

Tom

Re: [RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[Eli Zaretskii]

> Date: Tue, 07 May 2013 18:02:59 -0700
> From: Doug Evans <dje@google.com>
> 
> 1) Rename "maint check-symtabs" to "maint check-psymtabs".
> 2) Add -n option to "maint check-psymtabs" to prevent symtab expansion.
> 3) Add "maint check-symtabs".
> 
> The new "maint check-symtabs" command is pretty minimal at the moment,
> it only contains what I need at present.
> We can always add whatever other consistency/sanity checks we want to
> it in the future.
> 
> Ok to check in?

The documentation parts are OK with one comment:

> +@kindex maint check-psymtabs
> +@item maint check-psymtabs [-n]
> +Check the consistency of psymtabs versus symtabs.
> +Use this to check, for example, whether a symbol is one but not the other.

Did you mean "a symbol is in one but not the other"?  IOW, did you
omit "in"?

Thanks.

Re: [RFA, doc RFA] Rename "maint check-symtabs" to "maint check-psymtabs"

[Doug Evans]

On Wed, May 8, 2013 at 10:08 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> +@kindex maint check-psymtabs
>> +@item maint check-psymtabs [-n]
>> +Check the consistency of psymtabs versus symtabs.
>> +Use this to check, for example, whether a symbol is one but not the other.
>
> Did you mean "a symbol is in one but not the other"?  IOW, did you
> omit "in"?

Yeah, already fixed in my sandbox.
Patch to implement my preferred choice (maint expand-symtabs) to follow.