[gdb/symtab] Handle empty file name in .debug_line section
Commit Message
With DWARF 5, it's possible to produce an empty file name in the File Name
Table of the .debug_line section:
...
The File Name Table (offset 0x112, lines 1, columns 2):
Entry Dir Name
0 1 (indirect line string, offset: 0x2d):
...
Currently, when gdb reads an exec containing such debug info, it segfaults:
...
Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
0x000000000072cd38 in dwarf2_start_subfile (cu=0x2badc50, fe=..., lh=...) at \
gdb/dwarf2/read.c:18716
18716 if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
...
because read_direct_string transforms "" into a nullptr, and we end up
dereferencing the nullptr.
Note that the behaviour of read_direct_string has been present since repo
creation.
Fix this in read_formatted_entries, by transforming nullptr filenames in to ""
filenames.
Tested on x86_64-linux.
PR symtab/30357
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30357
---
gdb/dwarf2/line-header.c | 4 ++
.../gdb.dwarf2/dw2-empty-file-name.exp | 66 +++++++++++++++++++
2 files changed, 70 insertions(+)
create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-empty-file-name.exp
base-commit: 55003828f4d81a3d857ccfdfe4841704004a6468
Comments
>>>>> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> because read_direct_string transforms "" into a nullptr, and we end up
> dereferencing the nullptr.
> Note that the behaviour of read_direct_string has been present since repo
> creation.
> Fix this in read_formatted_entries, by transforming nullptr filenames in to ""
> filenames.
Thanks, this looks good to me.
I'm curious if an entry of "" even makes sense, but I think the answer
doesn't affect the validity of this patch.
Reviewed-By: Tom Tromey <tom@tromey.com>
Tom
On 4/17/23 16:56, Tom Tromey wrote:
>>>>>> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> because read_direct_string transforms "" into a nullptr, and we end up
>> dereferencing the nullptr.
>
>> Note that the behaviour of read_direct_string has been present since repo
>> creation.
>
>> Fix this in read_formatted_entries, by transforming nullptr filenames in to ""
>> filenames.
>
> Thanks, this looks good to me.
>
Thanks for the review.
> I'm curious if an entry of "" even makes sense, but I think the answer
> doesn't affect the validity of this patch.
>
Ack, I don't know either, but agreed, so committed.
Thanks,
- Tom
> Reviewed-By: Tom Tromey <tom@tromey.com>
>
> Tom
On 4/17/23 18:14, Tom de Vries via Gdb-patches wrote:
> On 4/17/23 16:56, Tom Tromey wrote:
>>>>>>> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> because read_direct_string transforms "" into a nullptr, and we end up
>>> dereferencing the nullptr.
>>
>>> Note that the behaviour of read_direct_string has been present since
>>> repo
>>> creation.
>>
>>> Fix this in read_formatted_entries, by transforming nullptr filenames
>>> in to ""
>>> filenames.
>>
>> Thanks, this looks good to me.
>>
>
> Thanks for the review.
>
>> I'm curious if an entry of "" even makes sense, but I think the answer
>> doesn't affect the validity of this patch.
>>
>
> Ack, I don't know either, but agreed, so committed.
>
Btw, as I've just confirmed, this is a regression from gdb 12 to gdb 13,
so I wonder if we should backport this to the gdb-13-branch.
The only remaining requirement seems to be "A Global Maintainer must
approve the commit for that branch" but it seems a bit forward to
self-approve so, anybody second this?
Thanks,
- Tom
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> The only remaining requirement seems to be "A Global Maintainer must
Tom> approve the commit for that branch" but it seems a bit forward to
Tom> self-approve so, anybody second this?
Yes. It is safe & correct.
thanks,
Tom
@@ -215,6 +215,10 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
break;
}
+ /* Normalize nullptr string. */
+ if (string.has_value () && *string == nullptr)
+ string.emplace ("");
+
switch (content_type)
{
case DW_LNCT_path:
new file mode 100644
@@ -0,0 +1,66 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Check that an empty file name in the .debug_line section doesn't cause
+# problems.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+# The .c files use __attribute__.
+require is_c_compiler_gcc
+
+standard_testfile main.c -dw.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ declare_labels Llines
+ global srcdir subdir srcfile
+
+ cu { version 5 } {
+ compile_unit {
+ {language @DW_LANG_C}
+ {name $srcfile}
+ {stmt_list $Llines DW_FORM_sec_offset}
+ } {
+ subprogram {
+ {external 1 flag}
+ {MACRO_AT_func {main}}
+ }
+ }
+ }
+
+ lines {version 5} Llines {
+ include_dir "${srcdir}/${subdir}"
+ file_name "" 1
+ }
+}
+
+if { [build_executable "failed to prepare" ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+}
+
+clean_restart
+
+# If there's an ERROR during gdb_load, the assert will produce an UNRESOLVED
+# rather than a FAIL.
+gdb_assert { [gdb_load $binfile] == 0 }
+
+# But this will produce an actual FAIL, which is more visible than
+# ERROR/UNRESOLVED.
+gdb_assert { [info exists gdb_spawn_id] }