[review] Avoid infinite recursion in find_pc_sect_line

[review] Avoid infinite recursion in find_pc_sect_line

[Kevin Buettner (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................

Avoid infinite recursion in find_pc_sect_line

This is a patch which has been in Fedora GDB for well over a decade.  It
was written by Jan Kratochvil, though I've made some minor changes to
the comment and warning message.

It was motivated by a customer reported bug (back in 2006!) which
showed infinite mutual recursion between find_pc_sect_line and
find_pc_line.  Here is a portion of the backtrace from the bug report:

    (gdb) bt
    #0  0x00000000004450a4 in lookup_minimal_symbol_by_pc_section (
	pc=251700325328, section=0x570f500) at gdb/minsyms.c:484
    #1  0x00000000004bbfb2 in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2057
    #2  0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #3  0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081

    ...   (lots and lots of the same two functions with the same parameters)

    #1070 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1071 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1072 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1073 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1074 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1075 0x00000000004bc1ff in find_pc_sect_line (pc=251696794399,
	section=0x59b0df8, notcurrent=0) at gdb/symtab.c:2081
    #1076 0x00000000004bc480 in find_pc_line (pc=251696794399, notcurrent=0)
	at gdb/symtab.c:2232
    #1077 0x000000000055550e in find_frame_sal (frame=0xb3f3e0, sal=0x7fff1d1a8200)
	at gdb/frame.c:1392
    #1078 0x00000000004d86fd in set_current_sal_from_frame (frame=0x1648, center=1)
	at gdb/stack.c:379
    #1079 0x00000000004cf137 in normal_stop () at gdb/infrun.c:3147
    ...

The test case was a large application.  Attempts were made to make a
small(er) test case, but those attempts were not successful.
Therefore, I cannot provide a new test for this patch.

That said, we ought to guard against recursively calling
find_pc_sect_line (via find_pc_line) with the identical PC value that
it had been called with.  Should this happen, infinite recursion (as
shown in the above backtrace) is the result.  This patch prevents
that from happening.

Also, it seems likely that if this case should happen, there is a bug
somewhere, possibly in the C library or in some other part of the
toolchain, so we print a warning message asking the user to file a bug
report.  We don't error out here because falling through may produce
a satisfactory result.

I spent some time looking at the surrounding code and commentary which
handle the case of PC being in a stub/trampoline.  It first appeared
in the public GDB repository in April, 1999.  The ChangeLog entry for
this commit is from 1998-12-31.  The relevant portion is:

	(find_pc_sect_line): Return correct information if pc is in import
	or export stub (trampoline).

What's remarkable about the overall ChangeLog entry is that it's over
2500+ lines long!  I believe that this was part of the infamous "HP
merge" (in which insufficient due diligence was given in accepting
a large batch of changes from an outside source).  In the years that
followed, much of this code was either significantly revised or
outright removed.

For this particular case, I'm grateful that extensive comments were
provided by "RT".  (I haven't been able to figure out who RT is/was.)
I've decided against attempting to revise this stub/trampoline handling
code any further than adding Jan's test which prevents an obvious case
of infinite recursion.

I've tested on Fedora 31, x86-64.  I see no regressions.  I've also
searched the logfile for the new message, but as expected, no message
was found (which is good).

gdb/ChangeLog:

	* symtab.c (find_pc_sect_line): Add check which prevents infinite
	recursion.

Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
---
M gdb/symtab.c
1 file changed, 10 insertions(+), 0 deletions(-)



diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0064800..8a01317 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3142,6 +3142,16 @@
 	     SYMBOL_LINKAGE_NAME (msymbol)); */
 	  ;
 	/* fall through */
+	else if (BMSYMBOL_VALUE_ADDRESS (mfunsym) == pc)
+	  {
+	    /* Avoid infinite (mutual) recursion between
+	       find_pc_sect_line and find_pc_line.  */
+	    warning ("Could not dereference stub/trampoline %s (0x%s).  "
+	             "Please file a bug report.",
+	             MSYMBOL_LINKAGE_NAME (msymbol.minsym),
+		     paddress(target_gdbarch (), pc));
+	    /* Fall through.  */
+	  }
 	else
 	  return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
       }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-MessageType: newchange

[review] Avoid infinite recursion in find_pc_sect_line

[Luis Machado (Code Review)]

Luis Machado has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................


Patch Set 1:

Out of curiosity, is the example backtrace from a code using the patched version of GDB? How did we break out of the recursion and into lookup_minimal_symbol_by_pc_section?


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-Comment-Date: Wed, 27 Nov 2019 02:19:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review] Avoid infinite recursion in find_pc_sect_line

[Kevin Buettner (Code Review)]

Kevin Buettner has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> Out of curiosity, is the example backtrace from a code using the patched version of GDB? How did we break out of the recursion and into lookup_minimal_symbol_by_pc_section?

I don't know the answers to either of these questions. The example backtrace was pasted from a very old customer bug report.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:13:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review] Avoid infinite recursion in find_pc_sect_line

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................


Patch Set 1:

I'm not 100% comfortable in merging fixes like this, in an area that's already very tricky, without reproducer.  I'm not strongly opposing either, but just sharing my opinion.

It just doesn't sound logical.  To reach this "else", we need to:

1. lookup a minsym for PC, that minsym is a trampoline.  It means that this trampoline minsym is the closest (smaller or equal) minsym from PC, all minsym types considered.
2. lookup a _text_ minsym by name, we find one that is exactly equal to pc

If that text minsym we have found in #2 is equal to PC, why did we not find it in #1?  The only way I see is for the two minsyms (the trampoline one and the text one) to have the same value.  The lookup function would have returned the trampoline one, because it can only return one symbol, so it has to choose, and the trampoline one happens to come before.  Then, we search specifically text symbols by name, we find the text one.  But that case would already handled with the existing

	else if (BMSYMBOL_VALUE_ADDRESS (mfunsym)
		 == BMSYMBOL_VALUE_ADDRESS (msymbol))

So the only explanation I can imagine is that there was another GDB bug, causing it to mess up the minsym creation or lookup.  And either:

- that bug was fixed, and this fix is no longer needed
- that bug still isn't fixed, in which case this fix just covers up the problem.

I think you are already conscious of this, since you added a warning to tell the user that this is not expected.  But if this is a condition we expect to never legitimately happen, I would suggest using an assertion instead:

	    gdb_assert (BMSYMBOL_VALUE_ADDRESS (mfunsym) != pc);
	    return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);

If this ever happens again in the field, we have more chances to hear about it than if it's a warning that users can easily overlook.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 27 Nov 2019 23:00:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review] Avoid infinite recursion in find_pc_sect_line

[Kevin Buettner (Code Review)]

Kevin Buettner has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> I'm not 100% comfortable in merging fixes like this, in an area that's already very tricky, without reproducer.  I'm not strongly opposing either, but just sharing my opinion.
> 
> It just doesn't sound logical.  To reach this "else", we need to:
> 
> 1. lookup a minsym for PC, that minsym is a trampoline.  It means that this trampoline minsym is the closest (smaller or equal) minsym from PC, all minsym types considered.
> 2. lookup a _text_ minsym by name, we find one that is exactly equal to pc
> 
> If that text minsym we have found in #2 is equal to PC, why did we not find it in #1?  The only way I see is for the two minsyms (the trampoline one and the text one) to have the same value.  The lookup function would have returned the trampoline one, because it can only return one symbol, so it has to choose, and the trampoline one happens to come before.  Then, we search specifically text symbols by name, we find the text one.  But that case would already handled with the existing
> 
> 	else if (BMSYMBOL_VALUE_ADDRESS (mfunsym)
> 		 == BMSYMBOL_VALUE_ADDRESS (msymbol))
> 
> So the only explanation I can imagine is that there was another GDB bug, causing it to mess up the minsym creation or lookup.  And either:
> 
> - that bug was fixed, and this fix is no longer needed
> - that bug still isn't fixed, in which case this fix just covers up the problem.
> 
> I think you are already conscious of this, since you added a warning to tell the user that this is not expected.  But if this is a condition we expect to never legitimately happen, I would suggest using an assertion instead:
> 
> 	    gdb_assert (BMSYMBOL_VALUE_ADDRESS (mfunsym) != pc);
> 	    return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
> 
> If this ever happens again in the field, we have more chances to hear about it than if it's a warning that users can easily overlook.

As a GDB user, I'd prefer that GDB do its best to provide a reasonable result. However, as I developer, I too, really want a bug report. I don't have a strong preference either way, so I went (mostly) with your suggestion, but the new patch calls internal_error instead of using an assert. This way, I could include a plea to file a bug report. (Though that may be obvious if you get an assertion failure.)

Anyway, the new patch can be found here:

https://www.sourceware.org/ml/gdb-patches/2019-12/msg00168.html

(I didn't use gerrit to post it.)


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 04 Dec 2019 22:04:44 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[pushed] Avoid infinite recursion in find_pc_sect_line

[Sourceware to Gerrit sync (Code Review)]

The original change was created by Kevin Buettner.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................

Avoid infinite recursion in find_pc_sect_line

A patch somewhat like this patch has been in Fedora GDB for well over
a decade.  The Fedora patch was written by Jan Kratochvil.  The Fedora
version prints a warning and attempts to continue.  This version will
error out, fatally.  An earlier version of this patch was more like
the Fedora version than this one.  Simon Marchi recommended use of an
assertion to test for the infinite recursion; I decided to use an
explicit test (with an "if" statement) along with a call to
internal_error() if the condition is met.  This way, I could include
a plea to file a bug report.

It was motivated by a customer reported bug (back in 2006!) which
showed infinite mutual recursion between find_pc_sect_line and
find_pc_line.  Here is a portion of the backtrace from the bug report:

    (gdb) bt
    #0  0x00000000004450a4 in lookup_minimal_symbol_by_pc_section (
	pc=251700325328, section=0x570f500) at gdb/minsyms.c:484
    #1  0x00000000004bbfb2 in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2057
    #2  0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #3  0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081

    ...   (lots and lots of the same two functions with the same parameters)

    #1070 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1071 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1072 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1073 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1074 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1075 0x00000000004bc1ff in find_pc_sect_line (pc=251696794399,
	section=0x59b0df8, notcurrent=0) at gdb/symtab.c:2081
    #1076 0x00000000004bc480 in find_pc_line (pc=251696794399, notcurrent=0)
	at gdb/symtab.c:2232
    #1077 0x000000000055550e in find_frame_sal (frame=0xb3f3e0, sal=0x7fff1d1a8200)
	at gdb/frame.c:1392
    #1078 0x00000000004d86fd in set_current_sal_from_frame (frame=0x1648, center=1)
	at gdb/stack.c:379
    #1079 0x00000000004cf137 in normal_stop () at gdb/infrun.c:3147
    ...

The test case was a large application.  Attempts were made to make a
small(er) test case, but those attempts were not successful.
Therefore, I cannot provide a new test for this patch.

That said, we ought to guard against recursively calling
find_pc_sect_line (via find_pc_line) with the identical PC value that
it had been called with.  Should this happen, infinite recursion (as
shown in the above backtrace) is the result.  This patch prevents
that from happening.

If this should happens, there is a bug somewhere, perhaps in GDB, perhaps
in some other part of the toolchain or a library.  We error out fatally
with a message briefly describing the condition along with a plea to file
a bug report.

I spent some time looking at the surrounding code and commentary which
handle the case of PC being in a stub/trampoline.  It first appeared
in the public GDB repository in April, 1999.  The ChangeLog entry for
this commit is from 1998-12-31.  The relevant portion is:

	(find_pc_sect_line): Return correct information if pc is in import
	or export stub (trampoline).

What's remarkable about the overall ChangeLog entry is that it's over
2500+ lines long!  I believe that this was part of the infamous "HP
merge" (in which insufficient due diligence was given in accepting
a large batch of changes from an outside source).  In the years that
followed, much of this code was either significantly revised or
outright removed.

For this particular case, I'm grateful that extensive comments were
provided by "RT".  (I haven't been able to figure out who RT is/was.)
I've decided against attempting to revise this stub/trampoline handling
code any further than adding Jan's test which prevents an obvious case
of infinite recursion.

I've tested on Fedora 31, x86-64.  I see no regressions.  I've also
searched the logfile for the new message, but as expected, no message
was found (which is good).

gdb/ChangeLog:

	* symtab.c (find_pc_sect_line): Add check which prevents infinite
	recursion.

Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
---
M gdb/ChangeLog
M gdb/symtab.c
2 files changed, 16 insertions(+), 1 deletion(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8fc63c..f6bd31c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-11  Kevin Buettner  <kevinb@redhat.com>
+
+	* symtab.c (find_pc_sect_line): Add check which prevents infinite
+	recursion.
+	
 2020-03-11  Simon Marchi  <simon.marchi@efficios.com>
 
 	* configure: Re-generate.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 44b7113..aa415a9 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3158,7 +3158,17 @@
 	  ;
 	/* fall through */
 	else
-	  return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
+	  {
+	    /* Detect an obvious case of infinite recursion.  If this
+	       should occur, we'd like to know about it, so error out,
+	       fatally.  */
+	    if (BMSYMBOL_VALUE_ADDRESS (mfunsym) == pc)
+	      internal_error (__FILE__, __LINE__,
+	        _("Infinite recursion detected in find_pc_sect_line;"
+		  "please file a bug report"));
+
+	    return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
+	  }
       }
 
   symtab_and_line val;

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 2
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset

[pushed] Avoid infinite recursion in find_pc_sect_line

[Sourceware to Gerrit sync (Code Review)]

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/682
......................................................................

Avoid infinite recursion in find_pc_sect_line

A patch somewhat like this patch has been in Fedora GDB for well over
a decade.  The Fedora patch was written by Jan Kratochvil.  The Fedora
version prints a warning and attempts to continue.  This version will
error out, fatally.  An earlier version of this patch was more like
the Fedora version than this one.  Simon Marchi recommended use of an
assertion to test for the infinite recursion; I decided to use an
explicit test (with an "if" statement) along with a call to
internal_error() if the condition is met.  This way, I could include
a plea to file a bug report.

It was motivated by a customer reported bug (back in 2006!) which
showed infinite mutual recursion between find_pc_sect_line and
find_pc_line.  Here is a portion of the backtrace from the bug report:

    (gdb) bt
    #0  0x00000000004450a4 in lookup_minimal_symbol_by_pc_section (
	pc=251700325328, section=0x570f500) at gdb/minsyms.c:484
    #1  0x00000000004bbfb2 in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2057
    #2  0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #3  0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081

    ...   (lots and lots of the same two functions with the same parameters)

    #1070 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1071 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1072 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1073 0x00000000004bc1ff in find_pc_sect_line (pc=251700325328,
	section=0x570f500, notcurrent=0) at gdb/symtab.c:2081
    #1074 0x00000000004bc480 in find_pc_line (pc=251700325328, notcurrent=0)
	at gdb/symtab.c:2232
    #1075 0x00000000004bc1ff in find_pc_sect_line (pc=251696794399,
	section=0x59b0df8, notcurrent=0) at gdb/symtab.c:2081
    #1076 0x00000000004bc480 in find_pc_line (pc=251696794399, notcurrent=0)
	at gdb/symtab.c:2232
    #1077 0x000000000055550e in find_frame_sal (frame=0xb3f3e0, sal=0x7fff1d1a8200)
	at gdb/frame.c:1392
    #1078 0x00000000004d86fd in set_current_sal_from_frame (frame=0x1648, center=1)
	at gdb/stack.c:379
    #1079 0x00000000004cf137 in normal_stop () at gdb/infrun.c:3147
    ...

The test case was a large application.  Attempts were made to make a
small(er) test case, but those attempts were not successful.
Therefore, I cannot provide a new test for this patch.

That said, we ought to guard against recursively calling
find_pc_sect_line (via find_pc_line) with the identical PC value that
it had been called with.  Should this happen, infinite recursion (as
shown in the above backtrace) is the result.  This patch prevents
that from happening.

If this should happens, there is a bug somewhere, perhaps in GDB, perhaps
in some other part of the toolchain or a library.  We error out fatally
with a message briefly describing the condition along with a plea to file
a bug report.

I spent some time looking at the surrounding code and commentary which
handle the case of PC being in a stub/trampoline.  It first appeared
in the public GDB repository in April, 1999.  The ChangeLog entry for
this commit is from 1998-12-31.  The relevant portion is:

	(find_pc_sect_line): Return correct information if pc is in import
	or export stub (trampoline).

What's remarkable about the overall ChangeLog entry is that it's over
2500+ lines long!  I believe that this was part of the infamous "HP
merge" (in which insufficient due diligence was given in accepting
a large batch of changes from an outside source).  In the years that
followed, much of this code was either significantly revised or
outright removed.

For this particular case, I'm grateful that extensive comments were
provided by "RT".  (I haven't been able to figure out who RT is/was.)
I've decided against attempting to revise this stub/trampoline handling
code any further than adding Jan's test which prevents an obvious case
of infinite recursion.

I've tested on Fedora 31, x86-64.  I see no regressions.  I've also
searched the logfile for the new message, but as expected, no message
was found (which is good).

gdb/ChangeLog:

	* symtab.c (find_pc_sect_line): Add check which prevents infinite
	recursion.

Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
---
M gdb/ChangeLog
M gdb/symtab.c
2 files changed, 16 insertions(+), 1 deletion(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8fc63c..f6bd31c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-11  Kevin Buettner  <kevinb@redhat.com>
+
+	* symtab.c (find_pc_sect_line): Add check which prevents infinite
+	recursion.
+	
 2020-03-11  Simon Marchi  <simon.marchi@efficios.com>
 
 	* configure: Re-generate.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 44b7113..aa415a9 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3158,7 +3158,17 @@
 	  ;
 	/* fall through */
 	else
-	  return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
+	  {
+	    /* Detect an obvious case of infinite recursion.  If this
+	       should occur, we'd like to know about it, so error out,
+	       fatally.  */
+	    if (BMSYMBOL_VALUE_ADDRESS (mfunsym) == pc)
+	      internal_error (__FILE__, __LINE__,
+	        _("Infinite recursion detected in find_pc_sect_line;"
+		  "please file a bug report"));
+
+	    return find_pc_line (BMSYMBOL_VALUE_ADDRESS (mfunsym), 0);
+	  }
       }
 
   symtab_and_line val;

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I595470be6ab5f61ca7e4e9e70c61a252c0deaeaa
Gerrit-Change-Number: 682
Gerrit-PatchSet: 2
Gerrit-Owner: Kevin Buettner <kevinb@redhat.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: merged