* [PATCH] gdb: Recognize -1 as a tombstone value in .debug_line @ 2020-06-30 23:18 Fangrui Song 2020-07-01 8:26 ` Andrew Burgess 0 siblings, 1 reply; 4+ messages in thread From: Fangrui Song @ 2020-06-30 23:18 UTC (permalink / raw) To: gdb-patches; +Cc: Fangrui Song LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to represent a relocation in .debug_line referencing a discarded symbol. Recognize -1 to fix gdb.base/break-on-linker-gcd-function.exp when the linker is a newer LLD. gdb/ChangeLog: * dwarf2/read.c (lnp_state_machine::check_line_address): Test -1. --- gdb/dwarf2/read.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index b097f624b6..7cf2691ae9 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -20380,9 +20380,13 @@ lnp_state_machine::check_line_address (struct dwarf2_cu *cu, /* If ADDRESS < UNRELOCATED_LOWPC then it's not a usable value, it's outside the pc range of the CU. However, we restrict the test to only ADDRESS values of zero to preserve GDB's previous behaviour which is to handle - the specific case of a function being GC'd by the linker. */ + the specific case of a function being GC'd by the linker. - if (address == 0 && address < unrelocated_lowpc) + LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to represent + the tombstone value. + */ + + if ((address == 0 && address < unrelocated_lowpc) || address == (CORE_ADDR)-1) { /* This line table is for a function which has been GCd by the linker. Ignore it. PR gdb/12528 */ -- 2.27.0.212.ge8ba1cc988-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: Recognize -1 as a tombstone value in .debug_line 2020-06-30 23:18 [PATCH] gdb: Recognize -1 as a tombstone value in .debug_line Fangrui Song @ 2020-07-01 8:26 ` Andrew Burgess 2020-07-01 17:39 ` [PATCH v2] " Fangrui Song 0 siblings, 1 reply; 4+ messages in thread From: Andrew Burgess @ 2020-07-01 8:26 UTC (permalink / raw) To: Fangrui Song; +Cc: gdb-patches * Fangrui Song via Gdb-patches <gdb-patches@sourceware.org> [2020-06-30 16:18:42 -0700]: > LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to > represent a relocation in .debug_line referencing a discarded symbol. > Recognize -1 to fix gdb.base/break-on-linker-gcd-function.exp when the > linker is a newer LLD. > > gdb/ChangeLog: > > * dwarf2/read.c (lnp_state_machine::check_line_address): Test -1. > --- > gdb/dwarf2/read.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index b097f624b6..7cf2691ae9 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -20380,9 +20380,13 @@ lnp_state_machine::check_line_address (struct dwarf2_cu *cu, > /* If ADDRESS < UNRELOCATED_LOWPC then it's not a usable value, it's outside > the pc range of the CU. However, we restrict the test to only ADDRESS > values of zero to preserve GDB's previous behaviour which is to handle > - the specific case of a function being GC'd by the linker. */ > + the specific case of a function being GC'd by the linker. > > - if (address == 0 && address < unrelocated_lowpc) > + LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to represent > + the tombstone value. > + */ If I understand this correctly then "tombstone value" here is the same as "a function being GC'd by the linker". If that's correct then could you just use that phrase please as it is clearer, and matches the terminology already used in the comment. The trailing '*/' should be on the same line as the end of the comment, with two whitespace before it, so '...value. */'. > + > + if ((address == 0 && address < unrelocated_lowpc) || address == (CORE_ADDR)-1) There should be a whitespace after the cast, so '(CORE_ADDR) -1'. Thanks, Andrewx > { > /* This line table is for a function which has been > GCd by the linker. Ignore it. PR gdb/12528 */ > -- > 2.27.0.212.ge8ba1cc988-goog > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] gdb: Recognize -1 as a tombstone value in .debug_line 2020-07-01 8:26 ` Andrew Burgess @ 2020-07-01 17:39 ` Fangrui Song 2020-07-01 19:17 ` Tom Tromey 0 siblings, 1 reply; 4+ messages in thread From: Fangrui Song @ 2020-07-01 17:39 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 2400 bytes --] Thanks for review. Attached PATCH v2 with comment adjustment. On 2020-07-01, Andrew Burgess wrote: >* Fangrui Song via Gdb-patches <gdb-patches@sourceware.org> [2020-06-30 16:18:42 -0700]: > >> LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to >> represent a relocation in .debug_line referencing a discarded symbol. >> Recognize -1 to fix gdb.base/break-on-linker-gcd-function.exp when the >> linker is a newer LLD. >> >> gdb/ChangeLog: >> >> * dwarf2/read.c (lnp_state_machine::check_line_address): Test -1. >> --- >> gdb/dwarf2/read.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >> index b097f624b6..7cf2691ae9 100644 >> --- a/gdb/dwarf2/read.c >> +++ b/gdb/dwarf2/read.c >> @@ -20380,9 +20380,13 @@ lnp_state_machine::check_line_address (struct dwarf2_cu *cu, >> /* If ADDRESS < UNRELOCATED_LOWPC then it's not a usable value, it's outside >> the pc range of the CU. However, we restrict the test to only ADDRESS >> values of zero to preserve GDB's previous behaviour which is to handle >> - the specific case of a function being GC'd by the linker. */ >> + the specific case of a function being GC'd by the linker. >> >> - if (address == 0 && address < unrelocated_lowpc) >> + LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to represent >> + the tombstone value. >> + */ > >If I understand this correctly then "tombstone value" here is the same >as "a function being GC'd by the linker". If that's correct then >could you just use that phrase please as it is clearer, and matches >the terminology already used in the comment. Thanks. Reworded the comment added by c3b7b696c231416ac90fd9cb7d5ce735b3683356 a bit. >The trailing '*/' should be on the same line as the end of the >comment, with two whitespace before it, so '...value. */'. >> + >> + if ((address == 0 && address < unrelocated_lowpc) || address == (CORE_ADDR)-1) > >There should be a whitespace after the cast, so '(CORE_ADDR) -1'. > >Thanks, >Andrewx clang-format deletes space in `(CORE_ADDR) -1`. I think I need to adjust some options for my minimal .clang-format ... % cat .clang-format BasedOnStyle: GNU >> { >> /* This line table is for a function which has been >> GCd by the linker. Ignore it. PR gdb/12528 */ >> -- >> 2.27.0.212.ge8ba1cc988-goog >> [-- Attachment #2: 0001-gdb-Recognize-1-as-a-tombstone-value-in-.debug_line.patch --] [-- Type: text/x-diff, Size: 2234 bytes --] From 709d115624db44d6f1a60488f1038e0552b9d534 Mon Sep 17 00:00:00 2001 From: Fangrui Song <maskray@google.com> Date: Wed, 1 Jul 2020 10:32:36 -0700 Subject: [PATCH] gdb: Recognize -1 as a tombstone value in .debug_line LLD from 11 onwards (https://reviews.llvm.org/D81784) uses -1 to represent a relocation in .debug_line referencing a discarded symbol. Recognize -1 to fix gdb.base/break-on-linker-gcd-function.exp when the linker is a newer LLD. gdb/ChangeLog: * dwarf2/read.c (lnp_state_machine::check_line_address): Test -1. --- gdb/dwarf2/read.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 4622d14a05..a83e225cb6 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -19983,7 +19983,7 @@ class lnp_state_machine we're processing the end of a sequence. */ void record_line (bool end_sequence); - /* Check ADDRESS is zero and less than UNRELOCATED_LOWPC and if true + /* Check ADDRESS is -1, or zero and less than UNRELOCATED_LOWPC, and if true nop-out rest of the lines in this sequence. */ void check_line_address (struct dwarf2_cu *cu, const gdb_byte *line_ptr, @@ -20377,12 +20377,13 @@ lnp_state_machine::check_line_address (struct dwarf2_cu *cu, const gdb_byte *line_ptr, CORE_ADDR unrelocated_lowpc, CORE_ADDR address) { - /* If ADDRESS < UNRELOCATED_LOWPC then it's not a usable value, it's outside - the pc range of the CU. However, we restrict the test to only ADDRESS - values of zero to preserve GDB's previous behaviour which is to handle - the specific case of a function being GC'd by the linker. */ + /* Linkers resolve a symbolic relocation referencing a GC'd function to 0 or + -1. If ADDRESS is 0, ignoring the opcode will err if the text section is + located at 0x0. In this case, additionaly check that + if ADDRESS < UNRELOCATED_LOWPC. */ - if (address == 0 && address < unrelocated_lowpc) + if ((address == 0 && address < unrelocated_lowpc) + || address == (CORE_ADDR) -1) { /* This line table is for a function which has been GCd by the linker. Ignore it. PR gdb/12528 */ -- 2.27.0.212.ge8ba1cc988-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gdb: Recognize -1 as a tombstone value in .debug_line 2020-07-01 17:39 ` [PATCH v2] " Fangrui Song @ 2020-07-01 19:17 ` Tom Tromey 0 siblings, 0 replies; 4+ messages in thread From: Tom Tromey @ 2020-07-01 19:17 UTC (permalink / raw) To: Fangrui Song via Gdb-patches; +Cc: Andrew Burgess, Fangrui Song >>>>> ">" == Fangrui Song via Gdb-patches <gdb-patches@sourceware.org> writes: >> clang-format deletes space in `(CORE_ADDR) -1`. I think I need to adjust >> some options for my minimal .clang-format ... >> % cat .clang-format >> BasedOnStyle: GNU gdb doesn't use clang-format. I wrote a more comprehensive .clang-format once, as a test, but in the end I couldn't make it produce reasonable-looking output. In particular, long function calls were reformatted illegibly. >> + /* Linkers resolve a symbolic relocation referencing a GC'd function to 0 or >> + -1. If ADDRESS is 0, ignoring the opcode will err if the text section is Two spaces after the period. >> + located at 0x0. In this case, additionaly check that Same here. Also a typo, should be "additionally". This is ok with these fixed. Thank you for the patch. Tom ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-01 19:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-30 23:18 [PATCH] gdb: Recognize -1 as a tombstone value in .debug_line Fangrui Song 2020-07-01 8:26 ` Andrew Burgess 2020-07-01 17:39 ` [PATCH v2] " Fangrui Song 2020-07-01 19:17 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).