Re: [PATCH][gdb/symtab] Ignore zero line table entries

[Simon Marchi <simark@simark.ca>]

On 2020-07-16 1:49 p.m., Tom de Vries wrote:
> Hi,
> 
> The DWARF standard states for the line register in the line number information
> state machine the following:
> ...
> An unsigned integer indicating a source line number.  Lines are numbered
> beginning at 1.  The compiler may emit the value 0 in cases where an
> instruction cannot be attributed to any source line.
> ...
> 
> So, it's possible to have a zero line number in the DWARF line table.
> 
> This is currently not handled by GDB.  The zero value is read in as any other
> line number, but internally the zero value has a special meaning:
> end-of-sequence, so the line table entry ends up having a different
> interpretation than intended in some situations.
> 
> I've created a test-case where various aspects are tested, which has these 4
> interesting tests.
> 
> 1. Next-step through a zero-line instruction, is_stmt == 1
> gdb.dwarf2/dw2-line-number-zero.exp: bar1, 2nd next
> 
> 2. Next-step through a zero-line instruction, is_stmt == 0
> gdb.dwarf2/dw2-line-number-zero.exp: bar2, 2nd next
> 
> 3. Show source location at zero-line instruction, is_stmt == 1
> gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar1_label_3
> 
> 4. Show source location at zero-line instruction, is_stmt == 0
> gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar2_label_3
> 
> And we have the following results:
> 
> 8.3.1, 9.2:
> ...
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar1, 2nd next
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: bar2, 2nd next
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar1_label_3
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar2_label_3
> ...
> 
> commit 8c95582da8 "gdb: Add support for tracking the DWARF line table is-stmt
> field":
> ...
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: bar1, 2nd next
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: bar2, 2nd next
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar1_label_3
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar2_label_3
> ...
> 
> commit d8cc8af6a1 "[gdb/symtab] Fix line-table end-of-sequence sorting",
> master:
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar1, 2nd next
> FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar2, 2nd next
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar1_label_3
> PASS: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar2_label_3
> ...
> 
> The regression in test 2 at commit d8cc8af6a1 was filed as PR symtab/26243,
> where clang emits zero line numbers.
> 
> The way to fix all tests is to make sure line number zero internally doesn't
> clash with special meaning values, and by handling it appropriately
> everywhere.  That however looks too intrusive for the GDB 10 release.
> 
> Instead, we decide to ensure defined behaviour for line number zero by
> ignoring it.  This gives us back the test results from before commit
> d8cc8af6a1, fixing PR26243.
> 
> We mark the FAILs for tests 3 and 4 as KFAILs.  Test 4 was already failing for
> the 9.2 release, and we consider the regression of test 3 from gdb 9.2 to gdb
> 10 the cost for having defined behaviour.
> 
> Build and reg-tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/symtab] Ignore zero line table entries
> 
> gdb/ChangeLog:
> 
> 2020-07-16  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/26243
> 	* dwarf2/read.c (lnp_state_machine::record_line): Ignore zero line
> 	entries.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-07-16  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/26243
> 	* gdb.dwarf2/dw2-line-number-zero.c: New test.
> 	* gdb.dwarf2/dw2-line-number-zero.exp: New file.

Ok, so the state as of today is that we have three patches for 26243:

Tom's patch (this one): filter out line 0 entres at the DWARF reader level

Andrew's patch [1]: replace internal references to line 0 (which means an end marker
                    in the line table) with an explicit linetable_entry::end_marker,
                    but otherwise don't try to handle the "real" line 0 entries coming
                    from the DWARF

Pedro's patch [2]: be smarter when stepping into or from some instructions that map to
                   line 0

Pedro's patch tackles one consequence of having "line 0" entries in the line table,
related to stepping.  But they have other consequences than stepping, like the
"disassemble /s" issue that I raised in response to Andrew's patch.

So the safe bet would be: merge Tom's patch for GDB 10 and keep the others for after
the branch creation would.  It brings the GDB 10 behavior back to what GDB 9 was, where
we pretend that the instruction mapped to line 0 belongs to the line of the previous
instruction.

Or, we go straight away with Andrew's patch that switches the end markers to -1,
Pedro's patch that makes stepping deal with "line 0" entries, and give a push for
GDB 10 to try to find and fix any other issues that this would cause.

I don't really mind, but I think it would be good to clear that up first to know where
we are going.

Simon

[1] https://sourceware.org/pipermail/gdb-patches/2020-July/170588.html
[2] https://sourceware.org/pipermail/gdb-patches/2020-July/170645.html