gdb: Remove an update of current_source_line and current_source_symtab

Message ID 20190612231322.25245-1-andrew.burgess@embecosm.com
State New
Headers show
Series
  • gdb: Remove an update of current_source_line and current_source_symtab
Related show

Commit Message

Andrew Burgess June 12, 2019, 11:13 p.m.
While reviewing some of the annotation code I noticed that
identify_source_line (in source.c) sets current_source_line,
current_source_symtab, and also calls clear_lines_listed_range.  This
seems a little strange, identify_source_line is really a wrapper
around annotate_source, and is only called when annotation_level is
greater than 0 (so annotations are turned on).

It seems weird (to me) that when annotations are on we update GDB's
idea of the "current" line/symtab, but when they are off we don't,
given that annotations are really about communicating GDB's state to a
user (GUI) and surely shouldn't be changing GDB's behaviour.

This commit removes from identify_source_line all of the setting of
current line/symtab and the call to clear_lines_listed_range, after
doing this GDB still passes all tests, so I don't believe these lines
were actually required.

With this code removed identify_source_line is only a wrapper around
annotate_source, so I moved identify_source_line to annotate.c and
renamed it to annotate_source_line.

gdb/ChangeLog:

	* annotate.c: Add 'source.h' and 'objfiles.h' includes.
	(annotate_source): Make static.
	(get_filename_and_charpos): Moved from source.c.
	(annotate_source_line): Moved from source.c and renamed from
	identify_source_line.  Update the return type.
	* annotate.h (annotate_source): Delete declaration.
	(annotate_source_line): Declaration moved from source.h, and
	renamed from identify_source_line.  Return type updated.
	* source.c (get_filename_and_charpos): Moved to annotate.c.
	(identify_source_line): Moved to annotate.c and renamed to
	annotate_source_line.
	(info_line_command): Remove check of annotation_level.
	* source.h (identify_source_line): Move declaration to annotate.h
	and rename to annotate_source_line.
	* stack.c: Add 'annotate.h' include.
	(print_frame_info): Remove check of annotation_level.
---
 gdb/ChangeLog  | 19 +++++++++++++++++++
 gdb/annotate.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/annotate.h | 12 ++++++++++--
 gdb/source.c   | 47 ++---------------------------------------------
 gdb/source.h   | 11 -----------
 gdb/stack.c    |  8 +++-----
 6 files changed, 82 insertions(+), 64 deletions(-)

-- 
2.14.5

Comments

Tom Tromey June 14, 2019, 2:44 p.m. | #1
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:


Andrew> It seems weird (to me) that when annotations are on we update GDB's
Andrew> idea of the "current" line/symtab, but when they are off we don't,
Andrew> given that annotations are really about communicating GDB's state to a
Andrew> user (GUI) and surely shouldn't be changing GDB's behaviour.

Andrew> This commit removes from identify_source_line all of the setting of
Andrew> current line/symtab and the call to clear_lines_listed_range, after
Andrew> doing this GDB still passes all tests, so I don't believe these lines
Andrew> were actually required.

Andrew> With this code removed identify_source_line is only a wrapper around
Andrew> annotate_source, so I moved identify_source_line to annotate.c and
Andrew> renamed it to annotate_source_line.

Thanks for doing this.  This looks good.

Andrew> +get_filename_and_charpos (struct symtab *s, char **fullname)

The fullname parameter could be removed now, since the only caller
passes NULL.

Andrew> +      bool done = annotate_source_line (sal.symtab, sal.line, mid_statement,
Andrew> +				       get_frame_pc (frame));
Andrew>        if (!done)
Andrew>  	{
Andrew>  	  if (deprecated_print_frame_info_listing_hook)

I wonder if this "!done" stuff even makes sense.  It seems a bit strange
that annotations would cause the regular output to change.

Tom

Patch

diff --git a/gdb/annotate.c b/gdb/annotate.c
index e6e8084e9b1..a1ea246c46a 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -26,6 +26,8 @@ 
 #include "inferior.h"
 #include "infrun.h"
 #include "top.h"
+#include "source.h"
+#include "objfiles.h"
 
 
 /* Prototypes for local functions.  */
@@ -431,7 +433,7 @@  annotate_arg_end (void)
     printf_filtered (("\n\032\032arg-end\n"));
 }
 
-void
+static void
 annotate_source (const char *filename, int line, int character, int mid,
 		 struct gdbarch *gdbarch, CORE_ADDR pc)
 {
@@ -444,6 +446,51 @@  annotate_source (const char *filename, int line, int character, int mid,
 		   mid ? "middle" : "beg", paddress (gdbarch, pc));
 }
 
+/* Get full pathname and line number positions for a symtab.  Set *FULLNAME
+   to the actual name of the file as extracted from S, or to NULL if the
+   file is not found (or could not be opened).  */
+
+static void
+get_filename_and_charpos (struct symtab *s, char **fullname)
+{
+  scoped_fd desc = open_source_file (s);
+  if (desc.get () < 0)
+    {
+      if (fullname)
+	*fullname = NULL;
+      return;
+    }
+  if (fullname)
+    *fullname = s->fullname;
+  if (s->line_charpos == nullptr)
+    find_source_lines (s, desc.get ());
+}
+
+/* See annotate.h.  */
+
+bool
+annotate_source_line (struct symtab *s, int line, int mid_statement,
+		      CORE_ADDR pc)
+{
+  if (annotation_level > 0)
+    {
+      if (s->line_charpos == nullptr)
+	get_filename_and_charpos (s, (char **) NULL);
+      if (s->fullname == nullptr)
+	return false;
+      if (line > s->nlines)
+	/* Don't index off the end of the line_charpos array.  */
+	return false;
+      annotate_source (s->fullname, line, s->line_charpos[line - 1],
+		       mid_statement, get_objfile_arch (SYMTAB_OBJFILE (s)),
+		       pc);
+      return true;
+    }
+
+  return false;
+}
+
+
 void
 annotate_frame_begin (int level, struct gdbarch *gdbarch, CORE_ADDR pc)
 {
diff --git a/gdb/annotate.h b/gdb/annotate.h
index ff10d459465..596b5dea499 100644
--- a/gdb/annotate.h
+++ b/gdb/annotate.h
@@ -87,8 +87,16 @@  struct annotate_arg_emitter
   DISABLE_COPY_AND_ASSIGN (annotate_arg_emitter);
 };
 
-extern void annotate_source (const char *, int, int, int,
-			     struct gdbarch *, CORE_ADDR);
+/* If annotations are turned on then print annotation describing the full
+   name of the source file S and the line number LINE and its corresponding
+   character position.
+
+   MID_STATEMENT is nonzero if the PC is not at the beginning of that line.
+
+   Return 1 if successful, 0 if could not find the file or if annotations
+   are turned off.  */
+extern bool annotate_source_line (struct symtab *s, int line,
+				  int mid_statement, CORE_ADDR pc);
 
 extern void annotate_frame_begin (int, struct gdbarch *, CORE_ADDR);
 extern void annotate_function_call (void);
diff --git a/gdb/source.c b/gdb/source.c
index 00052e67cb9..a87b8ea2a92 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1192,49 +1192,6 @@  find_source_lines (struct symtab *s, int desc)
 
 
 
-/* Get full pathname and line number positions for a symtab.
-   Set *FULLNAME to actual name of the file as found by `openp',
-   or to 0 if the file is not found.  */
-
-static void
-get_filename_and_charpos (struct symtab *s, char **fullname)
-{
-  scoped_fd desc = open_source_file (s);
-  if (desc.get () < 0)
-    {
-      if (fullname)
-	*fullname = NULL;
-      return;
-    }
-  if (fullname)
-    *fullname = s->fullname;
-  if (s->line_charpos == 0)
-    find_source_lines (s, desc.get ());
-}
-
-/* See source.h.  */
-
-int
-identify_source_line (struct symtab *s, int line, int mid_statement,
-		      CORE_ADDR pc)
-{
-  if (s->line_charpos == 0)
-    get_filename_and_charpos (s, (char **) NULL);
-  if (s->fullname == 0)
-    return 0;
-  if (line > s->nlines)
-    /* Don't index off the end of the line_charpos array.  */
-    return 0;
-  annotate_source (s->fullname, line, s->line_charpos[line - 1],
-		   mid_statement, get_objfile_arch (SYMTAB_OBJFILE (s)), pc);
-
-  current_source_line = line;
-  current_source_symtab = s;
-  clear_lines_listed_range ();
-  return 1;
-}
-
-
 /* Print source lines from the file of symtab S,
    starting with line number LINE and stopping before line number STOPLINE.  */
 
@@ -1519,8 +1476,8 @@  info_line_command (const char *arg, int from_tty)
 
 	  /* If this is the only line, show the source code.  If it could
 	     not find the file, don't do anything special.  */
-	  if (annotation_level && sals.size () == 1)
-	    identify_source_line (sal.symtab, sal.line, 0, start_pc);
+	  if (sals.size () == 1)
+	    annotate_source_line (sal.symtab, sal.line, 0, start_pc);
 	}
       else
 	/* Is there any case in which we get here, and have an address
diff --git a/gdb/source.h b/gdb/source.h
index f1b5f6e8f7f..4b7bbee270b 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -128,17 +128,6 @@  extern void clear_current_source_symtab_and_line (void);
 /* Add a source path substitution rule.  */
 extern void add_substitute_path_rule (char *, char *);
 
-/* Print text describing the full name of the source file S
-   and the line number LINE and its corresponding character position.
-   The text starts with two Ctrl-z so that the Emacs-GDB interface
-   can easily find it.
-
-   MID_STATEMENT is nonzero if the PC is not at the beginning of that line.
-
-   Return 1 if successful, 0 if could not find the file.  */
-extern int identify_source_line (struct symtab *s, int line,
-				 int mid_statement, CORE_ADDR pc);
-
 /* Flags passed as 4th argument to print_source_lines.  */
 enum print_source_lines_flag
   {
diff --git a/gdb/stack.c b/gdb/stack.c
index 408c795e385..96308f9df51 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -47,6 +47,7 @@ 
 #include "linespec.h"
 #include "cli/cli-utils.h"
 #include "objfiles.h"
+#include "annotate.h"
 
 #include "symfile.h"
 #include "extension.h"
@@ -869,13 +870,10 @@  print_frame_info (struct frame_info *frame, int print_level,
 
   if (source_print && sal.symtab)
     {
-      int done = 0;
       int mid_statement = ((print_what == SRC_LINE)
 			   && frame_show_address (frame, sal));
-
-      if (annotation_level)
-	done = identify_source_line (sal.symtab, sal.line, mid_statement,
-				     get_frame_pc (frame));
+      bool done = annotate_source_line (sal.symtab, sal.line, mid_statement,
+				       get_frame_pc (frame));
       if (!done)
 	{
 	  if (deprecated_print_frame_info_listing_hook)