This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] Rationalize prologue skipping on break FILE:LINE syntax.


This is something that I noticed while working on an issue that is
Ada-specific.  I can't contribute the Ada-specific part right now,
because it is inside a part of the ada-lang.c code that I submitted
a while ago, but didn't commit because of some (valid) reservations.
But the part in breakpoint.c seems relevant and useful, so here it is.

What I noticed was that breakpoint.c:expand_line_sal_maybe was taking
care of skipping the function prologue in the case where it is passed
multiple sals, whereas it wasn't when it was given just one. It seemed
inconsistent.

The reason why it matters to Ada is that we were computing SALS, then
adjusting the SAL's PC our way - without adjusting the SAL's line.
That was bad, because we then had an inconsistent SAL where the line
number and the PC don't correspond.  As a result, we tripped an assertion
during the expasion, because expand_line_sal_maybe was looking up PCs
for a given line, while our SAL was for a different line.  So none of
the SALs that expand_line_sal_maybe found matched the PC of the ones
we gave...

So the obvious fix in Ada was to update both line and PC in our SALs.
But testing revealed that this broke another situation involving
inlining, because the line number adjustment. We have a procedure
called Callee that looks like this:

  3 procedure Callee is
  4    [...]
  5    [...]
  6 begin
  7    Data := 0; --  BREAK
  8 end Callee;

And then we have another function which calls this routine.  We compiled
the code with -O2 and a switch that forces inlining.  As a result, we
get line 7 inlined in our calling function.

After the change I made on the Ada side, what happened is that GDB
searched line 7, found it in procedure Callee, skipped the prologue
(there was actually none on the archecture we were on - sparc-solaris),
and thus ended up adjusting the SAL to line 8.

But the problem is that, technically, line 8 is not inlined in our caller.
So, during the expansion, we found only one location instead of two,
because we were looking for line 8 instead of the original line 7.

What I thought is that the expansion needs to know about the original
line, or it cannot do its job properly. And in fact, it does do the
prologue-skipping in the case of multi-location SALs. But it does not
do that in the case of single-location ones, presumably because it
assumes that the skipping has already been done. This is not consistent,
and also forces all callers to do the skipping themselves, whereas
they really should not need to.

This patch is the first step towards moving the prologue-skipping
entirely in expand_line_sal_maybe, regardless of the number of entries
in the SAL...  Next up, if there is no objection, I can start looking
at the locations where we can remove the prologue-skipping because it
would be redundant with the skipping now always done during the expansion.

Thoughts?

gdb/
        * breakpoint.c (expand_line_sal_maybe): Adjust adjust the SAL
        past the function prologue in the case where we were given only
        one SAL.

Tested on x86_64-linux.  I also tested this with the AdaCore testsuite
on sparc-solaris.

Thanks,
-- 
Joel
 gdb/breakpoint.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c6140b0..bdef130 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6212,13 +6212,14 @@ expand_line_sal_maybe (struct symtab_and_line sal)
 
   if (expanded.nelts == 1)
     {
-      /* We had one sal, we got one sal.  Without futher
-	 processing, just return the original sal.  */
+      /* We had one sal, we got one sal.  Return that sal, adjusting it
+         past the function prologue if necessary.  */
       xfree (expanded.sals);
       expanded.nelts = 1;
       expanded.sals = xmalloc (sizeof (struct symtab_and_line));
       sal.pc = original_pc;
       expanded.sals[0] = sal;
+      skip_prologue_sal (&expanded.sals[0]);
       do_cleanups (old_chain);
       return expanded;      
     }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]