[RFA] Don't unwind past entry point

[RFA] Don't unwind past entry point

[Randolph Chung]

Here's a proposed patch to fix the problem i mentioned in
http://sources.redhat.com/ml/gdb/2004-11/msg00020.html

Briefly, if a compiler does a tail-call optimization to main, a function
called from main will return directly to the caller of main, and we
never terminate a backtrace.

It doesn't address Joel's problem with threads though.

is this ok?

randolph


2004-11-09  Randolph Chung  <tausq@debian.org>

	* frame.c (backtrace_past_entry): New flag.
	(get_prev_frame): Stop backtrace at the entry function if enabled
	by flag.  Update comments.
	(_initialize_frame): Add command to set backtrace_past_entry flag.

	doc/
	* gdb.texinfo: Document set/show backtrace past-entry commands.
	Rearrange index entries for set/show backtrace past-main.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.192
diff -u -p -r1.192 frame.c
--- frame.c	29 Oct 2004 20:23:06 -0000	1.192
+++ frame.c	9 Nov 2004 19:21:38 -0000
@@ -115,6 +115,7 @@ static int frame_debug;
 /* Flag to indicate whether backtraces should stop at main et.al.  */
 
 static int backtrace_past_main;
+static int backtrace_past_entry;
 static unsigned int backtrace_limit = UINT_MAX;
 
 static void
@@ -1213,8 +1214,6 @@ get_prev_frame (struct frame_info *this_
      dummy frame PCs typically land in the entry func.  Don't apply
      this test to the sentinel frame.  Sentinel frames should always
      be allowed to unwind.  */
-  /* NOTE: cagney/2003-02-25: Don't enable until someone has found
-     hard evidence that this is needed.  */
   /* NOTE: cagney/2003-07-07: Fixed a bug in inside_main_func() -
      wasn't checking for "main" in the minimal symbols.  With that
      fixed asm-source tests now stop in "main" instead of halting the
@@ -1227,13 +1226,12 @@ get_prev_frame (struct frame_info *this_
      I guess) to determine the address range of the start function.
      That should provide a far better stopper than the current
      heuristics.  */
-  /* NOTE: cagney/2003-07-15: Need to add a "set backtrace
-     beyond-entry-func" command so that this can be selectively
-     disabled.  */
-  if (0
-#if 0
-      && backtrace_beyond_entry_func
-#endif
+  /* NOTE: tausq/2004-10-09: this is needed if, for example, the compiler
+     applied tail-call optimizations to main so that a function called 
+     from main returns directly to the caller of main.  Since we don't
+     stop at main, we should at least stop at the entry point of the
+     application.  */
+  if (!backtrace_past_entry
       && this_frame->unwind->type != DUMMY_FRAME && this_frame->level >= 0
       && inside_entry_func (this_frame))
     {
@@ -1531,6 +1529,17 @@ Whether backtraces should continue past 
 			   NULL, NULL, &set_backtrace_cmdlist,
 			   &show_backtrace_cmdlist);
 
+  add_setshow_boolean_cmd ("past-entry", class_obscure,
+			   &backtrace_past_entry, "\
+Set whether backtraces should continue past the entry point of a program.", "\
+Show whether backtraces should continue past the entry point of a program.", "\
+Normally there are no callers beyond the entry point of a program, so GDB\n\
+will terminate the backtrace there.  Set this variable if you need to see \n\
+the rest of the stack trace.", "\
+Whether backtraces should continue past the entry point is %s.",
+			   NULL, NULL, &set_backtrace_cmdlist,
+			   &show_backtrace_cmdlist);
+
   add_setshow_uinteger_cmd ("limit", class_obscure,
 			    &backtrace_limit, "\
 Set an upper bound on the number of backtrace levels.", "\
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.223
diff -u -p -r1.223 gdb.texinfo
--- doc/gdb.texinfo	8 Nov 2004 17:25:49 -0000	1.223
+++ doc/gdb.texinfo	9 Nov 2004 19:21:47 -0000
@@ -4109,7 +4109,7 @@ in a backtrace, you can change this beha
 @table @code
 @item set backtrace past-main
 @itemx set backtrace past-main on
-@kindex set backtrace
+@kindex set backtrace past-main
 Backtraces will continue past the user entry point.
 
 @item set backtrace past-main off
@@ -4117,9 +4117,24 @@ Backtraces will stop when they encounter
 default.
 
 @item show backtrace past-main
-@kindex show backtrace
+@kindex show backtrace past-main
 Display the current user entry point backtrace policy.
 
+@item set backtrace past-entry
+@itemx set backtrace past-entry on
+@kindex set backtrace past-entry
+Backtraces will continue past the internal entry point of an application. 
+This entry point is encoded by the linker when the application is built,
+and is likely before the user entry point ``main'' (or equivalent) is called.
+
+@item set backtrace past-entry off
+Backtraces will stop when they encouter the internal entry point of an
+application.  This is the default.
+
+@item show backtrace past-entry
+@kindex show backtrace past-entry
+Display the current internal entry point backtrace policy.
+
 @item set backtrace limit @var{n}
 @itemx set backtrace limit 0
 @cindex backtrace limit

Re: [RFA] Don't unwind past entry point

[Andrew Cagney]

Randolph Chung wrote:
> Here's a proposed patch to fix the problem i mentioned in
> http://sources.redhat.com/ml/gdb/2004-11/msg00020.html
> 
> Briefly, if a compiler does a tail-call optimization to main, a function
> called from main will return directly to the caller of main, and we
> never terminate a backtrace.
> 
> It doesn't address Joel's problem with threads though.

Check eli for the doco.

The code change is ok (while your though though, can you do me a favour 
and move inside_entry_func to frame.c and make it static?).

This is missing a testcase but for that I'll need to take a raincheck - 
we've almost but not quite got a setbacktrace testcase and it should be 
extended to include this.  Can you watch for that.

Andrew


> randolph
> 
> 
> 2004-11-09  Randolph Chung  <tausq@debian.org>
> 
> 	* frame.c (backtrace_past_entry): New flag.
> 	(get_prev_frame): Stop backtrace at the entry function if enabled
> 	by flag.  Update comments.
> 	(_initialize_frame): Add command to set backtrace_past_entry flag.
> 
> 	doc/
> 	* gdb.texinfo: Document set/show backtrace past-entry commands.
> 	Rearrange index entries for set/show backtrace past-main.
> 
> Index: frame.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/frame.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 frame.c
> --- frame.c	29 Oct 2004 20:23:06 -0000	1.192
> +++ frame.c	9 Nov 2004 19:21:38 -0000
> @@ -115,6 +115,7 @@ static int frame_debug;
>  /* Flag to indicate whether backtraces should stop at main et.al.  */
>  
>  static int backtrace_past_main;
> +static int backtrace_past_entry;
>  static unsigned int backtrace_limit = UINT_MAX;
>  
>  static void
> @@ -1213,8 +1214,6 @@ get_prev_frame (struct frame_info *this_
>       dummy frame PCs typically land in the entry func.  Don't apply
>       this test to the sentinel frame.  Sentinel frames should always
>       be allowed to unwind.  */
> -  /* NOTE: cagney/2003-02-25: Don't enable until someone has found
> -     hard evidence that this is needed.  */
>    /* NOTE: cagney/2003-07-07: Fixed a bug in inside_main_func() -
>       wasn't checking for "main" in the minimal symbols.  With that
>       fixed asm-source tests now stop in "main" instead of halting the
> @@ -1227,13 +1226,12 @@ get_prev_frame (struct frame_info *this_
>       I guess) to determine the address range of the start function.
>       That should provide a far better stopper than the current
>       heuristics.  */
> -  /* NOTE: cagney/2003-07-15: Need to add a "set backtrace
> -     beyond-entry-func" command so that this can be selectively
> -     disabled.  */
> -  if (0
> -#if 0
> -      && backtrace_beyond_entry_func
> -#endif
> +  /* NOTE: tausq/2004-10-09: this is needed if, for example, the compiler
> +     applied tail-call optimizations to main so that a function called 
> +     from main returns directly to the caller of main.  Since we don't
> +     stop at main, we should at least stop at the entry point of the
> +     application.  */
> +  if (!backtrace_past_entry
>        && this_frame->unwind->type != DUMMY_FRAME && this_frame->level >= 0
>        && inside_entry_func (this_frame))
>      {
> @@ -1531,6 +1529,17 @@ Whether backtraces should continue past 
>  			   NULL, NULL, &set_backtrace_cmdlist,
>  			   &show_backtrace_cmdlist);
>  
> +  add_setshow_boolean_cmd ("past-entry", class_obscure,
> +			   &backtrace_past_entry, "\
> +Set whether backtraces should continue past the entry point of a program.", "\
> +Show whether backtraces should continue past the entry point of a program.", "\
> +Normally there are no callers beyond the entry point of a program, so GDB\n\
> +will terminate the backtrace there.  Set this variable if you need to see \n\
> +the rest of the stack trace.", "\
> +Whether backtraces should continue past the entry point is %s.",
> +			   NULL, NULL, &set_backtrace_cmdlist,
> +			   &show_backtrace_cmdlist);
> +
>    add_setshow_uinteger_cmd ("limit", class_obscure,
>  			    &backtrace_limit, "\
>  Set an upper bound on the number of backtrace levels.", "\
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.223
> diff -u -p -r1.223 gdb.texinfo
> --- doc/gdb.texinfo	8 Nov 2004 17:25:49 -0000	1.223
> +++ doc/gdb.texinfo	9 Nov 2004 19:21:47 -0000
> @@ -4109,7 +4109,7 @@ in a backtrace, you can change this beha
>  @table @code
>  @item set backtrace past-main
>  @itemx set backtrace past-main on
> -@kindex set backtrace
> +@kindex set backtrace past-main
>  Backtraces will continue past the user entry point.
>  
>  @item set backtrace past-main off
> @@ -4117,9 +4117,24 @@ Backtraces will stop when they encounter
>  default.
>  
>  @item show backtrace past-main
> -@kindex show backtrace
> +@kindex show backtrace past-main
>  Display the current user entry point backtrace policy.
>  
> +@item set backtrace past-entry
> +@itemx set backtrace past-entry on
> +@kindex set backtrace past-entry
> +Backtraces will continue past the internal entry point of an application. 
> +This entry point is encoded by the linker when the application is built,
> +and is likely before the user entry point ``main'' (or equivalent) is called.
> +
> +@item set backtrace past-entry off
> +Backtraces will stop when they encouter the internal entry point of an
> +application.  This is the default.
> +
> +@item show backtrace past-entry
> +@kindex show backtrace past-entry
> +Display the current internal entry point backtrace policy.
> +
>  @item set backtrace limit @var{n}
>  @itemx set backtrace limit 0
>  @cindex backtrace limit
> 

Re: [RFA] Don't unwind past entry point

[Randolph Chung]

> Check eli for the doco.

ok, i'll wait for Eli :)

> The code change is ok (while your though though, can you do me a favour 
> and move inside_entry_func to frame.c and make it static?).

will do.

> This is missing a testcase but for that I'll need to take a raincheck - 
> we've almost but not quite got a setbacktrace testcase and it should be 
> extended to include this.  Can you watch for that.

mmm.. this might be a bit tricky, as at least the original problem
depended on a particular compiler optimization that might not always
happen. but i can write up a testcase using the example i posted and
just use that...

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [RFA] Don't unwind past entry point

[Paul Gilliam]

On Wednesday 10 November 2004 09:07, Randolph Chung wrote:
-snip-
> > This is missing a testcase but for that I'll need to take a raincheck -
> > we've almost but not quite got a setbacktrace testcase and it should be
> > extended to include this.  Can you watch for that.
>
> mmm.. this might be a bit tricky, as at least the original problem
> depended on a particular compiler optimization that might not always
> happen. but i can write up a testcase using the example i posted and
> just use that...
>
> randolph

The setbacktrace testcase is waiting for approval to commit.  

Randolph,  I just looked at your example code and it seems easy enough to add 
to my setbacktrace.exp patch.

Should we wait till my patch adding setbacktrace.exp is accepted then do a new 
patch for this case, or modify my patch and try and get the modified patch 
accepted?

-=# Paul Gilliam #=-

Re: [RFA] Don't unwind past entry point

[Eli Zaretskii]

> Date: Wed, 10 Nov 2004 12:02:08 -0500
> From: Andrew Cagney <cagney@gnu.org>
> Cc: gdb-patches@sources.redhat.com
> 
> Check eli for the doco.

Thanks for the heads-up, I missed the original message.

>  @table @code
>  @item set backtrace past-main
>  @itemx set backtrace past-main on
> -@kindex set backtrace
> +@kindex set backtrace past-main

Please don't make this change.  When there are several commands that
begin with the same string, like "set backtrace SOMETHING" which all
begin with "set backtrace", we should always index only the common
part and do it only once, so that we end up with a single index entry
that points to the place where the whole family is described.

So please leave only the original "@kindex set backtrace" and "@kindex
show backtrace" entries, and remove all the rest.

> -@kindex show backtrace
> +@kindex show backtrace past-main

This change should also be undone.

> +This entry point is encoded by the linker when the application is built,
> +and is likely before the user entry point ``main'' (or equivalent) is called.

Please use @code{main}, without any quotes.  This produces `main' in
the Info manual and uses an appropriate typeface in the printed
version.

Otherwise, the doco patch is fine.  Thanks.