gdb: avoid nullptr access in dbxread.c from read_dbx_symtab

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

diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index ddda5df4ee161b2de28f05a33ee723e0d07737bd..165040dd7ce4fd7ecc29204f03b0c5ff72daf382 100644 (file)
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -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