From patchwork Fri Jul 7 19:11:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Aaron Merey X-Patchwork-Id: 72351 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EFC3238515F7 for ; Fri, 7 Jul 2023 19:12:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EFC3238515F7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1688757126; bh=wjuSW2GqvPxfRzVtC5PpINbDV/uofPCS7+xh9vOhZ7Q=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=kePtjfWGSA5yHU1/LlO1aZywx/wCJ25eGsDSh1ztGUeoun7KqB4E+tBy/+AqZUFnf P74FlxcY6WDSV4OOThgzBjcCKICPT05owpIx0gdD/F6peAne6s0MI2mRYUjPULE9xq Mnnm/sqRptKRr8PBszQiTOC1CayHeel9zM14+SDc= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 97AA53856948 for ; Fri, 7 Jul 2023 19:11:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 97AA53856948 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-529-Xqyy6Hf-MDOwXN6WdOtmoA-1; Fri, 07 Jul 2023 15:11:41 -0400 X-MC-Unique: Xqyy6Hf-MDOwXN6WdOtmoA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E5DE9185A7A4; Fri, 7 Jul 2023 19:11:40 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.22.16.101]) by smtp.corp.redhat.com (Postfix) with ESMTP id 653DAC09A09; Fri, 7 Jul 2023 19:11:39 +0000 (UTC) To: tom@tromey.com Cc: gdb-patches@sourceware.org, Aaron Merey Subject: [PATCH] gdb/cp-namespace.c: Fix assert failure caused by malformed user input Date: Fri, 7 Jul 2023 15:11:35 -0400 Message-ID: <20230707191136.3648913-1-amerey@redhat.com> In-Reply-To: <875y6xujz6.fsf@tromey.com> References: <875y6xujz6.fsf@tromey.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Aaron Merey via Gdb-patches From: Aaron Merey Reply-To: Aaron Merey Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Hi Tom, On Thu, Jul 6, 2023 at 11:40 AM Tom Tromey wrote: > > >>>>> "Aaron" == Aaron Merey via Gdb-patches writes: > Aaron> - if (strchr (name, '<') == NULL && strchr (name, '(') == NULL) > Aaron> - gdb_assert (strstr (name, "::") == NULL); > Aaron> + if (strchr (name, '<') == nullptr && strchr (name, '>') == nullptr > Aaron> + && strchr (name, '(') == nullptr) > Aaron> + gdb_assert (strstr (name, "::") == nullptr); > > I think it would be better to use strpbrk here, like: > > gdb_assert (strpbrk (name, "<(>") != nullptr || strstr (name, "::") == nullptr); > > Writing it like this, though, I wonder if a stray ")" could also make it > to this assert, the way that ">" did. The stray ')' does trigger the same assert. Below I've tweaked the patch to prevent the assert with stray ')'. Aaron Commit message: When debugging C++ programs, it is possible to trigger a spurious assert failure when attempting to set a breakpoint on a malformed symbol name. Names of the form 'A>::B' and 'A)::B' trigger this assert failure in cp_lookup_bare_symbol: $ gdb gdb [...] (gdb) br test>::assert Function "test>::assert" not defined. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (test>::assert) pending. (gdb) start [...] cp-namespace.c:181: internal-error: cp_lookup_bare_symbol: Assertion `strstr (name, "::") == NULL' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. ----- Backtrace ----- 0x5217e2 gdb_internal_backtrace_1 /home/amerey/binutils-gdb/gdb/bt-utils.c:122 0x521885 _Z22gdb_internal_backtracev /home/amerey/binutils-gdb/gdb/bt-utils.c:168 0xaf8303 internal_vproblem /home/amerey/binutils-gdb/gdb/utils.c:396 0xaf86be _Z15internal_verrorPKciS0_P13__va_list_tag /home/amerey/binutils-gdb/gdb/utils.c:476 0xccdb3f _Z18internal_error_locPKciS0_z /home/amerey/binutils-gdb/gdbsupport/errors.cc:58 0x5dded9 cp_lookup_bare_symbol /home/amerey/binutils-gdb/gdb/cp-namespace.c:181 0x5de39d cp_lookup_symbol_in_namespace /home/amerey/binutils-gdb/gdb/cp-namespace.c:328 [...] Currently this assert is skipped if the symbol name contains '<' or '('. Fix this spurious failure by also skipping the assert when the symbol name contains '>' or ')'. Regression tested on F38 x86_64. --- gdb/cp-namespace.c | 5 +++-- gdb/testsuite/gdb.cp/namespace.exp | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 14d807694b7..d4a47eaac1f 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -34,6 +34,7 @@ #include "namespace.h" #include #include +#include static struct block_symbol cp_lookup_nested_symbol_1 (struct type *container_type, @@ -177,8 +178,8 @@ cp_lookup_bare_symbol (const struct language_defn *langdef, /* Note: We can't do a simple assert for ':' not being in NAME because ':' may be in the args of a template spec. This isn't intended to be a complete test, just cheap and documentary. */ - if (strchr (name, '<') == NULL && strchr (name, '(') == NULL) - gdb_assert (strstr (name, "::") == NULL); + gdb_assert (strpbrk ("<>()", name) != nullptr + || strstr (name, "::") == nullptr); sym = lookup_symbol_in_static_block (name, block, domain); if (sym.symbol != NULL) diff --git a/gdb/testsuite/gdb.cp/namespace.exp b/gdb/testsuite/gdb.cp/namespace.exp index e364816fcb7..a9de09f087d 100644 --- a/gdb/testsuite/gdb.cp/namespace.exp +++ b/gdb/testsuite/gdb.cp/namespace.exp @@ -250,3 +250,9 @@ gdb_test "print AAA::ALPHA" "\\$\[0-9\].* = AAA::ALPHA" # Regression tests for PR 9496. gdb_test "whatis ::C::CClass::NestedClass" "type = C::CClass::NestedClass" gdb_test "whatis ::C::CClass::NestedClass *" "type = C::CClass::NestedClass \\*" + +# Break on functions with a malformed name. +gdb_test "break DNE>::DNE" "" "br malformed \'>\'" \ + "Make breakpoint pending on future shared library load?.*" "y" +gdb_test "break DNE)::DNE" "" "br malformed \')\'" \ + "Make breakpoint pending on future shared library load?.*" "y"