public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gdb: get_frame_language now takes a frame parameter.
  2015-08-04 15:40 [PATCH 0/2] Make get_frame_langauge take a frame argument Andrew Burgess
@ 2015-08-04 15:40 ` Andrew Burgess
  2015-08-04 16:02   ` Pedro Alves
  2015-08-04 15:40 ` [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c Andrew Burgess
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Burgess @ 2015-08-04 15:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

As part of a drive to remove deprecated_safe_get_selected_frame, make
the get_frame_language function take a frame parameter.  Given the name
of the function this actually seems to make a lot of sense.

The task of fetching a suitable frame is then passed to the calling
functions.  For get_frame_language there are not many callers, these are
updated to get the selected frame in a suitable way.

gdb/ChangeLog:

	* language.c (show_language_command): Find selected frame before
	asking for the language of that frame.
	(set_language_command): Likewise.
	* language.h (get_frame_language): Add frame parameter.
	* stack.c (get_frame_language): Add frame parameter, assert
	parameter is not NULL, update comment and reindent.
	* top.c (check_frame_language_change): Pass the selected frame
	into get_frame_language.
---
 gdb/ChangeLog  | 11 +++++++++++
 gdb/language.c | 33 +++++++++++++++++++++++--------
 gdb/language.h |  2 +-
 gdb/stack.c    | 61 ++++++++++++++++++++++++++++------------------------------
 gdb/top.c      |  7 ++++---
 5 files changed, 70 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index af41072..223d2e5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* language.c (show_language_command): Find selected frame before
+	asking for the language of that frame.
+	(set_language_command): Likewise.
+	* language.h (get_frame_language): Add frame parameter.
+	* stack.c (get_frame_language): Add frame parameter, assert
+	parameter is not NULL, update comment and reindent.
+	* top.c (check_frame_language_change): Pass the selected frame
+	into get_frame_language.
+
 2015-08-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* infcmd.c (signal_command): Call do_cleanups for args_chain.
diff --git a/gdb/language.c b/gdb/language.c
index a8b432e..989a8da 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -118,7 +118,8 @@ static void
 show_language_command (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  enum language flang;		/* The language of the current frame.  */
+  struct frame_info *frame;
+  enum language flang;		/* The language of the frame.  */
 
   if (language_mode == language_mode_auto)
     fprintf_filtered (gdb_stdout,
@@ -130,11 +131,15 @@ show_language_command (struct ui_file *file, int from_tty,
 		      _("The current source language is \"%s\".\n"),
 		      current_language->la_name);
 
-  flang = get_frame_language ();
-  if (flang != language_unknown &&
-      language_mode == language_mode_manual &&
-      current_language->la_language != flang)
-    printf_filtered ("%s\n", lang_frame_mismatch_warn);
+  if (target_has_execution)
+    {
+      frame = get_selected_frame (NULL);
+      flang = get_frame_language (frame);
+      if (flang != language_unknown &&
+	  language_mode == language_mode_manual &&
+	  current_language->la_language != flang)
+	printf_filtered ("%s\n", lang_frame_mismatch_warn);
+    }
 }
 
 /* Set command.  Change the current working language.  */
@@ -142,7 +147,7 @@ static void
 set_language_command (char *ignore, int from_tty, struct cmd_list_element *c)
 {
   int i;
-  enum language flang;
+  enum language flang = language_unknown;
 
   /* Search the list of languages for a match.  */
   for (i = 0; i < languages_size; i++)
@@ -155,7 +160,19 @@ set_language_command (char *ignore, int from_tty, struct cmd_list_element *c)
 	      /* Enter auto mode.  Set to the current frame's language, if
                  known, or fallback to the initial language.  */
 	      language_mode = language_mode_auto;
-	      flang = get_frame_language ();
+	      TRY
+		{
+		  struct frame_info *frame;
+
+		  frame = get_selected_frame (NULL);
+		  flang = get_frame_language (frame);
+		}
+	      CATCH (ex, RETURN_MASK_ERROR)
+		{
+		  flang = language_unknown;
+		}
+	      END_CATCH
+
 	      if (flang != language_unknown)
 		set_language (flang);
 	      else
diff --git a/gdb/language.h b/gdb/language.h
index 4ecb103..8782ef0 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -544,7 +544,7 @@ extern const char *language_str (enum language);
 
 extern void add_language (const struct language_defn *);
 
-extern enum language get_frame_language (void);	/* In stack.c */
+extern enum language get_frame_language (struct frame_info *frame);	/* In stack.c */
 
 /* Check for a language-specific trampoline.  */
 
diff --git a/gdb/stack.c b/gdb/stack.c
index b4cfdbd..31a723d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2560,46 +2560,43 @@ func_command (char *arg, int from_tty)
     select_and_print_frame (frame);
 }
 
-/* Gets the language of the current frame.  */
+/* Gets the language of FRAME.  */
 
 enum language
-get_frame_language (void)
+get_frame_language (struct frame_info *frame)
 {
-  struct frame_info *frame = deprecated_safe_get_selected_frame ();
+  CORE_ADDR pc = 0;
+  int pc_p = 0;
 
-  if (frame)
-    {
-      CORE_ADDR pc = 0;
-      int pc_p = 0;
+  gdb_assert (frame!= NULL);
 
-      /* We determine the current frame language by looking up its
-         associated symtab.  To retrieve this symtab, we use the frame
-         PC.  However we cannot use the frame PC as is, because it
-         usually points to the instruction following the "call", which
-         is sometimes the first instruction of another function.  So
-         we rely on get_frame_address_in_block(), it provides us with
-         a PC that is guaranteed to be inside the frame's code
-         block.  */
+    /* We determine the current frame language by looking up its
+       associated symtab.  To retrieve this symtab, we use the frame
+       PC.  However we cannot use the frame PC as is, because it
+       usually points to the instruction following the "call", which
+       is sometimes the first instruction of another function.  So
+       we rely on get_frame_address_in_block(), it provides us with
+       a PC that is guaranteed to be inside the frame's code
+       block.  */
 
-      TRY
-	{
-	  pc = get_frame_address_in_block (frame);
-	  pc_p = 1;
-	}
-      CATCH (ex, RETURN_MASK_ERROR)
-	{
-	  if (ex.error != NOT_AVAILABLE_ERROR)
-	    throw_exception (ex);
-	}
-      END_CATCH
+  TRY
+    {
+      pc = get_frame_address_in_block (frame);
+      pc_p = 1;
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
 
-      if (pc_p)
-	{
-	  struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
+  if (pc_p)
+    {
+      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
 
-	  if (cust != NULL)
-	    return compunit_language (cust);
-	}
+      if (cust != NULL)
+	return compunit_language (cust);
     }
 
   return language_unknown;
diff --git a/gdb/top.c b/gdb/top.c
index 3e88ac6..d17891e 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -329,10 +329,11 @@ void
 check_frame_language_change (void)
 {
   static int warned = 0;
+  struct frame_info *frame;
 
   /* First make sure that a new frame has been selected, in case the
      command or the hooks changed the program state.  */
-  deprecated_safe_get_selected_frame ();
+  frame = deprecated_safe_get_selected_frame ();
   if (current_language != expected_language)
     {
       if (language_mode == language_mode_auto && info_verbose)
@@ -348,11 +349,11 @@ check_frame_language_change (void)
   /* FIXME: This should be cacheing the frame and only running when
      the frame changes.  */
 
-  if (has_stack_frames ())
+  if (has_stack_frames () && frame != NULL)
     {
       enum language flang;
 
-      flang = get_frame_language ();
+      flang = get_frame_language (frame);
       if (!warned
 	  && flang != language_unknown
 	  && flang != current_language->la_language)
-- 
2.4.0

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

* [PATCH 0/2] Make get_frame_langauge take a frame argument
@ 2015-08-04 15:40 Andrew Burgess
  2015-08-04 15:40 ` [PATCH 1/2] gdb: get_frame_language now takes a frame parameter Andrew Burgess
  2015-08-04 15:40 ` [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c Andrew Burgess
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Burgess @ 2015-08-04 15:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

This two patch series was a result of looking into getting rid of
deprecated_safe_get_selected_frame.  I don't remove the function in
this patch set, in fact I only remove one use, inside
get_frame_langauge, however, I think the change is a good one and can
be pushed regardless of any other work in this area.

The first patch makes get_frame_langauge take a frame parameter,
rather than just operating on the currently selected frame.

The second patch moves get_frame_langauge into frame.c, which I think
is a better place for it.  If folk disagree then I'm happy to leave
the function where it is.

Tested on x86-64 Linux only with no regressions.

OK to apply?

Thanks,
Andrew

--

Andrew Burgess (2):
  gdb: get_frame_language now takes a frame parameter.
  gdb: Move get_frame_language from stack.c to frame.c.

 gdb/ChangeLog  | 21 +++++++++++++++++++++
 gdb/frame.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/frame.h    |  6 ++++++
 gdb/language.c | 34 ++++++++++++++++++++++++++--------
 gdb/language.h |  2 --
 gdb/stack.c    | 45 ---------------------------------------------
 gdb/top.c      |  8 +++++---
 7 files changed, 100 insertions(+), 58 deletions(-)

-- 
2.4.0

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

* [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c.
  2015-08-04 15:40 [PATCH 0/2] Make get_frame_langauge take a frame argument Andrew Burgess
  2015-08-04 15:40 ` [PATCH 1/2] gdb: get_frame_language now takes a frame parameter Andrew Burgess
@ 2015-08-04 15:40 ` Andrew Burgess
  2015-08-04 16:02   ` Pedro Alves
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Burgess @ 2015-08-04 15:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

The get_frame_language feels like it would be more at home in frame.c
rather than in stack.c, while the declaration, that is currently in
language.h can be moved into frame.h to match.

A couple of new includes are added, but otherwise no substantial change
here.

gdb/ChangeLog:

	* stack.c (get_frame_language): Moved ...
	* frame.c (get_frame_language): ... to here.
	* language.h (get_frame_language): Declaration moved ...
	* frame.h (get_frame_language): ... to here.
	(enum language): Add declaration.
	* language.c: Add frame.h include.
	* top.c: Add frame.h include.
---
 gdb/ChangeLog  | 10 ++++++++++
 gdb/frame.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/frame.h    |  6 ++++++
 gdb/language.c |  1 +
 gdb/language.h |  2 --
 gdb/stack.c    | 42 ------------------------------------------
 gdb/top.c      |  1 +
 7 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 223d2e5..f383dd7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* stack.c (get_frame_language): Moved ...
+	* frame.c (get_frame_language): ... to here.
+	* language.h (get_frame_language): Declaration moved ...
+	* frame.h (get_frame_language): ... to here.
+	(enum language): Add declaration.
+	* language.c: Add frame.h include.
+	* top.c: Add frame.h include.
+
+2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* language.c (show_language_command): Find selected frame before
 	asking for the language of that frame.
 	(set_language_command): Likewise.
diff --git a/gdb/frame.c b/gdb/frame.c
index da5bfb9..f05f739 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2571,6 +2571,48 @@ frame_unwind_caller_arch (struct frame_info *next_frame)
   return frame_unwind_arch (skip_artificial_frames (next_frame));
 }
 
+/* Gets the language of FRAME.  */
+
+enum language
+get_frame_language (struct frame_info *frame)
+{
+  CORE_ADDR pc = 0;
+  int pc_p = 0;
+
+  gdb_assert (frame!= NULL);
+
+    /* We determine the current frame language by looking up its
+       associated symtab.  To retrieve this symtab, we use the frame
+       PC.  However we cannot use the frame PC as is, because it
+       usually points to the instruction following the "call", which
+       is sometimes the first instruction of another function.  So
+       we rely on get_frame_address_in_block(), it provides us with
+       a PC that is guaranteed to be inside the frame's code
+       block.  */
+
+  TRY
+    {
+      pc = get_frame_address_in_block (frame);
+      pc_p = 1;
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
+
+  if (pc_p)
+    {
+      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
+
+      if (cust != NULL)
+	return compunit_language (cust);
+    }
+
+  return language_unknown;
+}
+
 /* Stack pointer methods.  */
 
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index be64c57..6c39b86 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -814,4 +814,10 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 extern int frame_unwinder_is (struct frame_info *fi,
 			      const struct frame_unwind *unwinder);
 
+/* Return the language of FRAME.  */
+
+enum language;
+extern enum language get_frame_language (struct frame_info *frame);
+
+
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/language.c b/gdb/language.c
index 989a8da..b130f50 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -43,6 +43,7 @@
 #include "demangle.h"
 #include "symfile.h"
 #include "cp-support.h"
+#include "frame.h"
 
 extern void _initialize_language (void);
 
diff --git a/gdb/language.h b/gdb/language.h
index 8782ef0..2265afc 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -544,8 +544,6 @@ extern const char *language_str (enum language);
 
 extern void add_language (const struct language_defn *);
 
-extern enum language get_frame_language (struct frame_info *frame);	/* In stack.c */
-
 /* Check for a language-specific trampoline.  */
 
 extern CORE_ADDR skip_language_trampoline (struct frame_info *, CORE_ADDR pc);
diff --git a/gdb/stack.c b/gdb/stack.c
index 31a723d..ae53ec8 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2559,48 +2559,6 @@ func_command (char *arg, int from_tty)
   else if (frame != get_selected_frame (NULL))
     select_and_print_frame (frame);
 }
-
-/* Gets the language of FRAME.  */
-
-enum language
-get_frame_language (struct frame_info *frame)
-{
-  CORE_ADDR pc = 0;
-  int pc_p = 0;
-
-  gdb_assert (frame!= NULL);
-
-    /* We determine the current frame language by looking up its
-       associated symtab.  To retrieve this symtab, we use the frame
-       PC.  However we cannot use the frame PC as is, because it
-       usually points to the instruction following the "call", which
-       is sometimes the first instruction of another function.  So
-       we rely on get_frame_address_in_block(), it provides us with
-       a PC that is guaranteed to be inside the frame's code
-       block.  */
-
-  TRY
-    {
-      pc = get_frame_address_in_block (frame);
-      pc_p = 1;
-    }
-  CATCH (ex, RETURN_MASK_ERROR)
-    {
-      if (ex.error != NOT_AVAILABLE_ERROR)
-	throw_exception (ex);
-    }
-  END_CATCH
-
-  if (pc_p)
-    {
-      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
-
-      if (cust != NULL)
-	return compunit_language (cust);
-    }
-
-  return language_unknown;
-}
 \f
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/top.c b/gdb/top.c
index d17891e..364ca88 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -49,6 +49,7 @@
 #include "observer.h"
 #include "maint.h"
 #include "filenames.h"
+#include "frame.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
-- 
2.4.0

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

* Re: [PATCH 1/2] gdb: get_frame_language now takes a frame parameter.
  2015-08-04 15:40 ` [PATCH 1/2] gdb: get_frame_language now takes a frame parameter Andrew Burgess
@ 2015-08-04 16:02   ` Pedro Alves
  2015-08-07  8:56     ` Andrew Burgess
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2015-08-04 16:02 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 08/04/2015 04:40 PM, Andrew Burgess wrote:
> As part of a drive to remove deprecated_safe_get_selected_frame, make
> the get_frame_language function take a frame parameter.  Given the name
> of the function this actually seems to make a lot of sense.
> 
> The task of fetching a suitable frame is then passed to the calling
> functions.  For get_frame_language there are not many callers, these are
> updated to get the selected frame in a suitable way.

Thanks.

> 
> gdb/ChangeLog:
> 
> 	* language.c (show_language_command): Find selected frame before
> 	asking for the language of that frame.
> 	(set_language_command): Likewise.
> 	* language.h (get_frame_language): Add frame parameter.
> 	* stack.c (get_frame_language): Add frame parameter, assert
> 	parameter is not NULL, update comment and reindent.
> 	* top.c (check_frame_language_change): Pass the selected frame
> 	into get_frame_language.
> ---
>  gdb/ChangeLog  | 11 +++++++++++
>  gdb/language.c | 33 +++++++++++++++++++++++--------
>  gdb/language.h |  2 +-
>  gdb/stack.c    | 61 ++++++++++++++++++++++++++++------------------------------
>  gdb/top.c      |  7 ++++---
>  5 files changed, 70 insertions(+), 44 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index af41072..223d2e5 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,14 @@
> +2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* language.c (show_language_command): Find selected frame before
> +	asking for the language of that frame.
> +	(set_language_command): Likewise.
> +	* language.h (get_frame_language): Add frame parameter.
> +	* stack.c (get_frame_language): Add frame parameter, assert
> +	parameter is not NULL, update comment and reindent.
> +	* top.c (check_frame_language_change): Pass the selected frame
> +	into get_frame_language.
> +
>  2015-08-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
>  
>  	* infcmd.c (signal_command): Call do_cleanups for args_chain.
> diff --git a/gdb/language.c b/gdb/language.c
> index a8b432e..989a8da 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -118,7 +118,8 @@ static void
>  show_language_command (struct ui_file *file, int from_tty,
>  		       struct cmd_list_element *c, const char *value)
>  {
> -  enum language flang;		/* The language of the current frame.  */
> +  struct frame_info *frame;
> +  enum language flang;		/* The language of the frame.  */
>  
>    if (language_mode == language_mode_auto)
>      fprintf_filtered (gdb_stdout,
> @@ -130,11 +131,15 @@ show_language_command (struct ui_file *file, int from_tty,
>  		      _("The current source language is \"%s\".\n"),
>  		      current_language->la_name);
>  
> -  flang = get_frame_language ();
> -  if (flang != language_unknown &&
> -      language_mode == language_mode_manual &&
> -      current_language->la_language != flang)
> -    printf_filtered ("%s\n", lang_frame_mismatch_warn);
> +  if (target_has_execution)

target_has_execution can't be right.  What about core files?

> +    {
> +      frame = get_selected_frame (NULL);
> +      flang = get_frame_language (frame);
> +      if (flang != language_unknown &&
> +	  language_mode == language_mode_manual &&
> +	  current_language->la_language != flang)

Please put the &&'s at the beginning of the next line while at it.

> +	printf_filtered ("%s\n", lang_frame_mismatch_warn);
> +    }
>  }
>  


> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -329,10 +329,11 @@ void
>  check_frame_language_change (void)
>  {
>    static int warned = 0;
> +  struct frame_info *frame;
>  
>    /* First make sure that a new frame has been selected, in case the
>       command or the hooks changed the program state.  */
> -  deprecated_safe_get_selected_frame ();
> +  frame = deprecated_safe_get_selected_frame ();
>    if (current_language != expected_language)
>      {
>        if (language_mode == language_mode_auto && info_verbose)
> @@ -348,11 +349,11 @@ check_frame_language_change (void)
>    /* FIXME: This should be cacheing the frame and only running when
>       the frame changes.  */
>  
> -  if (has_stack_frames ())
> +  if (has_stack_frames () && frame != NULL)

If has_stack_frames() returns true, how can FRAME be NULL?

>      {
>        enum language flang;
>  
> -      flang = get_frame_language ();
> +      flang = get_frame_language (frame);
>        if (!warned
>  	  && flang != language_unknown
>  	  && flang != current_language->la_language)
> 

Thanks,
Pedro Alves

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

* Re: [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c.
  2015-08-04 15:40 ` [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c Andrew Burgess
@ 2015-08-04 16:02   ` Pedro Alves
  2015-08-07  8:57     ` Andrew Burgess
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2015-08-04 16:02 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 08/04/2015 04:40 PM, Andrew Burgess wrote:

> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -814,4 +814,10 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
>  extern int frame_unwinder_is (struct frame_info *fi,
>  			      const struct frame_unwind *unwinder);
>  
> +/* Return the language of FRAME.  */
> +
> +enum language;

No new forward declarations of enums please.  That doesn't work in C++.

Thanks,
Pedro Alves

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

* Re: [PATCH 1/2] gdb: get_frame_language now takes a frame parameter.
  2015-08-04 16:02   ` Pedro Alves
@ 2015-08-07  8:56     ` Andrew Burgess
  2015-08-07  9:45       ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Burgess @ 2015-08-07  8:56 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

* Pedro Alves <palves@redhat.com> [2015-08-04 17:02:42 +0100]:

> On 08/04/2015 04:40 PM, Andrew Burgess wrote:

> >
> > -  flang = get_frame_language ();
> > -  if (flang != language_unknown &&
> > -      language_mode == language_mode_manual &&
> > -      current_language->la_language != flang)
> > -    printf_filtered ("%s\n", lang_frame_mismatch_warn);
> > +  if (target_has_execution)
> 
> target_has_execution can't be right.  What about core files?

Switched to has_stack_frames, which, given we're working with stack
frames seems like a much better check.

> 
> > +    {
> > +      frame = get_selected_frame (NULL);
> > +      flang = get_frame_language (frame);
> > +      if (flang != language_unknown &&
> > +	  language_mode == language_mode_manual &&
> > +	  current_language->la_language != flang)
> 
> Please put the &&'s at the beginning of the next line while at it.

Done.

> >  
> >    /* First make sure that a new frame has been selected, in case the
> >       command or the hooks changed the program state.  */
> > -  deprecated_safe_get_selected_frame ();
> > +  frame = deprecated_safe_get_selected_frame ();
> >    if (current_language != expected_language)
> >      {
> >        if (language_mode == language_mode_auto && info_verbose)
> > @@ -348,11 +349,11 @@ check_frame_language_change (void)
> >    /* FIXME: This should be cacheing the frame and only running when
> >       the frame changes.  */
> >  
> > -  if (has_stack_frames ())
> > +  if (has_stack_frames () && frame != NULL)
> 
> If has_stack_frames() returns true, how can FRAME be NULL?

Good point. Pointless check removed.

New version below.

Thanks,
Andrew

---

As part of a drive to remove deprecated_safe_get_selected_frame, make
the get_frame_language function take a frame parameter.  Given the name
of the function this actually seems to make a lot of sense.

The task of fetching a suitable frame is then passed to the calling
functions.  For get_frame_language there are not many callers, these are
updated to get the selected frame in a suitable way.

gdb/ChangeLog:

	* language.c (show_language_command): Find selected frame before
	asking for the language of that frame.
	(set_language_command): Likewise.
	* language.h (get_frame_language): Add frame parameter.
	* stack.c (get_frame_language): Add frame parameter, assert
	parameter is not NULL, update comment and reindent.
	* top.c (check_frame_language_change): Pass the selected frame
	into get_frame_language.
---
 gdb/ChangeLog  | 11 +++++++++++
 gdb/language.c | 33 +++++++++++++++++++++++--------
 gdb/language.h |  2 +-
 gdb/stack.c    | 61 ++++++++++++++++++++++++++++------------------------------
 gdb/top.c      |  5 +++--
 5 files changed, 69 insertions(+), 43 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index af41072..223d2e5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* language.c (show_language_command): Find selected frame before
+	asking for the language of that frame.
+	(set_language_command): Likewise.
+	* language.h (get_frame_language): Add frame parameter.
+	* stack.c (get_frame_language): Add frame parameter, assert
+	parameter is not NULL, update comment and reindent.
+	* top.c (check_frame_language_change): Pass the selected frame
+	into get_frame_language.
+
 2015-08-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* infcmd.c (signal_command): Call do_cleanups for args_chain.
diff --git a/gdb/language.c b/gdb/language.c
index a8b432e..75d4497 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -118,7 +118,8 @@ static void
 show_language_command (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  enum language flang;		/* The language of the current frame.  */
+  struct frame_info *frame;
+  enum language flang;		/* The language of the frame.  */
 
   if (language_mode == language_mode_auto)
     fprintf_filtered (gdb_stdout,
@@ -130,11 +131,15 @@ show_language_command (struct ui_file *file, int from_tty,
 		      _("The current source language is \"%s\".\n"),
 		      current_language->la_name);
 
-  flang = get_frame_language ();
-  if (flang != language_unknown &&
-      language_mode == language_mode_manual &&
-      current_language->la_language != flang)
-    printf_filtered ("%s\n", lang_frame_mismatch_warn);
+  if (has_stack_frames ())
+    {
+      frame = get_selected_frame (NULL);
+      flang = get_frame_language (frame);
+      if (flang != language_unknown
+	  && language_mode == language_mode_manual
+	  && current_language->la_language != flang)
+	printf_filtered ("%s\n", lang_frame_mismatch_warn);
+    }
 }
 
 /* Set command.  Change the current working language.  */
@@ -142,7 +147,7 @@ static void
 set_language_command (char *ignore, int from_tty, struct cmd_list_element *c)
 {
   int i;
-  enum language flang;
+  enum language flang = language_unknown;
 
   /* Search the list of languages for a match.  */
   for (i = 0; i < languages_size; i++)
@@ -155,7 +160,19 @@ set_language_command (char *ignore, int from_tty, struct cmd_list_element *c)
 	      /* Enter auto mode.  Set to the current frame's language, if
                  known, or fallback to the initial language.  */
 	      language_mode = language_mode_auto;
-	      flang = get_frame_language ();
+	      TRY
+		{
+		  struct frame_info *frame;
+
+		  frame = get_selected_frame (NULL);
+		  flang = get_frame_language (frame);
+		}
+	      CATCH (ex, RETURN_MASK_ERROR)
+		{
+		  flang = language_unknown;
+		}
+	      END_CATCH
+
 	      if (flang != language_unknown)
 		set_language (flang);
 	      else
diff --git a/gdb/language.h b/gdb/language.h
index 4ecb103..8782ef0 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -544,7 +544,7 @@ extern const char *language_str (enum language);
 
 extern void add_language (const struct language_defn *);
 
-extern enum language get_frame_language (void);	/* In stack.c */
+extern enum language get_frame_language (struct frame_info *frame);	/* In stack.c */
 
 /* Check for a language-specific trampoline.  */
 
diff --git a/gdb/stack.c b/gdb/stack.c
index b4cfdbd..31a723d 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2560,46 +2560,43 @@ func_command (char *arg, int from_tty)
     select_and_print_frame (frame);
 }
 
-/* Gets the language of the current frame.  */
+/* Gets the language of FRAME.  */
 
 enum language
-get_frame_language (void)
+get_frame_language (struct frame_info *frame)
 {
-  struct frame_info *frame = deprecated_safe_get_selected_frame ();
+  CORE_ADDR pc = 0;
+  int pc_p = 0;
 
-  if (frame)
-    {
-      CORE_ADDR pc = 0;
-      int pc_p = 0;
+  gdb_assert (frame!= NULL);
 
-      /* We determine the current frame language by looking up its
-         associated symtab.  To retrieve this symtab, we use the frame
-         PC.  However we cannot use the frame PC as is, because it
-         usually points to the instruction following the "call", which
-         is sometimes the first instruction of another function.  So
-         we rely on get_frame_address_in_block(), it provides us with
-         a PC that is guaranteed to be inside the frame's code
-         block.  */
+    /* We determine the current frame language by looking up its
+       associated symtab.  To retrieve this symtab, we use the frame
+       PC.  However we cannot use the frame PC as is, because it
+       usually points to the instruction following the "call", which
+       is sometimes the first instruction of another function.  So
+       we rely on get_frame_address_in_block(), it provides us with
+       a PC that is guaranteed to be inside the frame's code
+       block.  */
 
-      TRY
-	{
-	  pc = get_frame_address_in_block (frame);
-	  pc_p = 1;
-	}
-      CATCH (ex, RETURN_MASK_ERROR)
-	{
-	  if (ex.error != NOT_AVAILABLE_ERROR)
-	    throw_exception (ex);
-	}
-      END_CATCH
+  TRY
+    {
+      pc = get_frame_address_in_block (frame);
+      pc_p = 1;
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
 
-      if (pc_p)
-	{
-	  struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
+  if (pc_p)
+    {
+      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
 
-	  if (cust != NULL)
-	    return compunit_language (cust);
-	}
+      if (cust != NULL)
+	return compunit_language (cust);
     }
 
   return language_unknown;
diff --git a/gdb/top.c b/gdb/top.c
index 3e88ac6..0130acf 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -329,10 +329,11 @@ void
 check_frame_language_change (void)
 {
   static int warned = 0;
+  struct frame_info *frame;
 
   /* First make sure that a new frame has been selected, in case the
      command or the hooks changed the program state.  */
-  deprecated_safe_get_selected_frame ();
+  frame = deprecated_safe_get_selected_frame ();
   if (current_language != expected_language)
     {
       if (language_mode == language_mode_auto && info_verbose)
@@ -352,7 +353,7 @@ check_frame_language_change (void)
     {
       enum language flang;
 
-      flang = get_frame_language ();
+      flang = get_frame_language (frame);
       if (!warned
 	  && flang != language_unknown
 	  && flang != current_language->la_language)
-- 
2.4.0

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

* Re: [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c.
  2015-08-04 16:02   ` Pedro Alves
@ 2015-08-07  8:57     ` Andrew Burgess
  2015-08-07  9:45       ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Burgess @ 2015-08-07  8:57 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

* Pedro Alves <palves@redhat.com> [2015-08-04 17:02:49 +0100]:

> On 08/04/2015 04:40 PM, Andrew Burgess wrote:
> 
> > --- a/gdb/frame.h
> > +++ b/gdb/frame.h
> > @@ -814,4 +814,10 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
> >  extern int frame_unwinder_is (struct frame_info *fi,
> >  			      const struct frame_unwind *unwinder);
> >  
> > +/* Return the language of FRAME.  */
> > +
> > +enum language;
> 
> No new forward declarations of enums please.  That doesn't work in C++.

OK.

New version includes the language.h header file, which showed up a few
places where we were missing forward declarations of structs,
otherwise, no significant changes.

Thanks,
Andrew

--

The get_frame_language feels like it would be more at home in frame.c
rather than in stack.c, while the declaration, that is currently in
language.h can be moved into frame.h to match.

A couple of new includes are added, but otherwise no substantial change
here.

gdb/ChangeLog:

	* stack.c (get_frame_language): Moved ...
	* frame.c (get_frame_language): ... to here.
	* language.h (get_frame_language): Declaration moved to frame.h.
	* frame.h: Add language.h include, for language enum.
	(get_frame_language): Declaration moved from language.h.
	* language.c: Add frame.h include.
	* top.c: Add frame.h include.
	* symtab.h (struct obj_section): Declare.
	(struct cmd_list_element): Declare.
---
 gdb/ChangeLog  | 12 ++++++++++++
 gdb/frame.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/frame.h    |  7 +++++++
 gdb/language.c |  1 +
 gdb/language.h |  2 --
 gdb/stack.c    | 42 ------------------------------------------
 gdb/symtab.h   |  2 ++
 gdb/top.c      |  1 +
 8 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 223d2e5..d510c3d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* stack.c (get_frame_language): Moved ...
+	* frame.c (get_frame_language): ... to here.
+	* language.h (get_frame_language): Declaration moved to frame.h.
+	* frame.h: Add language.h include, for language enum.
+	(get_frame_language): Declaration moved from language.h.
+	* language.c: Add frame.h include.
+	* top.c: Add frame.h include.
+	* symtab.h (struct obj_section): Declare.
+	(struct cmd_list_element): Declare.
+
+2015-08-04  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* language.c (show_language_command): Find selected frame before
 	asking for the language of that frame.
 	(set_language_command): Likewise.
diff --git a/gdb/frame.c b/gdb/frame.c
index da5bfb9..f05f739 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2571,6 +2571,48 @@ frame_unwind_caller_arch (struct frame_info *next_frame)
   return frame_unwind_arch (skip_artificial_frames (next_frame));
 }
 
+/* Gets the language of FRAME.  */
+
+enum language
+get_frame_language (struct frame_info *frame)
+{
+  CORE_ADDR pc = 0;
+  int pc_p = 0;
+
+  gdb_assert (frame!= NULL);
+
+    /* We determine the current frame language by looking up its
+       associated symtab.  To retrieve this symtab, we use the frame
+       PC.  However we cannot use the frame PC as is, because it
+       usually points to the instruction following the "call", which
+       is sometimes the first instruction of another function.  So
+       we rely on get_frame_address_in_block(), it provides us with
+       a PC that is guaranteed to be inside the frame's code
+       block.  */
+
+  TRY
+    {
+      pc = get_frame_address_in_block (frame);
+      pc_p = 1;
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (ex.error != NOT_AVAILABLE_ERROR)
+	throw_exception (ex);
+    }
+  END_CATCH
+
+  if (pc_p)
+    {
+      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
+
+      if (cust != NULL)
+	return compunit_language (cust);
+    }
+
+  return language_unknown;
+}
+
 /* Stack pointer methods.  */
 
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index be64c57..03f3892 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -69,6 +69,8 @@
 
    */
 
+#include "language.h"
+
 struct symtab_and_line;
 struct frame_unwind;
 struct frame_base;
@@ -814,4 +816,9 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 extern int frame_unwinder_is (struct frame_info *fi,
 			      const struct frame_unwind *unwinder);
 
+/* Return the language of FRAME.  */
+
+extern enum language get_frame_language (struct frame_info *frame);
+
+
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/language.c b/gdb/language.c
index 75d4497..715efe5 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -43,6 +43,7 @@
 #include "demangle.h"
 #include "symfile.h"
 #include "cp-support.h"
+#include "frame.h"
 
 extern void _initialize_language (void);
 
diff --git a/gdb/language.h b/gdb/language.h
index 8782ef0..2265afc 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -544,8 +544,6 @@ extern const char *language_str (enum language);
 
 extern void add_language (const struct language_defn *);
 
-extern enum language get_frame_language (struct frame_info *frame);	/* In stack.c */
-
 /* Check for a language-specific trampoline.  */
 
 extern CORE_ADDR skip_language_trampoline (struct frame_info *, CORE_ADDR pc);
diff --git a/gdb/stack.c b/gdb/stack.c
index 31a723d..ae53ec8 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2559,48 +2559,6 @@ func_command (char *arg, int from_tty)
   else if (frame != get_selected_frame (NULL))
     select_and_print_frame (frame);
 }
-
-/* Gets the language of FRAME.  */
-
-enum language
-get_frame_language (struct frame_info *frame)
-{
-  CORE_ADDR pc = 0;
-  int pc_p = 0;
-
-  gdb_assert (frame!= NULL);
-
-    /* We determine the current frame language by looking up its
-       associated symtab.  To retrieve this symtab, we use the frame
-       PC.  However we cannot use the frame PC as is, because it
-       usually points to the instruction following the "call", which
-       is sometimes the first instruction of another function.  So
-       we rely on get_frame_address_in_block(), it provides us with
-       a PC that is guaranteed to be inside the frame's code
-       block.  */
-
-  TRY
-    {
-      pc = get_frame_address_in_block (frame);
-      pc_p = 1;
-    }
-  CATCH (ex, RETURN_MASK_ERROR)
-    {
-      if (ex.error != NOT_AVAILABLE_ERROR)
-	throw_exception (ex);
-    }
-  END_CATCH
-
-  if (pc_p)
-    {
-      struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
-
-      if (cust != NULL)
-	return compunit_language (cust);
-    }
-
-  return language_unknown;
-}
 \f
 
 /* Provide a prototype to silence -Wmissing-prototypes.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 61fc8c5..e90ce00 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -38,6 +38,8 @@ struct program_space;
 struct language_defn;
 struct probe;
 struct common_block;
+struct obj_section;
+struct cmd_list_element;
 
 /* Some of the structures in this file are space critical.
    The space-critical structures are:
diff --git a/gdb/top.c b/gdb/top.c
index 0130acf..061b52f 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -49,6 +49,7 @@
 #include "observer.h"
 #include "maint.h"
 #include "filenames.h"
+#include "frame.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
-- 
2.4.0

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

* Re: [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c.
  2015-08-07  8:57     ` Andrew Burgess
@ 2015-08-07  9:45       ` Pedro Alves
  0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2015-08-07  9:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 08/07/2015 09:57 AM, Andrew Burgess wrote:

> New version includes the language.h header file, which showed up a few
> places where we were missing forward declarations of structs,
> otherwise, no significant changes.
> 

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH 1/2] gdb: get_frame_language now takes a frame parameter.
  2015-08-07  8:56     ` Andrew Burgess
@ 2015-08-07  9:45       ` Pedro Alves
  0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2015-08-07  9:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 08/07/2015 09:55 AM, Andrew Burgess wrote:

> New version below.

OK, with ...

>  
>  	* infcmd.c (signal_command): Call do_cleanups for args_chain.
> diff --git a/gdb/language.c b/gdb/language.c
> index a8b432e..75d4497 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -118,7 +118,8 @@ static void
>  show_language_command (struct ui_file *file, int from_tty,
>  		       struct cmd_list_element *c, const char *value)
>  {
> -  enum language flang;		/* The language of the current frame.  */
> +  struct frame_info *frame;

This new variable is only used in the has_stack_frames block, so move the
declaration there please.

Thanks,
Pedro Alves

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

end of thread, other threads:[~2015-08-07  9:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-04 15:40 [PATCH 0/2] Make get_frame_langauge take a frame argument Andrew Burgess
2015-08-04 15:40 ` [PATCH 1/2] gdb: get_frame_language now takes a frame parameter Andrew Burgess
2015-08-04 16:02   ` Pedro Alves
2015-08-07  8:56     ` Andrew Burgess
2015-08-07  9:45       ` Pedro Alves
2015-08-04 15:40 ` [PATCH 2/2] gdb: Move get_frame_language from stack.c to frame.c Andrew Burgess
2015-08-04 16:02   ` Pedro Alves
2015-08-07  8:57     ` Andrew Burgess
2015-08-07  9:45       ` Pedro Alves

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).