public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] make language_of_main static
  2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
  2014-01-06 17:11 ` [PATCH 2/3] move main name into the progspace Tom Tromey
  2014-01-06 17:11 ` [PATCH 3/3] move the "main" data into the per-BFD object Tom Tromey
@ 2014-01-06 17:11 ` Tom Tromey
  2014-01-06 19:02 ` [PATCH 0/3] main_name cleanups Pedro Alves
  2014-01-15 18:01 ` Tom Tromey
  4 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This makes the global language_of_main static.  Now it can be set only
via a new argument to set_main_name.

2014-01-06  Tom Tromey  <tromey@redhat.com>

	* dbxread.c (process_one_symbol): Update.
	* dwarf2read.c (read_partial_die): Update.
	* symfile.c (set_initial_language): Call main_language.
	* symtab.c (language_of_main): Now static.
	(set_main_name): Add 'lang' parameter.
	(find_main_name): Update.
	(main_language): New function.
	(symtab_observer_executable_changed): Update.
	* symtab.h (set_main_name): Update.
	(language_of_main): Remove.
	(main_language): Declare.
---
 gdb/ChangeLog    | 14 ++++++++++++++
 gdb/dbxread.c    |  2 +-
 gdb/dwarf2read.c |  8 +-------
 gdb/symfile.c    |  6 ++----
 gdb/symtab.c     | 25 +++++++++++++++++--------
 gdb/symtab.h     |  4 ++--
 6 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 1bab8b8..71eb5f8 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -3251,7 +3251,7 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
 	 N_MAIN within a given objfile, complain() and choose
 	 arbitrarily.  (kingdon) */
       if (name != NULL)
-	set_main_name (name);
+	set_main_name (name, language_unknown);
       break;
 
       /* The following symbol types can be ignored.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 23bcfe0..0bd7100 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15313,13 +15313,7 @@ read_partial_die (const struct die_reader_specs *reader,
 	     practice.  */
 	  if (DW_UNSND (&attr) == DW_CC_program
 	      && cu->language == language_fortran)
-	    {
-	      set_main_name (part_die->name);
-
-	      /* As this DIE has a static linkage the name would be difficult
-		 to look up later.  */
-	      language_of_main = language_fortran;
-	    }
+	    set_main_name (part_die->name, language_fortran);
 	  break;
 	case DW_AT_inline:
 	  if (DW_UNSND (&attr) == DW_INL_inlined
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 4f0fea8..b4b4150 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1628,11 +1628,9 @@ symbol_file_command (char *args, int from_tty)
 void
 set_initial_language (void)
 {
-  enum language lang = language_unknown;
+  enum language lang = main_language ();
 
-  if (language_of_main != language_unknown)
-    lang = language_of_main;
-  else
+  if (lang == language_unknown)
     {
       char *name = main_name ();
       struct symbol *sym = lookup_symbol (name, NULL, VAR_DOMAIN, NULL);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f215586..188bc8a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5006,10 +5006,10 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 \f
 /* Track MAIN */
 static char *name_of_main;
-enum language language_of_main = language_unknown;
+static enum language language_of_main = language_unknown;
 
 void
-set_main_name (const char *name)
+set_main_name (const char *name, enum language lang)
 {
   if (name_of_main != NULL)
     {
@@ -5020,7 +5020,7 @@ set_main_name (const char *name)
   if (name != NULL)
     {
       name_of_main = xstrdup (name);
-      language_of_main = language_unknown;
+      language_of_main = lang;
     }
 }
 
@@ -5051,27 +5051,27 @@ find_main_name (void)
   new_main_name = ada_main_name ();
   if (new_main_name != NULL)
     {
-      set_main_name (new_main_name);
+      set_main_name (new_main_name, language_ada);
       return;
     }
 
   new_main_name = go_main_name ();
   if (new_main_name != NULL)
     {
-      set_main_name (new_main_name);
+      set_main_name (new_main_name, language_go);
       return;
     }
 
   new_main_name = pascal_main_name ();
   if (new_main_name != NULL)
     {
-      set_main_name (new_main_name);
+      set_main_name (new_main_name, language_pascal);
       return;
     }
 
   /* The languages above didn't identify the name of the main procedure.
      Fallback to "main".  */
-  set_main_name ("main");
+  set_main_name ("main", language_unknown);
 }
 
 char *
@@ -5083,13 +5083,22 @@ main_name (void)
   return name_of_main;
 }
 
+/* Return the language of the main function.  If it is not known,
+   return language_unknown.  */
+
+enum language
+main_language (void)
+{
+  return language_of_main;
+}
+
 /* Handle ``executable_changed'' events for the symtab module.  */
 
 static void
 symtab_observer_executable_changed (void)
 {
   /* NAME_OF_MAIN may no longer be the same, so reset it for now.  */
-  set_main_name (NULL);
+  set_main_name (NULL, language_unknown);
 }
 
 /* Return 1 if the supplied producer string matches the ARM RealView
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 97e9ad7..4a93374 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1324,9 +1324,9 @@ extern struct cleanup *make_cleanup_free_search_symbols (struct symbol_search
    FIXME: cagney/2001-03-20: Can't make main_name() const since some
    of the calling code currently assumes that the string isn't
    const.  */
-extern void set_main_name (const char *name);
+extern void set_main_name (const char *name, enum language lang);
 extern /*const */ char *main_name (void);
-extern enum language language_of_main;
+extern enum language main_language (void);
 
 /* Check global symbols in objfile.  */
 struct symbol *lookup_global_symbol_from_objfile (const struct objfile *,
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
  2014-01-06 17:11 ` [PATCH 2/3] move main name into the progspace Tom Tromey
@ 2014-01-06 17:11 ` Tom Tromey
  2014-01-06 17:39   ` Doug Evans
  2014-01-06 17:11 ` [PATCH 1/3] make language_of_main static Tom Tromey
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This adds the "main"-related data into the per-BFD.  This is needed
because once symbol sharing across objfiles is complete, computing the
main name as a side effect of symbol reading will no longer work --
the symbols simply won't be re-read.

After this change, set_main_name is only used by the main_name
machinery itself, so this patch makes it static.

2014-01-06  Tom Tromey  <tromey@redhat.com>

	* dbxread.c (process_one_symbol): Use set_objfile_main_name.
	* dwarf2read.c (read_partial_die): Use set_objfile_main_name.
	* objfiles.c (get_objfile_bfd_data): Initialize language_of_main.
	(set_objfile_main_name): New function.
	* objfiles.h (struct objfile_per_bfd_storage) <name_of_main,
	language_of_main>: New fields.
	(set_objfile_main_name): Declare.
	* symtab.c (find_main_name): Loop over objfiles to find the main
	name and language.
	(set_main_name): Now static.
	* symtab.h (set_main_name): Don't declare.
---
 gdb/ChangeLog    | 14 ++++++++++++++
 gdb/dbxread.c    |  2 +-
 gdb/dwarf2read.c |  2 +-
 gdb/objfiles.c   | 15 +++++++++++++++
 gdb/objfiles.h   | 12 ++++++++++++
 gdb/symtab.c     | 13 ++++++++++++-
 gdb/symtab.h     |  1 -
 7 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 71eb5f8..6512735 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -3251,7 +3251,7 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, char *name,
 	 N_MAIN within a given objfile, complain() and choose
 	 arbitrarily.  (kingdon) */
       if (name != NULL)
-	set_main_name (name, language_unknown);
+	set_objfile_main_name (objfile, name, language_unknown);
       break;
 
       /* The following symbol types can be ignored.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0bd7100..68de1df 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -15313,7 +15313,7 @@ read_partial_die (const struct die_reader_specs *reader,
 	     practice.  */
 	  if (DW_UNSND (&attr) == DW_CC_program
 	      && cu->language == language_fortran)
-	    set_main_name (part_die->name, language_fortran);
+	    set_objfile_main_name (objfile, part_die->name, language_fortran);
 	  break;
 	case DW_AT_inline:
 	  if (DW_UNSND (&attr) == DW_INL_inlined
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 73847bd..84b13d7 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -152,6 +152,7 @@ get_objfile_bfd_data (struct objfile *objfile, struct bfd *abfd)
       obstack_init (&storage->storage_obstack);
       storage->filename_cache = bcache_xmalloc (NULL, NULL);
       storage->macro_cache = bcache_xmalloc (NULL, NULL);
+      storage->language_of_main = language_unknown;
     }
 
   return storage;
@@ -186,6 +187,20 @@ set_objfile_per_bfd (struct objfile *objfile)
   objfile->per_bfd = get_objfile_bfd_data (objfile, objfile->obfd);
 }
 
+/* Set the objfile's per-BFD notion of the "main" name and
+   language.  */
+
+void
+set_objfile_main_name (struct objfile *objfile,
+		       const char *name, enum language lang)
+{
+  if (objfile->per_bfd->name_of_main == NULL
+      || strcmp (objfile->per_bfd->name_of_main, name) != 0)
+    objfile->per_bfd->name_of_main
+      = obstack_copy0 (&objfile->per_bfd->storage_obstack, name, strlen (name));
+  objfile->per_bfd->language_of_main = lang;
+}
+
 \f
 
 /* Called via bfd_map_over_sections to build up the section table that
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index c2b6177..a7db251 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -192,6 +192,13 @@ struct objfile_per_bfd_storage
      name, and the second is the demangled name or just a zero byte
      if the name doesn't demangle.  */
   struct htab *demangled_names_hash;
+
+  /* The name and language of any "main" found in this objfile.  The
+     name can be NULL, which means that the information was not
+     recorded.  */
+
+  const char *name_of_main;
+  enum language language_of_main;
 };
 
 /* Master structure for keeping track of each file from which
@@ -685,4 +692,9 @@ void set_objfile_per_bfd (struct objfile *obj);
 
 const char *objfile_name (const struct objfile *objfile);
 
+/* Set the objfile's notion of the "main" name and language.  */
+
+extern void set_objfile_main_name (struct objfile *objfile,
+				   const char *name, enum language lang);
+
 #endif /* !defined (OBJFILES_H) */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index d01a7d7..0c25609 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5057,7 +5057,7 @@ main_info_cleanup (struct program_space *pspace, void *data)
   xfree (info);
 }
 
-void
+static void
 set_main_name (const char *name, enum language lang)
 {
   struct main_info *info = get_main_info ();
@@ -5082,6 +5082,17 @@ static void
 find_main_name (void)
 {
   const char *new_main_name;
+  struct objfile *objfile;
+
+  ALL_OBJFILES (objfile)
+  {
+    if (objfile->per_bfd->name_of_main != NULL)
+      {
+	set_main_name (objfile->per_bfd->name_of_main,
+		       objfile->per_bfd->language_of_main);
+	return;
+      }
+  }
 
   /* Try to see if the main procedure is in Ada.  */
   /* FIXME: brobecker/2005-03-07: Another way of doing this would
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 4a93374..25ac028 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1324,7 +1324,6 @@ extern struct cleanup *make_cleanup_free_search_symbols (struct symbol_search
    FIXME: cagney/2001-03-20: Can't make main_name() const since some
    of the calling code currently assumes that the string isn't
    const.  */
-extern void set_main_name (const char *name, enum language lang);
 extern /*const */ char *main_name (void);
 extern enum language main_language (void);
 
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] move main name into the progspace
  2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
@ 2014-01-06 17:11 ` Tom Tromey
  2014-01-06 17:11 ` [PATCH 3/3] move the "main" data into the per-BFD object Tom Tromey
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 17:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This moves the "main" name and language into an object attached to the
current progspace.  This prevents problems if there are multiple
inferiors tha have different ideas of "main" -- which matters at least
for unwinding, see frame.c:inside_main_func.

2014-01-06  Tom Tromey  <tromey@redhat.com>

	* symtab.c (main_progspace_key): New global.
	(struct main_info): New.
	(name_of_main, language_of_main): Remove.
	(get_main_info, main_info_cleanup): New function.
	(set_main_name, main_name, main_language): Use get_main_info.
	(_initialize_symtab): Initialize main_progspace_key.
---
 gdb/ChangeLog |  9 +++++++
 gdb/symtab.c  | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 188bc8a..d01a7d7 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -106,6 +106,23 @@ void _initialize_symtab (void);
 
 /* */
 
+/* Program space key for finding name and language of "main".  */
+
+static const struct program_space_data *main_progspace_key;
+
+/* Type of the data stored on the program space.  */
+
+struct main_info
+{
+  /* Name of "main".  */
+
+  char *name_of_main;
+
+  /* Language of "main".  */
+
+  enum language language_of_main;
+};
+
 /* When non-zero, print debugging messages related to symtab creation.  */
 unsigned int symtab_create_debug = 0;
 
@@ -5005,22 +5022,56 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 }
 \f
 /* Track MAIN */
-static char *name_of_main;
-static enum language language_of_main = language_unknown;
+
+/* Return the "main_info" object for the current program space.  If
+   the object has not yet been created, create it and fill in some
+   default values.  */
+
+static struct main_info *
+get_main_info (void)
+{
+  struct main_info *info = program_space_data (current_program_space,
+					       main_progspace_key);
+
+  if (info == NULL)
+    {
+      info = XCNEW (struct main_info);
+      info->language_of_main = language_unknown;
+      set_program_space_data (current_program_space, main_progspace_key,
+			      info);
+    }
+
+  return info;
+}
+
+/* A cleanup to destroy a struct main_info when a progspace is
+   destroyed.  */
+
+static void
+main_info_cleanup (struct program_space *pspace, void *data)
+{
+  struct main_info *info = data;
+
+  if (info != NULL)
+    xfree (info->name_of_main);
+  xfree (info);
+}
 
 void
 set_main_name (const char *name, enum language lang)
 {
-  if (name_of_main != NULL)
+  struct main_info *info = get_main_info ();
+
+  if (info->name_of_main != NULL)
     {
-      xfree (name_of_main);
-      name_of_main = NULL;
-      language_of_main = language_unknown;
+      xfree (info->name_of_main);
+      info->name_of_main = NULL;
+      info->language_of_main = language_unknown;
     }
   if (name != NULL)
     {
-      name_of_main = xstrdup (name);
-      language_of_main = lang;
+      info->name_of_main = xstrdup (name);
+      info->language_of_main = lang;
     }
 }
 
@@ -5077,10 +5128,12 @@ find_main_name (void)
 char *
 main_name (void)
 {
-  if (name_of_main == NULL)
+  struct main_info *info = get_main_info ();
+
+  if (info->name_of_main == NULL)
     find_main_name ();
 
-  return name_of_main;
+  return info->name_of_main;
 }
 
 /* Return the language of the main function.  If it is not known,
@@ -5089,7 +5142,12 @@ main_name (void)
 enum language
 main_language (void)
 {
-  return language_of_main;
+  struct main_info *info = get_main_info ();
+
+  if (info->name_of_main == NULL)
+    find_main_name ();
+
+  return info->language_of_main;
 }
 
 /* Handle ``executable_changed'' events for the symtab module.  */
@@ -5278,6 +5336,9 @@ _initialize_symtab (void)
 {
   initialize_ordinary_address_classes ();
 
+  main_progspace_key
+    = register_program_space_data_with_cleanup (NULL, main_info_cleanup);
+
   add_info ("variables", variables_info, _("\
 All global and static variable names, or those matching REGEXP."));
   if (dbx_commands)
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/3] main_name cleanups
@ 2014-01-06 17:11 Tom Tromey
  2014-01-06 17:11 ` [PATCH 2/3] move main name into the progspace Tom Tromey
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 17:11 UTC (permalink / raw)
  To: gdb-patches

This series cleans up the "main_name" code both for multi-inferior and
for the long-running objfile splitting project.

Currently, the name and language of main are globals.  Also, they can
set in a couple of spots in the debuginfo readers.

Their global-ness can, I think, affect multi-inferior operation.  If
you have two inferiors that have different main names, then I think
perhaps some unwinding scenario could fail, because main_name will
necessarily be incorrect for one of them, and because inside_main_func
checks this value.  The fix here is to make the determination
per-progspace.

Setting them in the debuginfo readers is bad because, once debuginfo
sharing happens, the second progspace to use the debuginfo will not
pick up the main name automatically.  The fix here is to record the
debuginfo readers' findings in the per-BFD object.

Let me know what you think.

Built and regtested on x86-64 Fedora 18.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 17:11 ` [PATCH 3/3] move the "main" data into the per-BFD object Tom Tromey
@ 2014-01-06 17:39   ` Doug Evans
  2014-01-06 18:00     ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2014-01-06 17:39 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Mon, Jan 6, 2014 at 9:11 AM, Tom Tromey <tromey@redhat.com> wrote:
> This adds the "main"-related data into the per-BFD.  This is needed
> because once symbol sharing across objfiles is complete, computing the
> main name as a side effect of symbol reading will no longer work --
> the symbols simply won't be re-read.
>
> After this change, set_main_name is only used by the main_name
> machinery itself, so this patch makes it static.
>
> 2014-01-06  Tom Tromey  <tromey@redhat.com>
>
>         * dbxread.c (process_one_symbol): Use set_objfile_main_name.
>         * dwarf2read.c (read_partial_die): Use set_objfile_main_name.
>         * objfiles.c (get_objfile_bfd_data): Initialize language_of_main.
>         (set_objfile_main_name): New function.
>         * objfiles.h (struct objfile_per_bfd_storage) <name_of_main,
>         language_of_main>: New fields.
>         (set_objfile_main_name): Declare.
>         * symtab.c (find_main_name): Loop over objfiles to find the main
>         name and language.
>         (set_main_name): Now static.
>         * symtab.h (set_main_name): Don't declare.
> [...]
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -5057,7 +5057,7 @@ main_info_cleanup (struct program_space *pspace, void *data)
>    xfree (info);
>  }
>
> -void
> +static void
>  set_main_name (const char *name, enum language lang)
>  {
>    struct main_info *info = get_main_info ();
> @@ -5082,6 +5082,17 @@ static void
>  find_main_name (void)
>  {
>    const char *new_main_name;
> +  struct objfile *objfile;
> +
> +  ALL_OBJFILES (objfile)
> +  {
> +    if (objfile->per_bfd->name_of_main != NULL)
> +      {
> +       set_main_name (objfile->per_bfd->name_of_main,
> +                      objfile->per_bfd->language_of_main);
> +       return;
> +      }
> +  }
>
>    /* Try to see if the main procedure is in Ada.  */
>    /* FIXME: brobecker/2005-03-07: Another way of doing this would

Hi.

[setting aside a day when there are multiple main names,]
Seems like there ought to be an invariant that there is only one main name.
I ask because it's not clear this invariant is enforced (or if it is
it's too subtle) and thus what if this loop finds the wrong one?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 17:39   ` Doug Evans
@ 2014-01-06 18:00     ` Tom Tromey
  2014-01-06 18:23       ` Doug Evans
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 18:00 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

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

Doug> Seems like there ought to be an invariant that there is only one
Doug> main name.  I ask because it's not clear this invariant is
Doug> enforced (or if it is it's too subtle) and thus what if this loop
Doug> finds the wrong one?

I agree there's some room for improvement, but I don't think this series
makes the code worse in any notable way.  There is no such invariant
today and I don't know how it would be enforced.  I think the new code,
if anything, is slightly more likely to get the correct answer, due to
walking the objfiles in load order, rather than reverse order.  For the
DWARF reader at least this only even triggers for Fortran anyway (gdb
doesn't handle DW_AT_main_subprogram yet), which I think implies a lower
exposure to possible difficulties.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 18:00     ` Tom Tromey
@ 2014-01-06 18:23       ` Doug Evans
  2014-01-06 21:11         ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Evans @ 2014-01-06 18:23 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Mon, Jan 6, 2014 at 10:00 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> Seems like there ought to be an invariant that there is only one
> Doug> main name.  I ask because it's not clear this invariant is
> Doug> enforced (or if it is it's too subtle) and thus what if this loop
> Doug> finds the wrong one?
>
> I agree there's some room for improvement, but I don't think this series
> makes the code worse in any notable way.  There is no such invariant
> today and I don't know how it would be enforced.  I think the new code,
> if anything, is slightly more likely to get the correct answer, due to
> walking the objfiles in load order, rather than reverse order.  For the
> DWARF reader at least this only even triggers for Fortran anyway (gdb
> doesn't handle DW_AT_main_subprogram yet), which I think implies a lower
> exposure to possible difficulties.

Sure.
Can I ask for a comment in the code explaining why things are the way they are.
E.g., the new code may be more correct, but relying on an implicit
"walking the objfiles in load order, rather than reverse order" is a
teensy bit on the subtle side. (btw, is it written down anywhere that
ALL_OBJFILES traverses files in load order? If there is such an
invariant, it would be good for future readers to know).
I can't promise to remember this discussion when I'm in the code 6
months from now. :-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] main_name cleanups
  2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
                   ` (2 preceding siblings ...)
  2014-01-06 17:11 ` [PATCH 1/3] make language_of_main static Tom Tromey
@ 2014-01-06 19:02 ` Pedro Alves
  2014-01-13 20:31   ` Tom Tromey
  2014-01-15 18:01 ` Tom Tromey
  4 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2014-01-06 19:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 01/06/2014 05:11 PM, Tom Tromey wrote:
> This series cleans up the "main_name" code both for multi-inferior and
> for the long-running objfile splitting project.
> 
> Currently, the name and language of main are globals.  Also, they can
> set in a couple of spots in the debuginfo readers.
> 
> Their global-ness can, I think, affect multi-inferior operation.  

Definitely.

> If
> you have two inferiors that have different main names, then I think
> perhaps some unwinding scenario could fail, because main_name will
> necessarily be incorrect for one of them, and because inside_main_func
> checks this value.  The fix here is to make the determination
> per-progspace.

Sounds right to me.

I was just wondering why duplicate the main name/language both in the
progspace and in the objfiles?  Wouldn't just storing a pointer to the
objfile that has the main name and its language in the progspace
be enough?  If not, then it'd be good to mention that in a comment,
I think.

> Setting them in the debuginfo readers is bad because, once debuginfo
> sharing happens, the second progspace to use the debuginfo will not
> pick up the main name automatically.  The fix here is to record the
> debuginfo readers' findings in the per-BFD object.
> 
> Let me know what you think.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 18:23       ` Doug Evans
@ 2014-01-06 21:11         ` Tom Tromey
  2014-01-13 20:21           ` Tom Tromey
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2014-01-06 21:11 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

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

Doug> Can I ask for a comment in the code explaining why things are the
Doug> way they are.

Sure.

Doug> (btw, is it written down anywhere that
Doug> ALL_OBJFILES traverses files in load order? If there is such an
Doug> invariant, it would be good for future readers to know).

I don't recall that it is.  It's an effect of how the objfile list is
created.  But IIRC other code depends on this already; but perhaps there
are scenarios where it can be made in another order, I am not certain.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] move the "main" data into the per-BFD object
  2014-01-06 21:11         ` Tom Tromey
@ 2014-01-13 20:21           ` Tom Tromey
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-13 20:21 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug> Can I ask for a comment in the code explaining why things are the
Doug> way they are.

Tom> Sure.

I added this:

  /* First check the objfiles to see whether a debuginfo reader has
     picked up the appropriate main name.  Historically the main name
     was found in a more or less random way; this approach instead
     relies on the order of objfile creation -- which still isn't
     guaranteed to get the correct answer, but is just probably more
     accurate.  */


Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] main_name cleanups
  2014-01-06 19:02 ` [PATCH 0/3] main_name cleanups Pedro Alves
@ 2014-01-13 20:31   ` Tom Tromey
  0 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-13 20:31 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I was just wondering why duplicate the main name/language both in the
Pedro> progspace and in the objfiles?  Wouldn't just storing a pointer to the
Pedro> objfile that has the main name and its language in the progspace
Pedro> be enough?  If not, then it'd be good to mention that in a comment,
Pedro> I think.

The current series is just a simple transform of the existing code.
I think your proposed approach would work ok, though the current code
returns "main" even if it isn't defined in the program -- so we'd need
some extra hack for that.  I'll add a note about this.

If we ever want to support DW_AT_main_subprogram, we'll need some other
changes as well, since multiple functions can be marked that way.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] main_name cleanups
  2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
                   ` (3 preceding siblings ...)
  2014-01-06 19:02 ` [PATCH 0/3] main_name cleanups Pedro Alves
@ 2014-01-15 18:01 ` Tom Tromey
  4 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2014-01-15 18:01 UTC (permalink / raw)
  To: gdb-patches

Tom> This series cleans up the "main_name" code both for multi-inferior and
Tom> for the long-running objfile splitting project.

I'm going to check this in.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-01-15 18:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-06 17:11 [PATCH 0/3] main_name cleanups Tom Tromey
2014-01-06 17:11 ` [PATCH 2/3] move main name into the progspace Tom Tromey
2014-01-06 17:11 ` [PATCH 3/3] move the "main" data into the per-BFD object Tom Tromey
2014-01-06 17:39   ` Doug Evans
2014-01-06 18:00     ` Tom Tromey
2014-01-06 18:23       ` Doug Evans
2014-01-06 21:11         ` Tom Tromey
2014-01-13 20:21           ` Tom Tromey
2014-01-06 17:11 ` [PATCH 1/3] make language_of_main static Tom Tromey
2014-01-06 19:02 ` [PATCH 0/3] main_name cleanups Pedro Alves
2014-01-13 20:31   ` Tom Tromey
2014-01-15 18:01 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).