[rfc] Do not make up line information

[rfc] Do not make up line information

[Daniel Jacobowitz]

This patch is for a similar problem to the one I was discussing with Michael
Eager on gdb@ a few weeks ago.  Suppose we've got a file that looks like
this:

0x1000	foo		(from foo.o, which has debug info)
0x2000	bar		(from bar.o, no debug info)
0x3000	foo_init	(from foo.o ".text.init")

We'll build a psymtab for foo.o which says it covers 0x1000-0x3010 or so.
This is already bogus, but that's a problem for another day; we need to do
away with explicit hi/lo addresses and record the exact ranges.  Meanwhile,
though, suppose the user says "break bar".  From minsym_found, we try to
find the source line containing that minimal symbol (it's not clear why, but
that's what we do).  We get to find_pc_sect_line with the symtab for foo.o,
which is "closest" by some definition.

We'll never return a line number for function "foo", because with modern
debug info we'll have an end marker for that part of the line table,
represented as an entry for line 0.  But if the code for foo_init has
inlined code from any header file that foo doesn't have, we'll have another
symtab sharing the same blockvector whose lowest PC value is after "bar".
We'll select that as the "best match", since it's the closest thing
following bar.  We'll subtract one from the line number (probably putting us
on the start of an inline function definition) and report that as the
line for bar.

My test case was a Linux kernel image.  __irq_svc has no debug info;
and .text is very large.  So this is absurdly far off:

(gdb) p __irq_svc
$1 = {<text variable, no debug info>} 0xc0024a40 <__irq_svc>
(gdb) b __irq_svc
Breakpoint 1 at 0xc02cf390: file sctp/structs.h, line 1791.

Oops, we jumped about 3MB forward.

I think that we should parallel the behavior for the end of the line table
when handling the start of the line table.  If the line table doesn't cover
the line in question, then return no line.  With this patch, the breakpoint
gets set at exactly the start of __irq_svc.

No regressions on x86_64-pc-linux-gnu, and it seems right to me.  Any
opinions on this patch, or shall I commit it?

-- 
Daniel Jacobowitz
CodeSourcery

2006-11-22  Daniel Jacobowitz  <dan@codesourcery.com>

	* symtab.c (find_pc_sect_line): Do not return a line before
	the start of a symtab.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.148
diff -u -p -r1.148 symtab.c
--- symtab.c	17 Oct 2006 20:17:44 -0000	1.148
+++ symtab.c	22 Nov 2006 15:26:33 -0000
@@ -2222,23 +2222,11 @@ find_pc_sect_line (CORE_ADDR pc, struct 
 
   if (!best_symtab)
     {
-      if (!alt_symtab)
-	{			/* If we didn't find any line # info, just
-				   return zeros.  */
-	  val.pc = pc;
-	}
-      else
-	{
-	  val.symtab = alt_symtab;
-	  val.line = alt->line - 1;
-
-	  /* Don't return line 0, that means that we didn't find the line.  */
-	  if (val.line == 0)
-	    ++val.line;
-
-	  val.pc = BLOCK_END (BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK));
-	  val.end = alt->pc;
-	}
+      /* If we didn't find any line number info, just return zeros.
+	 We used to return alt->line - 1 here, but that could be
+	 anywhere; if we don't have line number info for this PC,
+	 don't make some up.  */
+      val.pc = pc;
     }
   else if (best->line == 0)
     {

Re: [rfc] Do not make up line information

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> We'll never return a line number for function "foo", because with modern
> debug info we'll have an end marker for that part of the line table,
> represented as an entry for line 0.  But if the code for foo_init has
> inlined code from any header file that foo doesn't have, we'll have another
> symtab sharing the same blockvector whose lowest PC value is after "bar".
> We'll select that as the "best match", since it's the closest thing
> following bar.  We'll subtract one from the line number (probably putting us
> on the start of an inline function definition) and report that as the
> line for bar.

Wow --- that's weird code.  Where did that alt->line - 1 come from?

If we can't find the original motivation, the change looks like an
obvious improvement to me.

Re: [rfc] Do not make up line information

[Daniel Jacobowitz]

On Wed, Nov 22, 2006 at 11:54:13AM -0800, Jim Blandy wrote:
> Wow --- that's weird code.  Where did that alt->line - 1 come from?
> 
> If we can't find the original motivation, the change looks like an
> obvious improvement to me.

It predates all our changelogs.  I imagine it predates debug info for
inlining, and DWARF-2, too.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Do not make up line information

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> On Wed, Nov 22, 2006 at 11:54:13AM -0800, Jim Blandy wrote:
>> Wow --- that's weird code.  Where did that alt->line - 1 come from?
>> 
>> If we can't find the original motivation, the change looks like an
>> obvious improvement to me.
>
> It predates all our changelogs.  I imagine it predates debug info for
> inlining, and DWARF-2, too.

It seems to me that, when one can't find the rationale for something
weird after a responsible look, it's best to just replace it with
something reasonable and see what happens.  Otherwise, the overall
dynamic is for things to become choked with obscure bits that nobody
can test alternatives to.

So I'd say, commit it.

Re: [rfc] Do not make up line information

[Daniel Jacobowitz]

On Wed, Nov 22, 2006 at 01:59:43PM -0800, Jim Blandy wrote:
> So I'd say, commit it.

Done.

-- 
Daniel Jacobowitz
CodeSourcery