* [PATCH] Fix reading .dwp files without .debug_tu_index
@ 2017-09-19 19:58 Alexander Shaposhnikov
2017-09-24 23:37 ` Alexander Shaposhnikov
2017-09-26 3:15 ` Doug Evans via gdb-patches
0 siblings, 2 replies; 7+ messages in thread
From: Alexander Shaposhnikov @ 2017-09-19 19:58 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2653 bytes --]
This patch fixes segmentation fault (due to dereferencing of a null pointer)
in dwarf2read.c in the code dwp_file->cus->version != dwp_file->tus->version
by adding defensive checks similarly to how it's already done
at the lines 11208 - 11215 (in the same file dwarf2read.c).
The issue can be reproduced with dwp packages built by llvm-dwp utility
(from the current upstream) since at the moment llvm-dwp doesn't create
.debug_tu_index section, thus dwp_file->tus will be null.
Test plan:
BUILD:
main.cpp:
int f() {
int x = 0;
int y = 1;
return x + y;
}
g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe
llvm-dwp main.dwo -o main.exe.dwp
# This step is a workaround to a separate issue (unrelated to this patch).
# At the moment llvm tools & clang put .strtab section first (its index is
1),
# while gcc/gold/binutils put it at the end.
# Unfortunately gdb (in the code reading dwps) appears to depend on the
order
# of sections, to workaround this (to reproduce the issue which this patch
# aims to address) we use objcopy to do the following trick:
# if one asks to remove .strtab objcopy will do that but at the same time
# it will create a new .shstrtab section at the end.
objcopy --remove-section .strtab main.exe.dwp
RUN:
gdb main.exe
One can observe that now there is no crash and debugging functionality
works as expected (setting breakpoints, printing local variable, exploring
the stack).
gdb/ChangeLog:
yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com>
* dwarf2read.c: Fix segmentation fault on reading
.dwp files without .debug_tu_index section.
Patch:
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b1914cf876..547e3f034e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void)
dwp_file->tus = create_dwp_hash_table (dwp_file, 1);
/* The DWP file version is stored in the hash table. Oh well. */
- if (dwp_file->cus->version != dwp_file->tus->version)
+ if (dwp_file->cus && dwp_file->tus
+ && dwp_file->cus->version != dwp_file->tus->version)
{
/* Technically speaking, we should try to limp along, but this is
pretty bizarre. We use pulongest here because that's the established
@@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void)
pulongest (dwp_file->cus->version),
pulongest (dwp_file->tus->version), dwp_name.c_str ());
}
- dwp_file->version = dwp_file->cus->version;
+ dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0;
if (dwp_file->version == 2)
bfd_map_over_sections (dwp_file->dbfd, dwarf2_locate_v2_dwp_sections,
Kind regards,
Alexander Shaposhnikov
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Fix reading .dwp files without .debug_tu_index 2017-09-19 19:58 [PATCH] Fix reading .dwp files without .debug_tu_index Alexander Shaposhnikov @ 2017-09-24 23:37 ` Alexander Shaposhnikov 2017-09-26 3:15 ` Doug Evans via gdb-patches 1 sibling, 0 replies; 7+ messages in thread From: Alexander Shaposhnikov @ 2017-09-24 23:37 UTC (permalink / raw) To: gdb-patches, dje, palves cc: Doug Evans, Pedro Alves Kind regards, Alexander Shaposhnikov On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov < alexander.v.shaposhnikov@gmail.com> wrote: > This patch fixes segmentation fault (due to dereferencing of a null > pointer) > in dwarf2read.c in the code dwp_file->cus->version != > dwp_file->tus->version > by adding defensive checks similarly to how it's already done > at the lines 11208 - 11215 (in the same file dwarf2read.c). > The issue can be reproduced with dwp packages built by llvm-dwp utility > (from the current upstream) since at the moment llvm-dwp doesn't create > .debug_tu_index section, thus dwp_file->tus will be null. > > Test plan: > > BUILD: > main.cpp: > int f() { > int x = 0; > int y = 1; > return x + y; > } > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe > llvm-dwp main.dwo -o main.exe.dwp > # This step is a workaround to a separate issue (unrelated to this patch). > # At the moment llvm tools & clang put .strtab section first (its index is > 1), > # while gcc/gold/binutils put it at the end. > # Unfortunately gdb (in the code reading dwps) appears to depend on the > order > # of sections, to workaround this (to reproduce the issue which this patch > # aims to address) we use objcopy to do the following trick: > # if one asks to remove .strtab objcopy will do that but at the same time > # it will create a new .shstrtab section at the end. > objcopy --remove-section .strtab main.exe.dwp > RUN: > gdb main.exe > One can observe that now there is no crash and debugging functionality > works as expected (setting breakpoints, printing local variable, exploring > the stack). > > gdb/ChangeLog: > yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> > > * dwarf2read.c: Fix segmentation fault on reading > > .dwp files without .debug_tu_index section. > > Patch: > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > > index b1914cf876..547e3f034e 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); > > /* The DWP file version is stored in the hash table. Oh well. */ > - if (dwp_file->cus->version != dwp_file->tus->version) > + if (dwp_file->cus && dwp_file->tus > + && dwp_file->cus->version != dwp_file->tus->version) > { > /* Technically speaking, we should try to limp along, but this is > pretty bizarre. We use pulongest here because that's the established > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) > pulongest (dwp_file->cus->version), > pulongest (dwp_file->tus->version), dwp_name.c_str ()); > } > - dwp_file->version = dwp_file->cus->version; > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; > > if (dwp_file->version == 2) > bfd_map_over_sections (dwp_file->dbfd, dwarf2_locate_v2_dwp_sections, > > > Kind regards, > Alexander Shaposhnikov > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix reading .dwp files without .debug_tu_index 2017-09-19 19:58 [PATCH] Fix reading .dwp files without .debug_tu_index Alexander Shaposhnikov 2017-09-24 23:37 ` Alexander Shaposhnikov @ 2017-09-26 3:15 ` Doug Evans via gdb-patches 2017-09-26 4:13 ` Alexander Shaposhnikov 1 sibling, 1 reply; 7+ messages in thread From: Doug Evans via gdb-patches @ 2017-09-26 3:15 UTC (permalink / raw) To: Alexander Shaposhnikov; +Cc: gdb-patches On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> wrote: > This patch fixes segmentation fault (due to dereferencing of a null pointer) > in dwarf2read.c in the code dwp_file->cus->version != dwp_file->tus->version > by adding defensive checks similarly to how it's already done > at the lines 11208 - 11215 (in the same file dwarf2read.c). > The issue can be reproduced with dwp packages built by llvm-dwp utility > (from the current upstream) since at the moment llvm-dwp doesn't create > .debug_tu_index section, thus dwp_file->tus will be null. > > Test plan: > > BUILD: > main.cpp: > int f() { > int x = 0; > int y = 1; > return x + y; > } > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe > llvm-dwp main.dwo -o main.exe.dwp > # This step is a workaround to a separate issue (unrelated to this patch). > # At the moment llvm tools & clang put .strtab section first (its index is > 1), > # while gcc/gold/binutils put it at the end. > # Unfortunately gdb (in the code reading dwps) appears to depend on the > order > # of sections, That's odd. Depends on the order of sections how? > to workaround this (to reproduce the issue which this patch > # aims to address) we use objcopy to do the following trick: > # if one asks to remove .strtab objcopy will do that but at the same time > # it will create a new .shstrtab section at the end. > objcopy --remove-section .strtab main.exe.dwp > RUN: > gdb main.exe > One can observe that now there is no crash and debugging functionality > works as expected (setting breakpoints, printing local variable, exploring > the stack). > > gdb/ChangeLog: > yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> > > * dwarf2read.c: Fix segmentation fault on reading > > .dwp files without .debug_tu_index section. > > Patch: > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > > index b1914cf876..547e3f034e 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); > > /* The DWP file version is stored in the hash table. Oh well. */ > - if (dwp_file->cus->version != dwp_file->tus->version) > + if (dwp_file->cus && dwp_file->tus > + && dwp_file->cus->version != dwp_file->tus->version) > { > /* Technically speaking, we should try to limp along, but this is > pretty bizarre. We use pulongest here because that's the established > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) > pulongest (dwp_file->cus->version), > pulongest (dwp_file->tus->version), dwp_name.c_str ()); > } > - dwp_file->version = dwp_file->cus->version; > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; A version of 0 doesn't seem right. I'd go with something like (untested and likely improperly formatted): if (dwp_file->cus) dwp_file->version = dwp_file-cus->version; else if (dwp_file->tus) dwp_file->version = dwp_file->tus->version else dwp_file->version = 2; ok with that change. > > if (dwp_file->version == 2) > bfd_map_over_sections (dwp_file->dbfd, dwarf2_locate_v2_dwp_sections, > > > Kind regards, > Alexander Shaposhnikov ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix reading .dwp files without .debug_tu_index 2017-09-26 3:15 ` Doug Evans via gdb-patches @ 2017-09-26 4:13 ` Alexander Shaposhnikov 2017-09-26 17:51 ` Doug Evans via gdb-patches 0 siblings, 1 reply; 7+ messages in thread From: Alexander Shaposhnikov @ 2017-09-26 4:13 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches >I'd go with something like (untested and likely improperly formatted): >if (dwp_file->cus) > dwp_file->version = dwp_file-cus->version; > else if (dwp_file->tus) > dwp_file->version = dwp_file->tus->version > else > dwp_file->version = 2; i'm ok with that change, many thanks, do i need to resend the patch ? (and yeah, i can double check / test it when i get home) regarding the dependence on the order of sections - (if i am not mistaken) yeah, it's a separate issue - i can provide more details when i get to my laptop. Best regards, Alexander Shaposhnikov On Mon, Sep 25, 2017 at 8:14 PM, Doug Evans <dje@google.com> wrote: > On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov > <alexander.v.shaposhnikov@gmail.com> wrote: > > This patch fixes segmentation fault (due to dereferencing of a null > pointer) > > in dwarf2read.c in the code dwp_file->cus->version != > dwp_file->tus->version > > by adding defensive checks similarly to how it's already done > > at the lines 11208 - 11215 (in the same file dwarf2read.c). > > The issue can be reproduced with dwp packages built by llvm-dwp utility > > (from the current upstream) since at the moment llvm-dwp doesn't create > > .debug_tu_index section, thus dwp_file->tus will be null. > > > > Test plan: > > > > BUILD: > > main.cpp: > > int f() { > > int x = 0; > > int y = 1; > > return x + y; > > } > > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe > > llvm-dwp main.dwo -o main.exe.dwp > > # This step is a workaround to a separate issue (unrelated to this > patch). > > # At the moment llvm tools & clang put .strtab section first (its index > is > > 1), > > # while gcc/gold/binutils put it at the end. > > # Unfortunately gdb (in the code reading dwps) appears to depend on the > > order > > # of sections, > > That's odd. Depends on the order of sections how? > > > to workaround this (to reproduce the issue which this patch > > # aims to address) we use objcopy to do the following trick: > > # if one asks to remove .strtab objcopy will do that but at the same time > > # it will create a new .shstrtab section at the end. > > objcopy --remove-section .strtab main.exe.dwp > > RUN: > > gdb main.exe > > One can observe that now there is no crash and debugging functionality > > works as expected (setting breakpoints, printing local variable, > exploring > > the stack). > > > > gdb/ChangeLog: > > yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> > > > > * dwarf2read.c: Fix segmentation fault on reading > > > > .dwp files without .debug_tu_index section. > > > > Patch: > > > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > > > > index b1914cf876..547e3f034e 100644 > > --- a/gdb/dwarf2read.c > > +++ b/gdb/dwarf2read.c > > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) > > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); > > > > /* The DWP file version is stored in the hash table. Oh well. */ > > - if (dwp_file->cus->version != dwp_file->tus->version) > > + if (dwp_file->cus && dwp_file->tus > > + && dwp_file->cus->version != dwp_file->tus->version) > > { > > /* Technically speaking, we should try to limp along, but this is > > pretty bizarre. We use pulongest here because that's the > established > > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) > > pulongest (dwp_file->cus->version), > > pulongest (dwp_file->tus->version), dwp_name.c_str ()); > > } > > - dwp_file->version = dwp_file->cus->version; > > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; > > A version of 0 doesn't seem right. > I'd go with something like (untested and likely improperly formatted): > > if (dwp_file->cus) > dwp_file->version = dwp_file-cus->version; > else if (dwp_file->tus) > dwp_file->version = dwp_file->tus->version > else > dwp_file->version = 2; > > ok with that change. > > > > > if (dwp_file->version == 2) > > bfd_map_over_sections (dwp_file->dbfd, > dwarf2_locate_v2_dwp_sections, > > > > > > Kind regards, > > Alexander Shaposhnikov > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix reading .dwp files without .debug_tu_index 2017-09-26 4:13 ` Alexander Shaposhnikov @ 2017-09-26 17:51 ` Doug Evans via gdb-patches 2017-09-26 18:48 ` Alexander Shaposhnikov 0 siblings, 1 reply; 7+ messages in thread From: Doug Evans via gdb-patches @ 2017-09-26 17:51 UTC (permalink / raw) To: Alexander Shaposhnikov; +Cc: gdb-patches On Mon, Sep 25, 2017 at 9:13 PM, Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> wrote: > > >I'd go with something like (untested and likely improperly formatted): > >if (dwp_file->cus) > > dwp_file->version = dwp_file-cus->version; > > else if (dwp_file->tus) > > dwp_file->version = dwp_file->tus->version > > else > > dwp_file->version = 2; > > i'm ok with that change, many thanks, > do i need to resend the patch ? (and yeah, i can double check / test it when > i get home) No need, if you're confident all the nits are taken care of [ChangeLog syntax and contents, patch whitespace, testing, and so on ... :-)] > regarding the dependence on the order of sections - (if i am not mistaken) > yeah, it's a separate issue - i can provide more details > when i get to my laptop. Thanks. > > Best regards, > Alexander Shaposhnikov > > On Mon, Sep 25, 2017 at 8:14 PM, Doug Evans <dje@google.com> wrote: >> >> On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov >> <alexander.v.shaposhnikov@gmail.com> wrote: >> > This patch fixes segmentation fault (due to dereferencing of a null >> > pointer) >> > in dwarf2read.c in the code dwp_file->cus->version != >> > dwp_file->tus->version >> > by adding defensive checks similarly to how it's already done >> > at the lines 11208 - 11215 (in the same file dwarf2read.c). >> > The issue can be reproduced with dwp packages built by llvm-dwp utility >> > (from the current upstream) since at the moment llvm-dwp doesn't create >> > .debug_tu_index section, thus dwp_file->tus will be null. >> > >> > Test plan: >> > >> > BUILD: >> > main.cpp: >> > int f() { >> > int x = 0; >> > int y = 1; >> > return x + y; >> > } >> > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe >> > llvm-dwp main.dwo -o main.exe.dwp >> > # This step is a workaround to a separate issue (unrelated to this >> > patch). >> > # At the moment llvm tools & clang put .strtab section first (its index >> > is >> > 1), >> > # while gcc/gold/binutils put it at the end. >> > # Unfortunately gdb (in the code reading dwps) appears to depend on the >> > order >> > # of sections, >> >> That's odd. Depends on the order of sections how? >> >> > to workaround this (to reproduce the issue which this patch >> > # aims to address) we use objcopy to do the following trick: >> > # if one asks to remove .strtab objcopy will do that but at the same >> > time >> > # it will create a new .shstrtab section at the end. >> > objcopy --remove-section .strtab main.exe.dwp >> > RUN: >> > gdb main.exe >> > One can observe that now there is no crash and debugging functionality >> > works as expected (setting breakpoints, printing local variable, >> > exploring >> > the stack). >> > >> > gdb/ChangeLog: >> > yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> >> > >> > * dwarf2read.c: Fix segmentation fault on reading >> > >> > .dwp files without .debug_tu_index section. >> > >> > Patch: >> > >> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c >> > >> > index b1914cf876..547e3f034e 100644 >> > --- a/gdb/dwarf2read.c >> > +++ b/gdb/dwarf2read.c >> > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) >> > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); >> > >> > /* The DWP file version is stored in the hash table. Oh well. */ >> > - if (dwp_file->cus->version != dwp_file->tus->version) >> > + if (dwp_file->cus && dwp_file->tus >> > + && dwp_file->cus->version != dwp_file->tus->version) >> > { >> > /* Technically speaking, we should try to limp along, but this is >> > pretty bizarre. We use pulongest here because that's the >> > established >> > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) >> > pulongest (dwp_file->cus->version), >> > pulongest (dwp_file->tus->version), dwp_name.c_str ()); >> > } >> > - dwp_file->version = dwp_file->cus->version; >> > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; >> >> A version of 0 doesn't seem right. >> I'd go with something like (untested and likely improperly formatted): >> >> if (dwp_file->cus) >> dwp_file->version = dwp_file-cus->version; >> else if (dwp_file->tus) >> dwp_file->version = dwp_file->tus->version >> else >> dwp_file->version = 2; >> >> ok with that change. >> >> > >> > if (dwp_file->version == 2) >> > bfd_map_over_sections (dwp_file->dbfd, >> > dwarf2_locate_v2_dwp_sections, >> > >> > >> > Kind regards, >> > Alexander Shaposhnikov > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix reading .dwp files without .debug_tu_index 2017-09-26 17:51 ` Doug Evans via gdb-patches @ 2017-09-26 18:48 ` Alexander Shaposhnikov 0 siblings, 0 replies; 7+ messages in thread From: Alexander Shaposhnikov @ 2017-09-26 18:48 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches (updated patch + changelog) changelog: 2017-09-26 Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> * dwarf2read.c (open_and_init_dwp_file): Protect against dwp_file having NULL cus or tus. patch: diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index b3c5fabfc0..4e6bf27364 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11196,7 +11196,8 @@ open_and_init_dwp_file (void) dwp_file->tus = create_dwp_hash_table (dwp_file, 1); /* The DWP file version is stored in the hash table. Oh well. */ - if (dwp_file->cus->version != dwp_file->tus->version) + if (dwp_file->cus && dwp_file->tus + && dwp_file->cus->version != dwp_file->tus->version) { /* Technically speaking, we should try to limp along, but this is pretty bizarre. We use pulongest here because that's the established @@ -11206,7 +11207,13 @@ open_and_init_dwp_file (void) pulongest (dwp_file->cus->version), pulongest (dwp_file->tus->version), dwp_name.c_str ()); } - dwp_file->version = dwp_file->cus->version; + + if (dwp_file->cus) + dwp_file->version = dwp_file->cus->version; + else if (dwp_file->tus) + dwp_file->version = dwp_file->tus->version; + else + dwp_file->version = 2; if (dwp_file->version == 2) bfd_map_over_sections (dwp_file->dbfd, dwarf2_locate_v2_dwp_sections, Kind regards, Alexander Shaposhnikov On Tue, Sep 26, 2017 at 10:51 AM, Doug Evans <dje@google.com> wrote: > On Mon, Sep 25, 2017 at 9:13 PM, Alexander Shaposhnikov > <alexander.v.shaposhnikov@gmail.com> wrote: > > > > >I'd go with something like (untested and likely improperly formatted): > > >if (dwp_file->cus) > > > dwp_file->version = dwp_file-cus->version; > > > else if (dwp_file->tus) > > > dwp_file->version = dwp_file->tus->version > > > else > > > dwp_file->version = 2; > > > > i'm ok with that change, many thanks, > > do i need to resend the patch ? (and yeah, i can double check / test it > when > > i get home) > > No need, if you're confident all the nits are taken care of [ChangeLog > syntax and contents, patch whitespace, testing, and so on ... :-)] > > > regarding the dependence on the order of sections - (if i am not > mistaken) > > yeah, it's a separate issue - i can provide more details > > when i get to my laptop. > > Thanks. > > > > > > Best regards, > > Alexander Shaposhnikov > > > > On Mon, Sep 25, 2017 at 8:14 PM, Doug Evans <dje@google.com> wrote: > >> > >> On Tue, Sep 19, 2017 at 12:58 PM, Alexander Shaposhnikov > >> <alexander.v.shaposhnikov@gmail.com> wrote: > >> > This patch fixes segmentation fault (due to dereferencing of a null > >> > pointer) > >> > in dwarf2read.c in the code dwp_file->cus->version != > >> > dwp_file->tus->version > >> > by adding defensive checks similarly to how it's already done > >> > at the lines 11208 - 11215 (in the same file dwarf2read.c). > >> > The issue can be reproduced with dwp packages built by llvm-dwp > utility > >> > (from the current upstream) since at the moment llvm-dwp doesn't > create > >> > .debug_tu_index section, thus dwp_file->tus will be null. > >> > > >> > Test plan: > >> > > >> > BUILD: > >> > main.cpp: > >> > int f() { > >> > int x = 0; > >> > int y = 1; > >> > return x + y; > >> > } > >> > g++ -fPIC -gsplit-dwarf -g -O0 main.cpp -o main.exe > >> > llvm-dwp main.dwo -o main.exe.dwp > >> > # This step is a workaround to a separate issue (unrelated to this > >> > patch). > >> > # At the moment llvm tools & clang put .strtab section first (its > index > >> > is > >> > 1), > >> > # while gcc/gold/binutils put it at the end. > >> > # Unfortunately gdb (in the code reading dwps) appears to depend on > the > >> > order > >> > # of sections, > >> > >> That's odd. Depends on the order of sections how? > >> > >> > to workaround this (to reproduce the issue which this patch > >> > # aims to address) we use objcopy to do the following trick: > >> > # if one asks to remove .strtab objcopy will do that but at the same > >> > time > >> > # it will create a new .shstrtab section at the end. > >> > objcopy --remove-section .strtab main.exe.dwp > >> > RUN: > >> > gdb main.exe > >> > One can observe that now there is no crash and debugging functionality > >> > works as expected (setting breakpoints, printing local variable, > >> > exploring > >> > the stack). > >> > > >> > gdb/ChangeLog: > >> > yyyy-mm-dd Alexander Shaposhnikov <alexander.v.shaposhnikov@ > gmail.com> > >> > > >> > * dwarf2read.c: Fix segmentation fault on reading > >> > > >> > .dwp files without .debug_tu_index section. > >> > > >> > Patch: > >> > > >> > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > >> > > >> > index b1914cf876..547e3f034e 100644 > >> > --- a/gdb/dwarf2read.c > >> > +++ b/gdb/dwarf2read.c > >> > @@ -11185,7 +11185,8 @@ open_and_init_dwp_file (void) > >> > dwp_file->tus = create_dwp_hash_table (dwp_file, 1); > >> > > >> > /* The DWP file version is stored in the hash table. Oh well. */ > >> > - if (dwp_file->cus->version != dwp_file->tus->version) > >> > + if (dwp_file->cus && dwp_file->tus > >> > + && dwp_file->cus->version != dwp_file->tus->version) > >> > { > >> > /* Technically speaking, we should try to limp along, but this > is > >> > pretty bizarre. We use pulongest here because that's the > >> > established > >> > @@ -11195,7 +11196,7 @@ open_and_init_dwp_file (void) > >> > pulongest (dwp_file->cus->version), > >> > pulongest (dwp_file->tus->version), dwp_name.c_str ()); > >> > } > >> > - dwp_file->version = dwp_file->cus->version; > >> > + dwp_file->version = dwp_file->cus ? dwp_file->cus->version : 0; > >> > >> A version of 0 doesn't seem right. > >> I'd go with something like (untested and likely improperly formatted): > >> > >> if (dwp_file->cus) > >> dwp_file->version = dwp_file-cus->version; > >> else if (dwp_file->tus) > >> dwp_file->version = dwp_file->tus->version > >> else > >> dwp_file->version = 2; > >> > >> ok with that change. > >> > >> > > >> > if (dwp_file->version == 2) > >> > bfd_map_over_sections (dwp_file->dbfd, > >> > dwarf2_locate_v2_dwp_sections, > >> > > >> > > >> > Kind regards, > >> > Alexander Shaposhnikov > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix reading .dwp files without .debug_tu_index @ 2017-09-28 16:30 Doug Evans via gdb-patches 0 siblings, 0 replies; 7+ messages in thread From: Doug Evans via gdb-patches @ 2017-09-28 16:30 UTC (permalink / raw) To: Alexander Shaposhnikov; +Cc: gdb-patches Alexander Shaposhnikov writes: > (updated patch + changelog) > > changelog: > > 2017-09-26 Alexander Shaposhnikov <alexander.v.shaposhnikov@gmail.com> > > * dwarf2read.c (open_and_init_dwp_file): Protect against dwp_file > having NULL cus or tus. Thanks! Committed. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-28 16:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-19 19:58 [PATCH] Fix reading .dwp files without .debug_tu_index Alexander Shaposhnikov 2017-09-24 23:37 ` Alexander Shaposhnikov 2017-09-26 3:15 ` Doug Evans via gdb-patches 2017-09-26 4:13 ` Alexander Shaposhnikov 2017-09-26 17:51 ` Doug Evans via gdb-patches 2017-09-26 18:48 ` Alexander Shaposhnikov 2017-09-28 16:30 Doug Evans via gdb-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox