From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7D0E23858412 for ; Sat, 19 Feb 2022 19:51:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7D0E23858412 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-156-_yfGzxGdOLKnHoHUvEd6xg-1; Sat, 19 Feb 2022 14:51:50 -0500 X-MC-Unique: _yfGzxGdOLKnHoHUvEd6xg-1 Received: by mail-wr1-f69.google.com with SMTP id j8-20020adfa548000000b001e33074ac51so5010707wrb.11 for ; Sat, 19 Feb 2022 11:51:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Ws8UgWVQWIRogxifpF2o06yGV2UCzbWMbPk6JAxLnzo=; b=qbyzijEP2898PPRr9FxMLtXUT6jfmMrOQNXbV7iuqxAxPUDYFzse47LQylSqOwhH42 CoRydx3TRq3GUUtq3szPtgTaOuGUmS8Z7cX7Hr8HqvdghJ1IFXScEzxVByvAfXboGTaa /F/OoW5tI/3tLDUJRzAyTgWVozWfm9rC3o9vDn/gbkybWXfLx9M/U/jIaUtOEQVCNRbv ixaGFIgTYU+w6dIT/xYvayL7ysXsJibSaICLvQNC8EqHC7blZjExd5Qrb9mhp5599yeI 6KHQRsOMNFunaP7BKUyv047Z0KHvxlqzeoQjYvmVQwChMO9qzfIl5Gr8KdenZ56n/P6A LwGg== X-Gm-Message-State: AOAM5319QrfosglHX6WSOZLOwWElHQCKBwaCjAHQxUhX+Eo/BStMQxJO IPXlNbT+R9FoeKiEVh6CYT/w/DYQOPvRblMpwOkDVAM1YE9302jPlUDTdimEjeaIWUxAz4QsJaz EumMV3xA9RZcbwMMIzt4a87DWo+yPJiDuKRIfetjn6s16phrrmv4Z+9y3/UeyamY92qlmLsvgRw == X-Received: by 2002:adf:82cb:0:b0:1e3:f20:fd0d with SMTP id 69-20020adf82cb000000b001e30f20fd0dmr9983068wrc.458.1645300308882; Sat, 19 Feb 2022 11:51:48 -0800 (PST) X-Google-Smtp-Source: ABdhPJxYDzsuK5QHYI+/Wvoain2sI6UDdkDGalwR0DCWvu+sWMav6a4Nuzl3kjWVUvcBVP/cBBpGmA== X-Received: by 2002:adf:82cb:0:b0:1e3:f20:fd0d with SMTP id 69-20020adf82cb000000b001e30f20fd0dmr9983047wrc.458.1645300308452; Sat, 19 Feb 2022 11:51:48 -0800 (PST) Received: from localhost (host86-134-151-224.range86-134.btcentralplus.com. [86.134.151.224]) by smtp.gmail.com with ESMTPSA id ba27sm17760579wrb.61.2022.02.19.11.51.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 19 Feb 2022 11:51:47 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/2] gdb: avoid nullptr access in dbxread.c from read_dbx_symtab Date: Sat, 19 Feb 2022 19:51:41 +0000 Message-Id: <8febea6adac3d80d41f41a2e5e49dc98d0b934ba.1645300222.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Feb 2022 19:51:55 -0000 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 --- gdb/dbxread.c | 137 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 49 deletions(-) diff --git a/gdb/dbxread.c b/gdb/dbxread.c index ddda5df4ee1..165040dd7ce 100644 --- 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 -- 2.25.4