Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfa:symtab] deprecate inside_entry_func
@ 2003-11-01  0:07 Andrew Cagney
  2003-11-01  0:37 ` Kevin Buettner
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Andrew Cagney @ 2003-11-01  0:07 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 803 bytes --]

Hello,

This patch deprecates the function inside_entry_func.  Per the new comments:

+  /* NOTE: cagney/2003-10-31: A very simple test, such as
+     get_frame_func == entry_point should be sufficient for
+     identifying a pc in the entry function.  Does anyone know why it
+     wasn't sufficient and hence, why the very convoluted
+     "deprecated_inside_entry_func" is needed.  */
+  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
+     and implement a clean frame termination.  Not doing that is
+     really a bug in the ABI/crt0, and, hence, not a reason for
+     enabling the call to deprecated_inside_entry_func.  */

It's "evil twin", inside_entry_file, has already been deprecated.

ok?

Andrew

PS: Ref: http://sources.redhat.com/ml/gdb-patches/2003-10/msg00533.html

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 9061 bytes --]

2003-10-31  Andrew Cagney  <cagney@redhat.com>

	* objfiles.h (struct entry_info): Deprecate the fields
	"entry_func_lowpc", and "entry_func_lowpc".
	* symfile.c (init_entry_point_info): Update.
	* objfiles.c (objfile_relocate): Update.
	* dwarfread.c (read_func_scope): Update.
	* dwarf2read.c (read_func_scope): Update.
	* blockframe.c (deprecated_inside_entry_func): Update.

	* defs.h (deprecated_inside_entry_func): 
	"inside_entry_func".
	* frv-tdep.c (frv_frame_this_id): Update.
	* frame.c (get_prev_frame): Update.
	* blockframe.c (deprecated_inside_entry_func): Rename
	"inside_entry_func".
	(legacy_frame_chain_valid): Update.

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.82
diff -u -r1.82 blockframe.c
--- blockframe.c	20 Oct 2003 14:38:42 -0000	1.82
+++ blockframe.c	31 Oct 2003 22:56:10 -0000
@@ -172,7 +172,7 @@
    A PC of zero is always considered to be the bottom of the stack. */
 
 int
-inside_entry_func (CORE_ADDR pc)
+deprecated_inside_entry_func (CORE_ADDR pc)
 {
   if (pc == 0)
     return 1;
@@ -186,8 +186,8 @@
       if (DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0))
 	return 0;
     }
-  return (symfile_objfile->ei.entry_func_lowpc <= pc &&
-	  symfile_objfile->ei.entry_func_highpc > pc);
+  return (symfile_objfile->ei.deprecated_entry_func_lowpc <= pc &&
+	  symfile_objfile->ei.deprecated_entry_func_highpc > pc);
 }
 
 /* Return nonzero if the function for this frame lacks a prologue.  Many
@@ -615,7 +615,7 @@
 
   /* If we're already inside the entry function for the main objfile, then it
      isn't valid.  */
-  if (inside_entry_func (get_frame_pc (fi)))
+  if (deprecated_inside_entry_func (get_frame_pc (fi)))
     return 0;
 
   /* If we're inside the entry file, it isn't valid.  */
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.133
diff -u -r1.133 defs.h
--- defs.h	31 Oct 2003 19:19:51 -0000	1.133
+++ defs.h	31 Oct 2003 22:56:11 -0000
@@ -316,7 +316,7 @@
 
 /* From blockframe.c */
 
-extern int inside_entry_func (CORE_ADDR);
+extern int deprecated_inside_entry_func (CORE_ADDR);
 
 extern int deprecated_inside_entry_file (CORE_ADDR addr);
 
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.110
diff -u -r1.110 dwarf2read.c
--- dwarf2read.c	2 Oct 2003 17:13:16 -0000	1.110
+++ dwarf2read.c	31 Oct 2003 22:56:12 -0000
@@ -2174,8 +2174,8 @@
   if (objfile->ei.entry_point >= lowpc &&
       objfile->ei.entry_point < highpc)
     {
-      objfile->ei.entry_func_lowpc = lowpc;
-      objfile->ei.entry_func_highpc = highpc;
+      objfile->ei.deprecated_entry_func_lowpc = lowpc;
+      objfile->ei.deprecated_entry_func_highpc = highpc;
     }
 
   /* Decode DW_AT_frame_base location descriptor if present, keep result
Index: dwarfread.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarfread.c,v
retrieving revision 1.30
diff -u -r1.30 dwarfread.c
--- dwarfread.c	12 Sep 2003 18:40:16 -0000	1.30
+++ dwarfread.c	31 Oct 2003 22:56:12 -0000
@@ -1767,8 +1767,8 @@
   if (objfile->ei.entry_point >= dip->at_low_pc &&
       objfile->ei.entry_point < dip->at_high_pc)
     {
-      objfile->ei.entry_func_lowpc = dip->at_low_pc;
-      objfile->ei.entry_func_highpc = dip->at_high_pc;
+      objfile->ei.deprecated_entry_func_lowpc = dip->at_low_pc;
+      objfile->ei.deprecated_entry_func_highpc = dip->at_high_pc;
     }
   new = push_context (0, dip->at_low_pc);
   new->name = new_symbol (dip, objfile);
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.147
diff -u -r1.147 frame.c
--- frame.c	24 Oct 2003 17:37:03 -0000	1.147
+++ frame.c	31 Oct 2003 22:56:13 -0000
@@ -1820,10 +1820,10 @@
      asm-source tests now stop in "main" instead of halting the
      backtrace in wierd and wonderful ways somewhere inside the entry
      file.  Suspect that deprecated_inside_entry_file and
-     inside_entry_func tests were added to work around that (now
-     fixed) case.  */
+     deprecated_inside_entry_func tests were added to work around that
+     (now fixed) case.  */
   /* NOTE: cagney/2003-07-15: danielj (if I'm reading it right)
-     suggested having the inside_entry_func test use the
+     suggested having the deprecated_inside_entry_func test use the
      inside_main_func msymbol trick (along with entry_point_address I
      guess) to determine the address range of the start function.
      That should provide a far better stopper than the current
@@ -1831,12 +1831,21 @@
   /* NOTE: cagney/2003-07-15: Need to add a "set backtrace
      beyond-entry-func" command so that this can be selectively
      disabled.  */
+  /* NOTE: cagney/2003-10-31: A very simple test, such as
+     get_frame_func == entry_point should be sufficient for
+     identifying a pc in the entry function.  Does anyone know why it
+     wasn't sufficient and hence, why the very convoluted
+     "deprecated_inside_entry_func" is needed.  */
+  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
+     and implement a clean frame termination.  Not doing that is
+     really a bug in the ABI/crt0, and, hence, not a reason for
+     enabling the call to deprecated_inside_entry_func.  */
   if (0
 #if 0
       && backtrace_beyond_entry_func
 #endif
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
-      && inside_entry_func (get_frame_pc (this_frame)))
+      && deprecated_inside_entry_func (get_frame_pc (this_frame)))
     {
       if (frame_debug)
 	{
Index: frv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/frv-tdep.c,v
retrieving revision 1.57
diff -u -r1.57 frv-tdep.c
--- frv-tdep.c	27 Oct 2003 06:30:49 -0000	1.57
+++ frv-tdep.c	31 Oct 2003 22:56:13 -0000
@@ -996,7 +996,7 @@
 
   /* This is meant to halt the backtrace at "_start".  Make sure we
      don't halt it at a generic dummy frame. */
-  if (inside_entry_func (func))
+  if (deprecated_inside_entry_func (func))
     return;
 
   /* Check if the stack is empty.  */
Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.39
diff -u -r1.39 objfiles.c
--- objfiles.c	29 Oct 2003 18:29:07 -0000	1.39
+++ objfiles.c	31 Oct 2003 22:56:13 -0000
@@ -781,10 +781,10 @@
       }
   }
 
-  if (objfile->ei.entry_func_lowpc != INVALID_ENTRY_LOWPC)
+  if (objfile->ei.deprecated_entry_func_lowpc != INVALID_ENTRY_LOWPC)
     {
-      objfile->ei.entry_func_lowpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
-      objfile->ei.entry_func_highpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
+      objfile->ei.deprecated_entry_func_lowpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
+      objfile->ei.deprecated_entry_func_highpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
     }
 
   if (objfile->ei.deprecated_entry_file_lowpc != INVALID_ENTRY_LOWPC)
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.27
diff -u -r1.27 objfiles.h
--- objfiles.h	29 Oct 2003 18:29:07 -0000	1.27
+++ objfiles.h	31 Oct 2003 22:56:13 -0000
@@ -106,7 +106,7 @@
    #define DEPRECATED_FRAME_CHAIN_VALID(chain, thisframe)     \
    (chain != 0                                   \
    && !(inside_main_func ((thisframe)->pc))     \
-   && !(inside_entry_func ((thisframe)->pc)))
+   && !(deprecated_inside_entry_func ((thisframe)->pc)))
 
    and add initializations of the four scope controlling variables inside
    the object file / debugging information processing modules.  */
@@ -125,8 +125,8 @@
     /* Start (inclusive) and end (exclusive) of function containing the
        entry point. */
 
-    CORE_ADDR entry_func_lowpc;
-    CORE_ADDR entry_func_highpc;
+    CORE_ADDR deprecated_entry_func_lowpc;
+    CORE_ADDR deprecated_entry_func_highpc;
 
     /* Start (inclusive) and end (exclusive) of object file containing the
        entry point. */
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.111
diff -u -r1.111 symfile.c
--- symfile.c	29 Oct 2003 18:29:07 -0000	1.111
+++ symfile.c	31 Oct 2003 22:56:14 -0000
@@ -329,8 +329,8 @@
     }
   objfile->ei.deprecated_entry_file_lowpc = INVALID_ENTRY_LOWPC;
   objfile->ei.deprecated_entry_file_highpc = INVALID_ENTRY_HIGHPC;
-  objfile->ei.entry_func_lowpc = INVALID_ENTRY_LOWPC;
-  objfile->ei.entry_func_highpc = INVALID_ENTRY_HIGHPC;
+  objfile->ei.deprecated_entry_func_lowpc = INVALID_ENTRY_LOWPC;
+  objfile->ei.deprecated_entry_func_highpc = INVALID_ENTRY_HIGHPC;
   objfile->ei.main_func_lowpc = INVALID_ENTRY_LOWPC;
   objfile->ei.main_func_highpc = INVALID_ENTRY_HIGHPC;
 }

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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:07 [rfa:symtab] deprecate inside_entry_func Andrew Cagney
@ 2003-11-01  0:37 ` Kevin Buettner
  2003-11-01  0:46   ` Andrew Cagney
  2003-11-01  0:42 ` Daniel Jacobowitz
  2003-11-21 19:53 ` Andrew Cagney
  2 siblings, 1 reply; 25+ messages in thread
From: Kevin Buettner @ 2003-11-01  0:37 UTC (permalink / raw)
  To: Andrew Cagney, gdb-patches

On Oct 31,  7:07pm, Andrew Cagney wrote:

> This patch deprecates the function inside_entry_func.  Per the new comments:
> 
> +  /* NOTE: cagney/2003-10-31: A very simple test, such as
> +     get_frame_func == entry_point should be sufficient for
> +     identifying a pc in the entry function.  Does anyone know why it
> +     wasn't sufficient and hence, why the very convoluted
> +     "deprecated_inside_entry_func" is needed.  */
> +  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
> +     and implement a clean frame termination.  Not doing that is
> +     really a bug in the ABI/crt0, and, hence, not a reason for
> +     enabling the call to deprecated_inside_entry_func.  */

I do agree that it'd be nice if all ABIs provided a clean way to detect
the bottom-most frame.  Not all of them do however, and in such cases,
a mechanism like inside_entry_func() is necessary.

So, in short, I oppose the deprecation of this function.

> It's "evil twin", inside_entry_file, has already been deprecated.

I think that's okay.

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:07 [rfa:symtab] deprecate inside_entry_func Andrew Cagney
  2003-11-01  0:37 ` Kevin Buettner
@ 2003-11-01  0:42 ` Daniel Jacobowitz
  2003-11-01  2:37   ` Andrew Cagney
  2003-11-21 19:53 ` Andrew Cagney
  2 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-01  0:42 UTC (permalink / raw)
  To: gdb-patches

On Fri, Oct 31, 2003 at 07:07:28PM -0500, Andrew Cagney wrote:
> Hello,
> 
> This patch deprecates the function inside_entry_func.  Per the new comments:
> 
> +  /* NOTE: cagney/2003-10-31: A very simple test, such as
> +     get_frame_func == entry_point should be sufficient for
> +     identifying a pc in the entry function.  Does anyone know why it
> +     wasn't sufficient and hence, why the very convoluted
> +     "deprecated_inside_entry_func" is needed.  */
> +  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
> +     and implement a clean frame termination.  Not doing that is
> +     really a bug in the ABI/crt0, and, hence, not a reason for
> +     enabling the call to deprecated_inside_entry_func.  */

If handling broken systems isn't the job of GDB, then I surely don't
know what is.

Do you have a reason for wanting to deprecate this function?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:37 ` Kevin Buettner
@ 2003-11-01  0:46   ` Andrew Cagney
  2003-11-01  0:55     ` Kevin Buettner
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-01  0:46 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> On Oct 31,  7:07pm, Andrew Cagney wrote:
> 
> 
>> This patch deprecates the function inside_entry_func.  Per the new comments:
>> 
>> +  /* NOTE: cagney/2003-10-31: A very simple test, such as
>> +     get_frame_func == entry_point should be sufficient for
>> +     identifying a pc in the entry function.  Does anyone know why it
>> +     wasn't sufficient and hence, why the very convoluted
>> +     "deprecated_inside_entry_func" is needed.  */
>> +  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
>> +     and implement a clean frame termination.  Not doing that is
>> +     really a bug in the ABI/crt0, and, hence, not a reason for
>> +     enabling the call to deprecated_inside_entry_func.  */
> 
> 
> I do agree that it'd be nice if all ABIs provided a clean way to detect
> the bottom-most frame.  Not all of them do however, and in such cases,
> a mechanism like inside_entry_func() is necessary.

> So, in short, I oppose the deprecation of this function.

Kevin, you previously wrote:

>> I'd like to avoid re-introducing a dependency on inside_entry_func() as 
>> that places garish requirements on the object file readers :-(
> 
> I agree that object file readers should not attempt to track of
> the bounds of the start function.  However, given an arbitrary
> address, it's not unreasonable to ask the symtab machinery to attempt
> to figure out the function bounds.  And, in fact, this is just what
> find_pc_partial_function() does.

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:46   ` Andrew Cagney
@ 2003-11-01  0:55     ` Kevin Buettner
  2003-11-01  2:27       ` Andrew Cagney
  0 siblings, 1 reply; 25+ messages in thread
From: Kevin Buettner @ 2003-11-01  0:55 UTC (permalink / raw)
  To: Andrew Cagney, Kevin Buettner; +Cc: gdb-patches

On Oct 31,  7:46pm, Andrew Cagney wrote:

> > On Oct 31,  7:07pm, Andrew Cagney wrote:
> > 
> >> This patch deprecates the function inside_entry_func.  Per the new comments:
> >> 
> >> +  /* NOTE: cagney/2003-10-31: A very simple test, such as
> >> +     get_frame_func == entry_point should be sufficient for
> >> +     identifying a pc in the entry function.  Does anyone know why it
> >> +     wasn't sufficient and hence, why the very convoluted
> >> +     "deprecated_inside_entry_func" is needed.  */
> >> +  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
> >> +     and implement a clean frame termination.  Not doing that is
> >> +     really a bug in the ABI/crt0, and, hence, not a reason for
> >> +     enabling the call to deprecated_inside_entry_func.  */
> > 
> > I do agree that it'd be nice if all ABIs provided a clean way to detect
> > the bottom-most frame.  Not all of them do however, and in such cases,
> > a mechanism like inside_entry_func() is necessary.
> 
> > So, in short, I oppose the deprecation of this function.
> 
> Kevin, you previously wrote:
> 
> >> I'd like to avoid re-introducing a dependency on inside_entry_func() as 
> >> that places garish requirements on the object file readers :-(
> > 
> > I agree that object file readers should not attempt to track of
> > the bounds of the start function.  However, given an arbitrary
> > address, it's not unreasonable to ask the symtab machinery to attempt
> > to figure out the function bounds.  And, in fact, this is just what
> > find_pc_partial_function() does.

Yes, the reason I wrote this was to note that there are other ways of
implementing inside_entry_func() which wouldn't place garish
requirements on the object file readers.

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:55     ` Kevin Buettner
@ 2003-11-01  2:27       ` Andrew Cagney
  2003-11-07 17:31         ` Kevin Buettner
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-01  2:27 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Kevin, you previously wrote:
>> 
> 
>> >> I'd like to avoid re-introducing a dependency on inside_entry_func() as 
>> >> that places garish requirements on the object file readers :-(
> 
>> > 
>> > I agree that object file readers should not attempt to track of
>> > the bounds of the start function.  However, given an arbitrary
>> > address, it's not unreasonable to ask the symtab machinery to attempt
>> > to figure out the function bounds.  And, in fact, this is just what
>> > find_pc_partial_function() does.
> 
> 
> Yes, the reason I wrote this was to note that there are other ways of
> implementing inside_entry_func() which wouldn't place garish
> requirements on the object file readers.

Then I'm puzzled as to why you are objecting to me deprecating this 
existing garish hack?  Remember, I also wrote:

 > +  /* NOTE: cagney/2003-10-31: A very simple test, such as
 > +     get_frame_func == entry_point should be sufficient for
 > +     identifying a pc in the entry function.  Does anyone know why it
 > +     wasn't sufficient and hence, why the very convoluted
 > +     "deprecated_inside_entry_func" is needed.  */

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:42 ` Daniel Jacobowitz
@ 2003-11-01  2:37   ` Andrew Cagney
  2003-11-09  0:13     ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-01  2:37 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> On Fri, Oct 31, 2003 at 07:07:28PM -0500, Andrew Cagney wrote:
> 
>> Hello,
>> 
>> This patch deprecates the function inside_entry_func.  Per the new comments:
>> 
>> +  /* NOTE: cagney/2003-10-31: A very simple test, such as
>> +     get_frame_func == entry_point should be sufficient for
>> +     identifying a pc in the entry function.  Does anyone know why it
>> +     wasn't sufficient and hence, why the very convoluted
>> +     "deprecated_inside_entry_func" is needed.  */
>> +  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
>> +     and implement a clean frame termination.  Not doing that is
>> +     really a bug in the ABI/crt0, and, hence, not a reason for
>> +     enabling the call to deprecated_inside_entry_func.  */
> 
> 
> If handling broken systems isn't the job of GDB, then I surely don't
> know what is.
> Do you have a reason for wanting to deprecate this function?

Per my first new comment:

/* NOTE: cagney/2003-10-31: A very simple test, such as
    get_frame_func == entry_point should be sufficient for
    identifying a pc in the entry function.  Does anyone know why it
    wasn't sufficient and hence, why the very convoluted
    "deprecated_inside_entry_func" is needed.  */

and the previous comment:

   /* NOTE: cagney/2003-02-25: Don't enable until someone has found
      hard evidence that this is needed.  */

There is _still_ no evidence that this disabled hack is needed - time to 
deprecate it.

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  2:27       ` Andrew Cagney
@ 2003-11-07 17:31         ` Kevin Buettner
  2003-11-07 21:25           ` Andrew Cagney
  0 siblings, 1 reply; 25+ messages in thread
From: Kevin Buettner @ 2003-11-07 17:31 UTC (permalink / raw)
  To: Andrew Cagney, Kevin Buettner; +Cc: gdb-patches

On Oct 31,  9:27pm, Andrew Cagney wrote:

> > Kevin, you previously wrote:
> > 
> >> >> I'd like to avoid re-introducing a dependency on inside_entry_func() as 
> >> >> that places garish requirements on the object file readers :-(
> > 
> >> > 
> >> > I agree that object file readers should not attempt to track of
> >> > the bounds of the start function.  However, given an arbitrary
> >> > address, it's not unreasonable to ask the symtab machinery to attempt
> >> > to figure out the function bounds.  And, in fact, this is just what
> >> > find_pc_partial_function() does.
> > 
> > 
> > Yes, the reason I wrote this was to note that there are other ways of
> > implementing inside_entry_func() which wouldn't place garish
> > requirements on the object file readers.
> 
> Then I'm puzzled as to why you are objecting to me deprecating this 
> existing garish hack?  Remember, I also wrote:
> 
>  > +  /* NOTE: cagney/2003-10-31: A very simple test, such as
>  > +     get_frame_func == entry_point should be sufficient for
>  > +     identifying a pc in the entry function.  Does anyone know why it
>  > +     wasn't sufficient and hence, why the very convoluted
>  > +     "deprecated_inside_entry_func" is needed.  */

What I'm suggesting is that you implement inside_entry_func() using
"get_frame_func == entry_point".  What's so garish about that?

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-07 17:31         ` Kevin Buettner
@ 2003-11-07 21:25           ` Andrew Cagney
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Cagney @ 2003-11-07 21:25 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

> Then I'm puzzled as to why you are objecting to me deprecating this 
>> existing garish hack?  Remember, I also wrote:
>> 
>>  > +  /* NOTE: cagney/2003-10-31: A very simple test, such as
>>  > +     get_frame_func == entry_point should be sufficient for
>>  > +     identifying a pc in the entry function.  Does anyone know why it
>>  > +     wasn't sufficient and hence, why the very convoluted
>>  > +     "deprecated_inside_entry_func" is needed.  */
> 
> 
> What I'm suggesting is that you implement inside_entry_func() using
> "get_frame_func == entry_point".  What's so garish about that?

That task is best left to the person with a direct requirement for the 
mechanism.  That way, at the time the new code is committed, it has both 
a clear need and is known to work.

This is of course separate to deprecating inside_entry_func's current 
implementation.  The reason for deprecated the old code is to make its 
status very clear - don't use it in new code and try to eliminate it 
from old code.

enjoy,
Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  2:37   ` Andrew Cagney
@ 2003-11-09  0:13     ` Daniel Jacobowitz
  2003-11-09  2:40       ` Andrew Cagney
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-09  0:13 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Fri, Oct 31, 2003 at 09:37:24PM -0500, Andrew Cagney wrote:
> >On Fri, Oct 31, 2003 at 07:07:28PM -0500, Andrew Cagney wrote:
> >
> >>Hello,
> >>
> >>This patch deprecates the function inside_entry_func.  Per the new 
> >>comments:
> >>
> >>+  /* NOTE: cagney/2003-10-31: A very simple test, such as
> >>+     get_frame_func == entry_point should be sufficient for
> >>+     identifying a pc in the entry function.  Does anyone know why it
> >>+     wasn't sufficient and hence, why the very convoluted
> >>+     "deprecated_inside_entry_func" is needed.  */
> >>+  /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define
> >>+     and implement a clean frame termination.  Not doing that is
> >>+     really a bug in the ABI/crt0, and, hence, not a reason for
> >>+     enabling the call to deprecated_inside_entry_func.  */
> >
> >
> >If handling broken systems isn't the job of GDB, then I surely don't
> >know what is.
> >Do you have a reason for wanting to deprecate this function?
> 
> Per my first new comment:
> 
> /* NOTE: cagney/2003-10-31: A very simple test, such as
>    get_frame_func == entry_point should be sufficient for
>    identifying a pc in the entry function.  Does anyone know why it
>    wasn't sufficient and hence, why the very convoluted
>    "deprecated_inside_entry_func" is needed.  */
> 
> and the previous comment:
> 
>   /* NOTE: cagney/2003-02-25: Don't enable until someone has found
>      hard evidence that this is needed.  */
> 
> There is _still_ no evidence that this disabled hack is needed - time to 
> deprecate it.

If you think about it for a little while, and if you consider my
objection to the comment about broken ABIs above, constructing a
testcase is really quite easy.  Here's one, though it's contrived. 
Bear with me.

Take gdb.asm/asm-source (or anything else, really), for an i386 target,
and this script:

b _start
r
bt
set $ebp = 2
bt


Here's what GDB produces:

Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1". 
	[Hrm, is there a missing from_tty somewhere?  This seems
	superfluous.]
Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14.

Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
14		gdbasm_startup
Current language:  auto; currently asm
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#1  0x00000001 in ?? ()
#2  0xbffff680 in ?? ()


Apply this patch:

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.147
diff -u -p -r1.147 frame.c
--- frame.c	24 Oct 2003 17:37:03 -0000	1.147
+++ frame.c	8 Nov 2003 23:58:31 -0000
@@ -1831,7 +1831,7 @@ get_prev_frame (struct frame_info *this_
   /* 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 (1
 #if 0
       && backtrace_beyond_entry_func
 #endif


and GDB's output isn't changed.  That's because of the hokeyness of
figuring out the bounds of the entry function.  Apply this patch
instead, though (proof of concept only; it needs to be cleaned up and
inside_entry_func's signature changed, but it illustrates the point):

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.82
diff -u -p -r1.82 blockframe.c
--- blockframe.c	20 Oct 2003 14:38:42 -0000	1.82
+++ blockframe.c	8 Nov 2003 23:58:30 -0000
@@ -186,6 +186,9 @@ inside_entry_func (CORE_ADDR pc)
       if (DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0))
 	return 0;
     }
+  if (symfile_objfile->ei.entry_point == pc)
+    return 1;
+
   return (symfile_objfile->ei.entry_func_lowpc <= pc &&
 	  symfile_objfile->ei.entry_func_highpc > pc);
 }
@@ -615,7 +618,7 @@ legacy_frame_chain_valid (CORE_ADDR fp, 
 
   /* If we're already inside the entry function for the main objfile, then it
      isn't valid.  */
-  if (inside_entry_func (get_frame_pc (fi)))
+  if (inside_entry_func (get_frame_func (fi)))
     return 0;
 
   /* If we're inside the entry file, it isn't valid.  */
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.147
diff -u -p -r1.147 frame.c
--- frame.c	24 Oct 2003 17:37:03 -0000	1.147
+++ frame.c	8 Nov 2003 23:58:31 -0000
@@ -1831,12 +1831,12 @@ get_prev_frame (struct frame_info *this_
   /* 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 (1
 #if 0
       && backtrace_beyond_entry_func
 #endif
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
-      && inside_entry_func (get_frame_pc (this_frame)))
+      && inside_entry_func (get_frame_func (this_frame)))
     {
       if (frame_debug)
 	{


and we get this output:

Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14.

Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
14		gdbasm_startup
Current language:  auto; currently asm
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14
#0  _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14


Which is, I think, an obvious improvement.  If the object file has an
entry point, by default we should not try to backtrace past it.  The
extra "set backtrace" option mentioned in your comment above is
probably called for.



Think my testcase was too contrived?  It's possible to rewrite
asm-source in a number of ways to acheive similar effect.  I believe
I've also produced this result in the ARM simulator using newlib, but I
don't have that setup around to recreate it at the moment.  You could
probably do it using static linking and -fomit-frame-pointer, except
that GDB on my host system gets lost in __libc_start_main in that case
and never gets to _start.


I believe the function should be fixed and re-enabled, not deleted.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-09  0:13     ` Daniel Jacobowitz
@ 2003-11-09  2:40       ` Andrew Cagney
  2003-11-09  3:50         ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-09  2:40 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> I believe the function should be fixed and re-enabled, not deleted.

So you agree then, that the existing code / mechanism should go 
(replaced by "A very simple test, such as get_frame_func == entry_point").

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-09  2:40       ` Andrew Cagney
@ 2003-11-09  3:50         ` Daniel Jacobowitz
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-09  3:50 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Sat, Nov 08, 2003 at 09:24:39PM -0500, Andrew Cagney wrote:
> >I believe the function should be fixed and re-enabled, not deleted.
> 
> So you agree then, that the existing code / mechanism should go 
> (replaced by "A very simple test, such as get_frame_func == entry_point").

Sure.

I'm just saying that I disagree with your patch; the comment about
ABI breakage is dubious, and deprecating a function that we should
repair doesn't seem to have any point.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-01  0:07 [rfa:symtab] deprecate inside_entry_func Andrew Cagney
  2003-11-01  0:37 ` Kevin Buettner
  2003-11-01  0:42 ` Daniel Jacobowitz
@ 2003-11-21 19:53 ` Andrew Cagney
  2003-11-21 19:59   ` Daniel Jacobowitz
  2003-11-21 20:07   ` David Carlton
  2 siblings, 2 replies; 25+ messages in thread
From: Andrew Cagney @ 2003-11-21 19:53 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

> Hello,
> 
> This patch deprecates the function inside_entry_func.
> 
> PS: Ref: http://sources.redhat.com/ml/gdb-patches/2003-10/msg00533.html

Here's a revised patch.  It now eliminates the [disabled] 
inside_entry_func call from "get_prev_frame", replacing it with the more 
direct [but equally disabled] get_frame_func() == entry_point_address() 
test.  This way all calls to inside_entry_func have been eliminated from 
up-to-date code.

Thoughts? Symtab ok?

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 10092 bytes --]

2003-11-21  Andrew Cagney  <cagney@redhat.com>

	* frame.c (get_prev_frame): Replace disabled call to
	deprecated_inside_entry_func with disabled test for the frame's
	function being at the entry_point_address.
	* objfiles.h (struct entry_info): Deprecate the members
	"entry_func_lowpc", and "entry_func_lowpc".
	* symfile.c (init_entry_point_info): Update.
	* objfiles.c (objfile_relocate): Update.
	* dwarfread.c (read_func_scope): Update.
	* dwarf2read.c (read_func_scope): Update.
	* blockframe.c (deprecated_inside_entry_func): Update.
	* defs.h (deprecated_inside_entry_func): Rename
	"inside_entry_func".
	* frv-tdep.c (frv_frame_this_id): Update.
	* blockframe.c (deprecated_inside_entry_func): Rename
	"inside_entry_func".
	(legacy_frame_chain_valid): Update.

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.82
diff -u -r1.82 blockframe.c
--- blockframe.c	20 Oct 2003 14:38:42 -0000	1.82
+++ blockframe.c	21 Nov 2003 19:40:17 -0000
@@ -164,15 +164,24 @@
 }
 
 /* Test a specified PC value to see if it is in the range of addresses
-   that correspond to the process entry point function.  See comments
-   in objfiles.h for why we might want to do this.
+   that correspond to the process entry point function.
 
-   Typically called from DEPRECATED_FRAME_CHAIN_VALID.
-
-   A PC of zero is always considered to be the bottom of the stack. */
+   NOTE: cagney/2003-11-21: Deprecate this specific _mechanism_ for
+   determining if the PC is inside the entry function.  While there's
+   nothing technically wrong with the idea of using "PC inside entry
+   function" as a condition for terminating a backtrace, there's
+   something seriously wrong with this specific implementation.  It
+   forces the symbol table code to go through all sorts of symbol
+   table load and relocation hoops just to get the test right.
+
+   NOTE: cagney/2003-11-21: The function "get_prev_frame" which could
+   reasonably be expected to test for "PC inside entry function"
+   currently has that code disabled (and if/when it is enabled will
+   use the far simpler "frame->func == entry_point_address" and not
+   this function.  */
 
 int
-inside_entry_func (CORE_ADDR pc)
+deprecated_inside_entry_func (CORE_ADDR pc)
 {
   if (pc == 0)
     return 1;
@@ -186,8 +195,8 @@
       if (DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0))
 	return 0;
     }
-  return (symfile_objfile->ei.entry_func_lowpc <= pc &&
-	  symfile_objfile->ei.entry_func_highpc > pc);
+  return (symfile_objfile->ei.deprecated_entry_func_lowpc <= pc &&
+	  symfile_objfile->ei.deprecated_entry_func_highpc > pc);
 }
 
 /* Return nonzero if the function for this frame lacks a prologue.  Many
@@ -615,7 +624,7 @@
 
   /* If we're already inside the entry function for the main objfile, then it
      isn't valid.  */
-  if (inside_entry_func (get_frame_pc (fi)))
+  if (deprecated_inside_entry_func (get_frame_pc (fi)))
     return 0;
 
   /* If we're inside the entry file, it isn't valid.  */
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.133
diff -u -r1.133 defs.h
--- defs.h	31 Oct 2003 19:19:51 -0000	1.133
+++ defs.h	21 Nov 2003 19:40:18 -0000
@@ -316,7 +316,7 @@
 
 /* From blockframe.c */
 
-extern int inside_entry_func (CORE_ADDR);
+extern int deprecated_inside_entry_func (CORE_ADDR);
 
 extern int deprecated_inside_entry_file (CORE_ADDR addr);
 
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.114
diff -u -r1.114 dwarf2read.c
--- dwarf2read.c	19 Nov 2003 15:08:01 -0000	1.114
+++ dwarf2read.c	21 Nov 2003 19:40:19 -0000
@@ -2131,8 +2131,8 @@
   if (objfile->ei.entry_point >= lowpc &&
       objfile->ei.entry_point < highpc)
     {
-      objfile->ei.entry_func_lowpc = lowpc;
-      objfile->ei.entry_func_highpc = highpc;
+      objfile->ei.deprecated_entry_func_lowpc = lowpc;
+      objfile->ei.deprecated_entry_func_highpc = highpc;
     }
 
   new = push_context (0, lowpc);
Index: dwarfread.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarfread.c,v
retrieving revision 1.31
diff -u -r1.31 dwarfread.c
--- dwarfread.c	6 Nov 2003 22:54:01 -0000	1.31
+++ dwarfread.c	21 Nov 2003 19:40:19 -0000
@@ -1767,8 +1767,8 @@
   if (objfile->ei.entry_point >= dip->at_low_pc &&
       objfile->ei.entry_point < dip->at_high_pc)
     {
-      objfile->ei.entry_func_lowpc = dip->at_low_pc;
-      objfile->ei.entry_func_highpc = dip->at_high_pc;
+      objfile->ei.deprecated_entry_func_lowpc = dip->at_low_pc;
+      objfile->ei.deprecated_entry_func_highpc = dip->at_high_pc;
     }
   new = push_context (0, dip->at_low_pc);
   new->name = new_symbol (dip, objfile);
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.148
diff -u -r1.148 frame.c
--- frame.c	19 Nov 2003 17:35:46 -0000	1.148
+++ frame.c	21 Nov 2003 19:40:20 -0000
@@ -1824,23 +1824,32 @@
      asm-source tests now stop in "main" instead of halting the
      backtrace in wierd and wonderful ways somewhere inside the entry
      file.  Suspect that deprecated_inside_entry_file and
-     inside_entry_func tests were added to work around that (now
-     fixed) case.  */
+     deprecated_inside_entry_func tests were added to work around that
+     (now fixed) case.  */
   /* NOTE: cagney/2003-07-15: danielj (if I'm reading it right)
-     suggested having the inside_entry_func test use the
+     suggested having the deprecated_inside_entry_func test use the
      inside_main_func msymbol trick (along with entry_point_address 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.  */
+     enabled/disabled.  */
+  /* NOTE: cagney/2003-10-31: Replaced disabled call to
+     deprecated_inside_entry_func with the far simpler test for the
+     frame's function being the entry point.  Still need someone to
+     firstly demonstrate a need for enabling this, and secondly
+     implement implement/test it along with the "set backtrace
+     beyond-entry-func" command.  */
   if (0
 #if 0
       && backtrace_beyond_entry_func
 #endif
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
-      && inside_entry_func (get_frame_pc (this_frame)))
+#if 0
+      && get_frame_func (this_frame) == entry_point_address ()
+#endif
+      )
     {
       if (frame_debug)
 	{
Index: frv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/frv-tdep.c,v
retrieving revision 1.61
diff -u -r1.61 frv-tdep.c
--- frv-tdep.c	7 Nov 2003 06:00:07 -0000	1.61
+++ frv-tdep.c	21 Nov 2003 19:40:20 -0000
@@ -1103,7 +1103,7 @@
 
   /* This is meant to halt the backtrace at "_start".  Make sure we
      don't halt it at a generic dummy frame. */
-  if (inside_entry_func (func))
+  if (deprecated_inside_entry_func (func))
     return;
 
   /* Check if the stack is empty.  */
Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.42
diff -u -r1.42 objfiles.c
--- objfiles.c	11 Nov 2003 20:04:52 -0000	1.42
+++ objfiles.c	21 Nov 2003 19:40:20 -0000
@@ -781,10 +781,10 @@
       }
   }
 
-  if (objfile->ei.entry_func_lowpc != INVALID_ENTRY_LOWPC)
+  if (objfile->ei.deprecated_entry_func_lowpc != INVALID_ENTRY_LOWPC)
     {
-      objfile->ei.entry_func_lowpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
-      objfile->ei.entry_func_highpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
+      objfile->ei.deprecated_entry_func_lowpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
+      objfile->ei.deprecated_entry_func_highpc += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
     }
 
   if (objfile->ei.deprecated_entry_file_lowpc != INVALID_ENTRY_LOWPC)
Index: objfiles.h
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.h,v
retrieving revision 1.28
diff -u -r1.28 objfiles.h
--- objfiles.h	6 Nov 2003 02:52:27 -0000	1.28
+++ objfiles.h	21 Nov 2003 19:40:21 -0000
@@ -106,7 +106,7 @@
    #define DEPRECATED_FRAME_CHAIN_VALID(chain, thisframe)     \
    (chain != 0                                   \
    && !(inside_main_func ((thisframe)->pc))     \
-   && !(inside_entry_func ((thisframe)->pc)))
+   && !(deprecated_inside_entry_func ((thisframe)->pc)))
 
    and add initializations of the four scope controlling variables inside
    the object file / debugging information processing modules.  */
@@ -125,8 +125,8 @@
     /* Start (inclusive) and end (exclusive) of function containing the
        entry point. */
 
-    CORE_ADDR entry_func_lowpc;
-    CORE_ADDR entry_func_highpc;
+    CORE_ADDR deprecated_entry_func_lowpc;
+    CORE_ADDR deprecated_entry_func_highpc;
 
     /* Start (inclusive) and end (exclusive) of object file containing the
        entry point. */
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.113
diff -u -r1.113 symfile.c
--- symfile.c	15 Nov 2003 19:39:04 -0000	1.113
+++ symfile.c	21 Nov 2003 19:40:21 -0000
@@ -329,8 +329,8 @@
     }
   objfile->ei.deprecated_entry_file_lowpc = INVALID_ENTRY_LOWPC;
   objfile->ei.deprecated_entry_file_highpc = INVALID_ENTRY_HIGHPC;
-  objfile->ei.entry_func_lowpc = INVALID_ENTRY_LOWPC;
-  objfile->ei.entry_func_highpc = INVALID_ENTRY_HIGHPC;
+  objfile->ei.deprecated_entry_func_lowpc = INVALID_ENTRY_LOWPC;
+  objfile->ei.deprecated_entry_func_highpc = INVALID_ENTRY_HIGHPC;
   objfile->ei.main_func_lowpc = INVALID_ENTRY_LOWPC;
   objfile->ei.main_func_highpc = INVALID_ENTRY_HIGHPC;
 }

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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 19:53 ` Andrew Cagney
@ 2003-11-21 19:59   ` Daniel Jacobowitz
  2003-11-21 20:11     ` Kevin Buettner
  2003-11-21 20:46     ` Andrew Cagney
  2003-11-21 20:07   ` David Carlton
  1 sibling, 2 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-21 19:59 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Fri, Nov 21, 2003 at 02:53:42PM -0500, Andrew Cagney wrote:
> >Hello,
> >
> >This patch deprecates the function inside_entry_func.
> >
> >PS: Ref: http://sources.redhat.com/ml/gdb-patches/2003-10/msg00533.html
> 
> Here's a revised patch.  It now eliminates the [disabled] 
> inside_entry_func call from "get_prev_frame", replacing it with the more 
> direct [but equally disabled] get_frame_func() == entry_point_address() 
> test.  This way all calls to inside_entry_func have been eliminated from 
> up-to-date code.
> 
> Thoughts? Symtab ok?
> 
> Andrew
> 

What Kevin and I have both repeatedly suggested, I think, is:
  - Do not deprecate inside_entry_func; fix it if you don't like the
    way it is implemented.  Change the implementation.
  - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
    your changelog, two lowpc's) if you really want to deprecate
    something.

The concept of inside_entry_func does not seem to be deprecated, so why
rename it?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 19:53 ` Andrew Cagney
  2003-11-21 19:59   ` Daniel Jacobowitz
@ 2003-11-21 20:07   ` David Carlton
  1 sibling, 0 replies; 25+ messages in thread
From: David Carlton @ 2003-11-21 20:07 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Fri, 21 Nov 2003 14:53:42 -0500, Andrew Cagney <ac131313@redhat.com> said:

>> This patch deprecates the function inside_entry_func.
>> PS: Ref:
>> http://sources.redhat.com/ml/gdb-patches/2003-10/msg00533.html

I skimmed this, though I haven't been following the discussion too
closely.  But I don't quite understand the philosophical stance here -
on the one hand, I see:

> This way all calls to inside_entry_func have been eliminated from
> up-to-date code.

which suggests that you want to get rid of the idea of
inside_entry_func(PC) entirely, but on the other hand I see:

> +   NOTE: cagney/2003-11-21: Deprecate this specific _mechanism_ for
> +   determining if the PC is inside the entry function.  While there's
> +   nothing technically wrong with the idea of using "PC inside entry
> +   function" as a condition for terminating a backtrace, there's
> +   something seriously wrong with this specific implementation.  It
> +   forces the symbol table code to go through all sorts of symbol
> +   table load and relocation hoops just to get the test right.

which suggests that you just want to get rid of the implementation.

So which is it?  It seems to me that the renaming you propose suggests
that the interface should go away, but the above comment suggests that
you're happy with the interface, you just don't like the
implementation.

If you're happy with the interface, it seems to me that the proper
thing to do is to add a big FIXME comment to inside_entry_func.  If
for some reason you think that isn't strong enough, then I could
imagine renaming it to something like "broken_inside_entry_func".  But
I don't like renaming it to deprecated_inside_entry_func if the above
NOTE is accurate.

(I don't have any opinion one way or another as to whether it's better
to deprecate the interface or to just flag the implementation as
broken.)

David Carlton
carlton@kealia.com


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 19:59   ` Daniel Jacobowitz
@ 2003-11-21 20:11     ` Kevin Buettner
  2003-11-21 20:46     ` Andrew Cagney
  1 sibling, 0 replies; 25+ messages in thread
From: Kevin Buettner @ 2003-11-21 20:11 UTC (permalink / raw)
  To: Daniel Jacobowitz, Andrew Cagney; +Cc: gdb-patches

On Nov 21,  2:59pm, Daniel Jacobowitz wrote:

> On Fri, Nov 21, 2003 at 02:53:42PM -0500, Andrew Cagney wrote:
> > >Hello,
> > >
> > >This patch deprecates the function inside_entry_func.
> > >
> > >PS: Ref: http://sources.redhat.com/ml/gdb-patches/2003-10/msg00533.html
> > 
> > Here's a revised patch.  It now eliminates the [disabled] 
> > inside_entry_func call from "get_prev_frame", replacing it with the more 
> > direct [but equally disabled] get_frame_func() == entry_point_address() 
> > test.  This way all calls to inside_entry_func have been eliminated from 
> > up-to-date code.
> > 
> > Thoughts? Symtab ok?
> > 
> > Andrew
> > 
> 
> What Kevin and I have both repeatedly suggested, I think, is:
>   - Do not deprecate inside_entry_func; fix it if you don't like the
>     way it is implemented.  Change the implementation.
>   - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
>     your changelog, two lowpc's) if you really want to deprecate
>     something.
> 
> The concept of inside_entry_func does not seem to be deprecated, so why
> rename it?

I agree with Daniel.

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 19:59   ` Daniel Jacobowitz
  2003-11-21 20:11     ` Kevin Buettner
@ 2003-11-21 20:46     ` Andrew Cagney
  2003-11-21 20:48       ` Daniel Jacobowitz
  1 sibling, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-21 20:46 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> 
> What Kevin and I have both repeatedly suggested, I think, is:
>   - Do not deprecate inside_entry_func; fix it if you don't like the
>     way it is implemented.  Change the implementation.
>   - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
>     your changelog, two lowpc's) if you really want to deprecate
>     something.

Please point me at a legitimate use of this function.

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 20:46     ` Andrew Cagney
@ 2003-11-21 20:48       ` Daniel Jacobowitz
  2003-11-21 20:55         ` Andrew Cagney
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-21 20:48 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Fri, Nov 21, 2003 at 03:46:46PM -0500, Andrew Cagney wrote:
> >
> >What Kevin and I have both repeatedly suggested, I think, is:
> >  - Do not deprecate inside_entry_func; fix it if you don't like the
> >    way it is implemented.  Change the implementation.
> >  - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
> >    your changelog, two lowpc's) if you really want to deprecate
> >    something.
> 
> Please point me at a legitimate use of this function.

Please read my previous response to you in this thread, in which I did
so at length.
  http://sources.redhat.com/ml/gdb-patches/2003-11/msg00158.html

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 20:48       ` Daniel Jacobowitz
@ 2003-11-21 20:55         ` Andrew Cagney
  2003-11-21 21:04           ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-21 20:55 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> On Fri, Nov 21, 2003 at 03:46:46PM -0500, Andrew Cagney wrote:
> 
>> >
>> >What Kevin and I have both repeatedly suggested, I think, is:
>> >  - Do not deprecate inside_entry_func; fix it if you don't like the
>> >    way it is implemented.  Change the implementation.
>> >  - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
>> >    your changelog, two lowpc's) if you really want to deprecate
>> >    something.
> 
>> 
>> Please point me at a legitimate use of this function.
> 
> 
> Please read my previous response to you in this thread, in which I did
> so at length.
>   http://sources.redhat.com/ml/gdb-patches/2003-11/msg00158.html

That's not what I'm asking.

With the call to inside_entry_func removed, from get_prev_frame, can you 
point me at any remainng _legitimate) uses of that function?

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 20:55         ` Andrew Cagney
@ 2003-11-21 21:04           ` Daniel Jacobowitz
  2003-11-21 21:24             ` Kevin Buettner
  2003-11-22  0:37             ` Andrew Cagney
  0 siblings, 2 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-21 21:04 UTC (permalink / raw)
  To: gdb-patches

On Fri, Nov 21, 2003 at 03:55:52PM -0500, Andrew Cagney wrote:
> >On Fri, Nov 21, 2003 at 03:46:46PM -0500, Andrew Cagney wrote:
> >
> >>>
> >>>What Kevin and I have both repeatedly suggested, I think, is:
> >>>  - Do not deprecate inside_entry_func; fix it if you don't like the
> >>>    way it is implemented.  Change the implementation.
> >>>  - Deprecate entry_func_lowpc and entry_func_highpc (there's a typo in
> >>>    your changelog, two lowpc's) if you really want to deprecate
> >>>    something.
> >
> >>
> >>Please point me at a legitimate use of this function.
> >
> >
> >Please read my previous response to you in this thread, in which I did
> >so at length.
> >  http://sources.redhat.com/ml/gdb-patches/2003-11/msg00158.html
> 
> That's not what I'm asking.
> 
> With the call to inside_entry_func removed, from get_prev_frame, can you 
> point me at any remainng _legitimate) uses of that function?

The other call to it in legacy_frame_chain_valid, which wants to know
the same thing?  I imagine the third caller, in frv-tdep.c, is bogus
and could be removed somehow.  But if it's going to be left there then
it seems reasonable to update it also.

Conceptually the patch you just posted sees to be:
 - Change the implementation of inside_entry_func
 - Inline the new inside_entry_func into the one caller you're fond of
 - Add a deprecated copy of the old implementation for the other
   callers

From the man who is always telling us how unimportant performance is
compared to clarity, I don't see the point.  Also, this leaves an old
implementation and a new implementation around for no visible reason.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 21:04           ` Daniel Jacobowitz
@ 2003-11-21 21:24             ` Kevin Buettner
  2003-11-21 21:54               ` Daniel Jacobowitz
  2003-11-22  0:37             ` Andrew Cagney
  1 sibling, 1 reply; 25+ messages in thread
From: Kevin Buettner @ 2003-11-21 21:24 UTC (permalink / raw)
  To: Daniel Jacobowitz, gdb-patches

On Nov 21,  4:04pm, Daniel Jacobowitz wrote:

> I imagine the third caller, in frv-tdep.c, is bogus
> and could be removed somehow.  But if it's going to be left there then
> it seems reasonable to update it also.

I had thought that get_prev_frame's call to inside_entry_func() had
been disabled pending a demonstration that it's needed.  Hmm...  I see
I was getting this confused with the code corresponding to
inside_entry_file().

Okay, that means that frv-tdep.c's use of inside_entry_func() can go
away.  I'll get rid of it.

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 21:24             ` Kevin Buettner
@ 2003-11-21 21:54               ` Daniel Jacobowitz
  2003-11-21 22:40                 ` Kevin Buettner
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-21 21:54 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

On Fri, Nov 21, 2003 at 02:24:16PM -0700, Kevin Buettner wrote:
> On Nov 21,  4:04pm, Daniel Jacobowitz wrote:
> 
> > I imagine the third caller, in frv-tdep.c, is bogus
> > and could be removed somehow.  But if it's going to be left there then
> > it seems reasonable to update it also.
> 
> I had thought that get_prev_frame's call to inside_entry_func() had
> been disabled pending a demonstration that it's needed.  Hmm...  I see
> I was getting this confused with the code corresponding to
> inside_entry_file().
> 
> Okay, that means that frv-tdep.c's use of inside_entry_func() can go
> away.  I'll get rid of it.

Are you sure?

  /* 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
      && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
      && inside_entry_func (get_frame_pc (this_frame)))

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 21:54               ` Daniel Jacobowitz
@ 2003-11-21 22:40                 ` Kevin Buettner
  0 siblings, 0 replies; 25+ messages in thread
From: Kevin Buettner @ 2003-11-21 22:40 UTC (permalink / raw)
  To: Daniel Jacobowitz, Kevin Buettner; +Cc: gdb-patches

On Nov 21,  4:54pm, Daniel Jacobowitz wrote:

> On Fri, Nov 21, 2003 at 02:24:16PM -0700, Kevin Buettner wrote:
> > On Nov 21,  4:04pm, Daniel Jacobowitz wrote:
> > 
> > > I imagine the third caller, in frv-tdep.c, is bogus
> > > and could be removed somehow.  But if it's going to be left there then
> > > it seems reasonable to update it also.
> > 
> > I had thought that get_prev_frame's call to inside_entry_func() had
> > been disabled pending a demonstration that it's needed.  Hmm...  I see
> > I was getting this confused with the code corresponding to
> > inside_entry_file().
> > 
> > Okay, that means that frv-tdep.c's use of inside_entry_func() can go
> > away.  I'll get rid of it.
> 
> Are you sure?
> 
>   /* 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
>       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
>       && inside_entry_func (get_frame_pc (this_frame)))

Oops.  You're right.

(My attention was so focused on the limited scope of the ``#if 0'' that
I failed to notice the 0 in the ``if'' conditional.)

Kevin


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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-21 21:04           ` Daniel Jacobowitz
  2003-11-21 21:24             ` Kevin Buettner
@ 2003-11-22  0:37             ` Andrew Cagney
  2003-11-22  0:41               ` Daniel Jacobowitz
  1 sibling, 1 reply; 25+ messages in thread
From: Andrew Cagney @ 2003-11-22  0:37 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches


>> That's not what I'm asking.
>> 
>> With the call to inside_entry_func removed, from get_prev_frame, can you 
>> point me at any remainng _legitimate) uses of that function?
> 
> 
> The other call to it in legacy_frame_chain_valid, which wants to know
> the same thing?  I imagine the third caller, in frv-tdep.c, is bogus
> and could be removed somehow.  But if it's going to be left there then
> it seems reasonable to update it also.

I see Kevin's finally removed that bogus call.

> Conceptually the patch you just posted sees to be:
>  - Change the implementation of inside_entry_func
>  - Inline the new inside_entry_func into the one caller you're fond of
>  - Add a deprecated copy of the old implementation for the other
>    callers
> 
>>From the man who is always telling us how unimportant performance is
> compared to clarity, I don't see the point.  Also, this leaves an old
> implementation and a new implementation around for no visible reason.

"Indirection can be helpful, but needless redirection is irritating."

Note that the implementation of the two mechanisms is very different. 
Better to leave old code using the old mechanism, and new code using the 
new mechanism.

Andrew



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

* Re: [rfa:symtab] deprecate inside_entry_func
  2003-11-22  0:37             ` Andrew Cagney
@ 2003-11-22  0:41               ` Daniel Jacobowitz
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2003-11-22  0:41 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Fri, Nov 21, 2003 at 07:37:31PM -0500, Andrew Cagney wrote:
> 
> >>That's not what I'm asking.
> >>
> >>With the call to inside_entry_func removed, from get_prev_frame, can you 
> >>point me at any remainng _legitimate) uses of that function?
> >
> >
> >The other call to it in legacy_frame_chain_valid, which wants to know
> >the same thing?  I imagine the third caller, in frv-tdep.c, is bogus
> >and could be removed somehow.  But if it's going to be left there then
> >it seems reasonable to update it also.
> 
> I see Kevin's finally removed that bogus call.

Mistakenly, as you may have seen from his followup.

> >Conceptually the patch you just posted sees to be:
> > - Change the implementation of inside_entry_func
> > - Inline the new inside_entry_func into the one caller you're fond of
> > - Add a deprecated copy of the old implementation for the other
> >   callers
> >
> >>From the man who is always telling us how unimportant performance is
> >compared to clarity, I don't see the point.  Also, this leaves an old
> >implementation and a new implementation around for no visible reason.
> 
> "Indirection can be helpful, but needless redirection is irritating."
> 
> Note that the implementation of the two mechanisms is very different. 
> Better to leave old code using the old mechanism, and new code using the 
> new mechanism.

I believe the point of this discussion is that Kevin and I disagree
with your conclusion.

It's a mechanism, it doesn't matter if the implementation is different;
and one of them works and the other doesn't, so why keep two
implementations of the same thing if only one works?  In any case,
Kevin's patch to resolve this issue looks reasonable to me.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

end of thread, other threads:[~2003-11-22  0:41 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-11-01  0:07 [rfa:symtab] deprecate inside_entry_func Andrew Cagney
2003-11-01  0:37 ` Kevin Buettner
2003-11-01  0:46   ` Andrew Cagney
2003-11-01  0:55     ` Kevin Buettner
2003-11-01  2:27       ` Andrew Cagney
2003-11-07 17:31         ` Kevin Buettner
2003-11-07 21:25           ` Andrew Cagney
2003-11-01  0:42 ` Daniel Jacobowitz
2003-11-01  2:37   ` Andrew Cagney
2003-11-09  0:13     ` Daniel Jacobowitz
2003-11-09  2:40       ` Andrew Cagney
2003-11-09  3:50         ` Daniel Jacobowitz
2003-11-21 19:53 ` Andrew Cagney
2003-11-21 19:59   ` Daniel Jacobowitz
2003-11-21 20:11     ` Kevin Buettner
2003-11-21 20:46     ` Andrew Cagney
2003-11-21 20:48       ` Daniel Jacobowitz
2003-11-21 20:55         ` Andrew Cagney
2003-11-21 21:04           ` Daniel Jacobowitz
2003-11-21 21:24             ` Kevin Buettner
2003-11-21 21:54               ` Daniel Jacobowitz
2003-11-21 22:40                 ` Kevin Buettner
2003-11-22  0:37             ` Andrew Cagney
2003-11-22  0:41               ` Daniel Jacobowitz
2003-11-21 20:07   ` David Carlton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox