FreeChainXenon/binutils-gdb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

gdb: avoid nullptr access in dbxread.c from read_dbx_symtab

Browse source 
This fixes a GDB crash reported in bug pr/28900, related to reading in
some stabs debug information.

In this commit my goal is to stop GDB crashing.  I am not trying to
ensure that GDB makes the best possible use of the available stabs
debug information.  At this point I consider stabs a legacy debug
format, with only limited support in GDB.

So, the problem appears to be that, when reading in the stabs data, we
need to find a N_SO entry, this is the entry that defines the start of
a compilation unit (or at least the location of a corresponding source
file).

It is while handling an N_SO that GDB creates a psymtab to hold the
incoming debug information (symbols, etc).

The problem we hit in the bug is that we encounter some symbol
information (an N_PC entry) outside of an N_SO entry - that is we find
some symbol information that is not associated with any source file.

We already have some protection for this case, look (in
read_dbx_symtab) at the handling of N_PC entries of type 'F' and 'f',
if we have no psymtab (the pst variable is nullptr) then we issue a
complaint.  However, for whatever reason, in both 'f' and 'F'
handling, there is one place where we assume that the pst
variable (the psymtab) is not nullptr.  This is a mistake.

In this commit, I guard these two locations (in 'f' and 'F' handling)
so we no longer assume pst is not nullptr.

While I was at it, I audited all the other uses of pst in
read_dbx_symtab, and in every potentially dangerous case I added a
nullptr check, and issue a suitable complaint if pst is found to be
nullptr.

It might well be true that we could/should do something smarter if we
see a debug symbol outside of an N_SO entry, and if anyone wanted to
do that work, they're welcome too.  But this commit is just about
preventing the nullptr access, and the subsequent GDB crash.

I don't have any tests for this change, I have no idea how to generate
weird stabs data for testing.  The original binary from the bug report
now loads just fine without GDB crashing.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28900
This commit is contained in:
Andrew Burgess 2022-02-19 13:09:34 +00:00
parent 9c6c44713f
commit 336125713f
1 changed files with 88 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

137
gdb/dbxread.c
View file

@ -1461,23 +1461,33 @@ read_dbx_symtab (minimal_symbol_reader &reader,
switch (p[1])
{
case 'S':
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_STATIC,
data_sect_index,
psymbol_placement::STATIC,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_STATIC,
data_sect_index,
psymbol_placement::STATIC,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
else
complaint (_("static `%*s' appears to be defined "
"outside of all compilation units"),
sym_len, sym_name);
continue;
case 'G':
/* The addresses in these entries are reported to be
wrong. See the code that reads 'G's for symtabs. */
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_STATIC,
data_sect_index,
psymbol_placement::GLOBAL,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_STATIC,
data_sect_index,
psymbol_placement::GLOBAL,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
else
complaint (_("global `%*s' appears to be defined "
"outside of all compilation units"),
sym_len, sym_name);
continue;
case 'T':
@ -1491,19 +1501,30 @@ read_dbx_symtab (minimal_symbol_reader &reader,
|| (p == namestring + 1
&& namestring[0] != ' '))
{
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, STRUCT_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, STRUCT_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
else
complaint (_("enum, struct, or union `%*s' appears "
"to be defined outside of all "
"compilation units"),
sym_len, sym_name);
if (p[2] == 't')
{
/* Also a typedef with the same name. */
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, VAR_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, VAR_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
else
complaint (_("typedef `%*s' appears to be defined "
"outside of all compilation units"),
sym_len, sym_name);
p += 1;
}
}
@ -1512,11 +1533,16 @@ read_dbx_symtab (minimal_symbol_reader &reader,
case 't':
if (p != namestring) /* a name is there, not just :T... */
{
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, VAR_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len),
true, VAR_DOMAIN, LOC_TYPEDEF, -1,
psymbol_placement::STATIC,
0, psymtab_language,
partial_symtabs, objfile);
else
complaint (_("typename `%*s' appears to be defined "
"outside of all compilation units"),
sym_len, sym_name);
}
check_enum:
/* If this is an enumerated type, we need to
@ -1574,11 +1600,16 @@ read_dbx_symtab (minimal_symbol_reader &reader,
;
/* Note that the value doesn't matter for
enum constants in psymtabs, just in symtabs. */
pst->add_psymbol (gdb::string_view (p, q - p), true,
VAR_DOMAIN, LOC_CONST, -1,
psymbol_placement::STATIC, 0,
psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (p, q - p), true,
VAR_DOMAIN, LOC_CONST, -1,
psymbol_placement::STATIC, 0,
psymtab_language,
partial_symtabs, objfile);
else
complaint (_("enum constant `%*s' appears to be defined "
"outside of all compilation units"),
((int) (q - p)), p);
/* Point past the name. */
p = q;
/* Skip over the value. */
@ -1593,11 +1624,17 @@ read_dbx_symtab (minimal_symbol_reader &reader,
case 'c':
/* Constant, e.g. from "const" in Pascal. */
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_CONST, -1,
psymbol_placement::STATIC, 0,
psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_CONST, -1,
psymbol_placement::STATIC, 0,
psymtab_language,
partial_symtabs, objfile);
else
complaint (_("constant `%*s' appears to be defined "
"outside of all compilation units"),
sym_len, sym_name);
continue;
case 'f':
@ -1644,12 +1681,13 @@ read_dbx_symtab (minimal_symbol_reader &reader,
pst->set_text_low (nlist.n_value);
textlow_not_set = 0;
}
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_BLOCK,
SECT_OFF_TEXT (objfile),
psymbol_placement::STATIC,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_BLOCK,
SECT_OFF_TEXT (objfile),
psymbol_placement::STATIC,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
continue;
/* Global functions were ignored here, but now they
@ -1699,12 +1737,13 @@ read_dbx_symtab (minimal_symbol_reader &reader,
pst->set_text_low (nlist.n_value);
textlow_not_set = 0;
}
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_BLOCK,
SECT_OFF_TEXT (objfile),
psymbol_placement::GLOBAL,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
if (pst != nullptr)
pst->add_psymbol (gdb::string_view (sym_name, sym_len), true,
VAR_DOMAIN, LOC_BLOCK,
SECT_OFF_TEXT (objfile),
psymbol_placement::GLOBAL,
nlist.n_value, psymtab_language,
partial_symtabs, objfile);
continue;
/* Two things show up here (hopefully); static symbols of