Re: [PATCH] gdb: Don't hard code 0 as end marker in GDB's line table

Simon Marchi
2020-07-20T16:01:23Z

On 2020-07-20 8:55 a.m., Andrew Burgess wrote:
> Currently GDB hard-codes the use of 0 as the end of sequence marker in
> its line tables. After this commit GDB now uses a named
> constant (linetable_entry::end_marker) as the end of sequence marker.
> The value of this constant is -1, not 0. This change was made in
> order to aid fixing bug PR gdb/26243.
>
> Bug PR gdb/26243 is about allowing line number 0 to be used to
> indicate a program address that has no corresponding source line (this
> use is specified in the DWARF standard). Currently GDB can't use line
> number 0 as this line number is hard coded as the end of sequence
> marker, but, after this commit line number 0 no longer has any special
> meaning.
>
> This commit does not fix PR gdb/26243, but it is step towards allowing
> that issue to be fixed.
>
> gdb/ChangeLog:
>
> PR gdb/26243
> * buildsym.c (buildsym_compunit::record_line): Add an assert for
> the incoming line number. Update comments to not mention 0
> specifically. Update to check for linetable_entry::end_marker
> rather than 0.
> * disasm.c (do_mixed_source_and_assembly_deprecated): Check for
> linetable_entry::end_marker not 0.
> (do_mixed_source_and_assembly): Likewise.
> * dwarf2/read.c (dwarf_finish_line): Pass
> linetable_entry::end_marker not 0.
> * mdebugread.c (add_line): Set is_stmt field.
> * python/py-linetable.c (ltpy_get_all_source_lines): Check for
> linetable_entry::end_marker not 0, update comments.
> (ltpy_iternext): Likewise.
> * record-btrace.c (btrace_find_line_range): Likewise.
> * symmisc.c (maintenance_print_one_line_table): Likewise.
> * symtab.c (find_pc_sect_line): Likewise.
> (find_line_symtab): Likewise.
> (skip_prologue_using_sal): Likewise.
> * symtab.h (linetable_entry::end_marker): New const static member
> variable. Add a static assert for this field.
> * xcoffread.c (arrange_linetable): Check for
> linetable_entry::end_marker not 0.

I think the idea is good. But we need to make sure to audit all the places
that use linetable_entry::line to see if they now need to treat the value 0
specially. One case that came to mind while reading your patch is
"disassemble /s". Here's an example using bug 26243's reproducer, showing
the end of "disassemble /s tree_insert". The instruction at 0x40135f maps
to line 0.

This is with GDB 8.3:

---
38 else
39 root->right = make_node(val);
0x000000000040134a <+138>: mov -0xc(%rbp),%edi
0x000000000040134d <+141>: callq 0x401250 <make_node(int)>
0x0000000000401352 <+146>: mov -0x8(%rbp),%rcx
0x0000000000401356 <+150>: mov %rax,0x10(%rcx)

40 }
0x000000000040135a <+154>: jmpq 0x40135f <tree_insert(node*, int)+159>
0x000000000040135f <+159>: jmpq 0x401364 <tree_insert(node*, int)+164>

41 }
0x0000000000401364 <+164>: add $0x10,%rsp
0x0000000000401368 <+168>: pop %rbp
0x0000000000401369 <+169>: retq
---

This made 0x40135f appear as if it was part of the previous line.

This is with GDB master:

---
38 else
39 root->right = make_node(val);
0x000000000040134a <+138>: mov -0xc(%rbp),%edi
0x000000000040134d <+141>: call 0x401250 <_Z9make_nodei>
0x0000000000401352 <+146>: mov -0x8(%rbp),%rcx
0x0000000000401356 <+150>: mov %rax,0x10(%rcx)

40 }
0x000000000040135a <+154>: jmp 0x40135f <_Z11tree_insertP4nodei+159>

unknown:
--- no source info for this pc ---
0x000000000040135f <+159>: jmp 0x401364 <_Z11tree_insertP4nodei+164>

test.cpp:
26 {
27 if (val < root->id)
28 {
29 if (root->left)
30 tree_insert (root->left, val);
31 else
32 root->left = make_node(val);
33 }
34 else if (val > root->id)
35 {
36 if (root->right)
37 tree_insert (root->right, val);
38 else
39 root->right = make_node(val);
40 }
41 }
0x0000000000401364 <+164>: add $0x10,%rsp
0x0000000000401368 <+168>: pop %rbp
0x0000000000401369 <+169>: ret
End of assembler dump.
---

I don't know what happens here, but it doesn't look good. The "no source info for this pc"
part is ok, but the "unknown:" and source lines 26-40 look wrong.

This is with GDB master + your patch:

---
38 else
39 root->right = make_node(val);
0x000000000040134a <+138>: mov -0xc(%rbp),%edi
0x000000000040134d <+141>: call 0x401250 <_Z9make_nodei>
0x0000000000401352 <+146>: mov -0x8(%rbp),%rcx
0x0000000000401356 <+150>: mov %rax,0x10(%rcx)

40 }
0x000000000040135a <+154>: jmp 0x40135f <_Z11tree_insertP4nodei+159>

Line number 0 out of range; test.cpp has 80 lines.
---

The disassembly gets interrupted, because the line number is invalid.

In this case, line 0 should probably get handled specially so that we print
the "no source info for this pc" message and keep printing the rest correctly.

So, I think we just need to be careful and look at the possible behavior changes
of all these spots, not just blindly change `line == 0` for `line == end_marker`.

Simon