From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id T6pFODuJbGJhZAMAWB0awg (envelope-from ) for ; Fri, 29 Apr 2022 20:56:27 -0400 Received: by simark.ca (Postfix, from userid 112) id DACA21E058; Fri, 29 Apr 2022 20:56:27 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 8A29F1E01D for ; Fri, 29 Apr 2022 20:56:26 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A60F03857340 for ; Sat, 30 Apr 2022 00:56:25 +0000 (GMT) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by sourceware.org (Postfix) with ESMTPS id A8A403858405 for ; Sat, 30 Apr 2022 00:56:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A8A403858405 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f43.google.com with SMTP id r11-20020a05600c35cb00b0039409c1111bso4657467wmq.3 for ; Fri, 29 Apr 2022 17:56:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=2f1MHbvny7/nnDfB6VVtRPrKAz6hsWjsZxVbEX0yz9M=; b=YIB372rQBY/IWbdD22SBZ5q9xQlCVsdH0w5j1dDBvm4frZPExZY/VG5aXPIZ4lIBFJ LQMH3gSs2dAXPBnvH3/Tgj1BmJMvOWhnoPpSoZwpXQNVAvvQzd21cimSNrkfXqq1OtHK UO3TWNkgaqvKJ12Lb1t7N0Tz0MFt4ZXcukA52SseIkIsn03IvC3Xj+SUqQoPC4m+x4/A 7v06cE353aKb27IMIVQNWMCPqiMaTOueSIGq2t270Gy9r92AJR1J91WWaPf7JxjbXpEz chqcKyThL2zABDGSoFYbf5QjUOnyQ9zwedoAbHHuETp31l/xLKKeqMp2aNw5AOwaGIr2 z+Qg== X-Gm-Message-State: AOAM531S89IYz86O6xmW86/We+cS6lprvKihTEsrz4QVshpkV0YEFphe ospwun83ibmZVfPngVRU8LQ= X-Google-Smtp-Source: ABdhPJxQflzn7psvXoeC4pMDYqIq3w2ixuBWQXGPSEWe8Fww2zsVM61VQKOEGCen9NBFKx/MtUH/ng== X-Received: by 2002:a1c:2185:0:b0:38f:f4ed:f964 with SMTP id h127-20020a1c2185000000b0038ff4edf964mr1312642wmh.115.1651280170561; Fri, 29 Apr 2022 17:56:10 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id h26-20020a7bc93a000000b003942a244f4fsm549192wml.40.2022.04.29.17.56.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 Apr 2022 17:56:09 -0700 (PDT) Message-ID: <06aaa8fe-6800-492b-9c01-e2fd360304fb@palves.net> Date: Sat, 30 Apr 2022 01:56:08 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE (was: Re: [PATCH] Fix gdb.cp/no-dmgl-verbose.exp test) Content-Language: en-US To: Carl Love , Keith Seitz , Lancelot SIX References: <5ee342cd5f5272da9970da8a077c2c5209b85d6c.camel@us.ibm.com> <20220429091234.62xhprge74gfpgks@ubuntu.lan> <4610920e52ea1bbeb5181c970887eb7cfe344f1a.camel@us.ibm.com> <032437ea-2ef4-90f5-7b96-8a729bae2252@redhat.com> <31261461-8f2a-8919-c882-3601a9adefd9@palves.net> <117e54956e10755d19aeff2936600dfb89f3b1cf.camel@us.ibm.com> From: Pedro Alves In-Reply-To: <117e54956e10755d19aeff2936600dfb89f3b1cf.camel@us.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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: , Cc: Ulrich Weigand , gdb-patches@sourceware.org, Rogerio Alves Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2022-04-29 20:13, Carl Love wrote: > GDB maintaners, Pedro, Lancelot: > > I am not much of a C++ programer. I will have to admit that there is a > fair bit of the deep C++ behaviour with old/new APIs in Pedro's post > that is beyond me. :-) Sorry. Additionally, there is concern that > the name of the test is not accurate, etc. > > So at this point, not sure where to go with fixing this test. I seem > to have opened a real can of worms. Suggestions on how to move > forward, remove the test, go with a minimal change to get the test to > pass, do a rewrite/name change, .... ? > I knew I wouldn't be able to sleep if I didn't give this a try, so here it goes... I think we should fix it properly such that "b f(std::string)" also works with the modern C++11 ABI. Given with the modern ABI, we don't even end up with standard substitutions in the mangled name, avoiding DMGL_VERBOSE seems pointless, as everyone who doesn't force old ABI in their compilations is getting expanded std::string typedefs anyhow. I think we should instead enforce usage of DMGL_VERBOSE throughout GDB, and remove the special treatment for std::string and others from cp-support.c. This way, old ABI behaves as new ABI. If we do this, then gdb.cp/no-dmgl-verbose.exp loses it's main purpose (making sure DMGL_VERBOSE doesn't creep back in), and it should be renamed/adjusted to test that setting a breakpoint at f(std::string) works, _and_ that setting a breakpoint at the expanded non-typedefed version of that function works too, similar to what Lancelot had originally suggested. (I think we should be able to use whatis instead of info types, though.) I'll have to leave writing a proper commit log for later. Meanwhile, here's a patch for comments. No regressions on x86-64 GNU/Linux, and setting a breakpoint at "f(std::string)" now works with either old or new ABI. >From d298d3c78f2d19bb70b312674b5ef9e5dbce2fde Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 29 Apr 2022 23:21:18 +0100 Subject: [PATCH] Fix "b func(std::string)", use DMGL_VERBOSE Change-Id: Ib54fab4cf0fd307bfd55bf1dd5056830096a653b --- gdb/cp-name-parser.y | 5 +- gdb/cp-support.c | 52 ++++++--- gdb/cp-support.h | 10 ++ ...no-dmgl-verbose.cc => break-std-string.cc} | 0 gdb/testsuite/gdb.cp/break-std-string.exp | 108 ++++++++++++++++++ gdb/testsuite/gdb.cp/no-dmgl-verbose.exp | 35 ------ 6 files changed, 156 insertions(+), 54 deletions(-) rename gdb/testsuite/gdb.cp/{no-dmgl-verbose.cc => break-std-string.cc} (100%) create mode 100644 gdb/testsuite/gdb.cp/break-std-string.exp delete mode 100644 gdb/testsuite/gdb.cp/no-dmgl-verbose.exp diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y index 6410363c87f..ceb4c968935 100644 --- a/gdb/cp-name-parser.y +++ b/gdb/cp-name-parser.y @@ -1959,8 +1959,7 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len) { size_t err; - char *res = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI, - result, estimated_len, &err); + char *res = gdb_cplus_demangle_print (result, estimated_len, &err); return gdb::unique_xmalloc_ptr (res); } @@ -2060,7 +2059,7 @@ cp_print (struct demangle_component *result) char *str; size_t err = 0; - str = cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI, result, 64, &err); + str = gdb_cplus_demangle_print (result, 64, &err); if (str == NULL) return; diff --git a/gdb/cp-support.c b/gdb/cp-support.c index f52055893d2..b3eb775f717 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -70,18 +70,16 @@ static void add_symbol_overload_list_qualified struct cmd_list_element *maint_cplus_cmd_list = NULL; -/* A list of typedefs which should not be substituted by replace_typedefs. */ -static const char * const ignore_typedefs[] = - { - "std::istream", "std::iostream", "std::ostream", "std::string" - }; - static void replace_typedefs (struct demangle_parse_info *info, struct demangle_component *ret_comp, canonicalization_ftype *finder, void *data); +static struct demangle_component * + gdb_cplus_demangle_v3_components (const char *mangled, + int options, void **mem); + /* A convenience function to copy STRING into OBSTACK, returning a pointer to the newly allocated string and saving the number of bytes saved in LEN. @@ -146,13 +144,6 @@ inspect_type (struct demangle_parse_info *info, memcpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len); name[ret_comp->u.s_name.len] = '\0'; - /* Ignore any typedefs that should not be substituted. */ - for (const char *ignorable : ignore_typedefs) - { - if (strcmp (name, ignorable) == 0) - return 0; - } - sym = NULL; try @@ -238,6 +229,14 @@ inspect_type (struct demangle_parse_info *info, string_file buf; try { + /* If the current language is C, and type is a + struct/class, type_print prefixes the type name with + "struct" or "class", which we don't want when we're + expanding a C++ typedef. Force language temporarily + to avoid it. */ + scoped_restore_current_language restore_language; + set_language (language_cplus); + type_print (type, "", &buf, -1); } /* If type_print threw an exception, there is little point @@ -670,8 +669,8 @@ mangled_name_to_comp (const char *mangled_name, int options, { struct demangle_component *ret; - ret = cplus_demangle_v3_components (mangled_name, - options, memory); + ret = gdb_cplus_demangle_v3_components (mangled_name, + options, memory); if (ret) { std::unique_ptr info (new demangle_parse_info); @@ -1635,7 +1634,7 @@ gdb_demangle (const char *name, int options) #endif if (crash_signal == 0) - result.reset (bfd_demangle (NULL, name, options)); + result.reset (bfd_demangle (NULL, name, options | DMGL_VERBOSE)); #ifdef HAVE_WORKING_FORK if (catch_demangler_crashes) @@ -1670,6 +1669,27 @@ gdb_demangle (const char *name, int options) /* See cp-support.h. */ +char * +gdb_cplus_demangle_print (struct demangle_component *tree, + int estimated_length, + size_t *p_allocated_size) +{ + return cplus_demangle_print (DMGL_PARAMS | DMGL_ANSI | DMGL_VERBOSE, + tree, estimated_length, p_allocated_size); +} + +/* A wrapper for cplus_demangle_v3_components that forces + DMGL_VERBOSE. */ + +static struct demangle_component * +gdb_cplus_demangle_v3_components (const char *mangled, + int options, void **mem) +{ + return cplus_demangle_v3_components (mangled, options | DMGL_VERBOSE, mem); +} + +/* See cp-support.h. */ + unsigned int cp_search_name_hash (const char *search_name) { diff --git a/gdb/cp-support.h b/gdb/cp-support.h index 4fbd53c8923..0a30e59d36d 100644 --- a/gdb/cp-support.h +++ b/gdb/cp-support.h @@ -186,10 +186,20 @@ extern void cp_merge_demangle_parse_infos (struct demangle_parse_info *, extern struct cmd_list_element *maint_cplus_cmd_list; +/* Wrappers for bfd and libiberty demangling entry points. Note they + all force DMGL_VERBOSE so that callers don't need to. This is so + that GDB consistently uses DMGL_VERBOSE throughout. */ + /* A wrapper for bfd_demangle. */ gdb::unique_xmalloc_ptr gdb_demangle (const char *name, int options); +/* A wrapper for cplus_demangle_print. Uses DMGL_PARAMS and + DMGL_ANSI, in addition to DMGL_VERBOSE. */ +extern char *gdb_cplus_demangle_print (struct demangle_component *tree, + int estimated_length, + size_t *p_allocated_size); + /* Find an instance of the character C in the string S that is outside of all parenthesis pairs, single-quoted strings, and double-quoted strings. Also, ignore the char within a template name, like a ',' diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.cc b/gdb/testsuite/gdb.cp/break-std-string.cc similarity index 100% rename from gdb/testsuite/gdb.cp/no-dmgl-verbose.cc rename to gdb/testsuite/gdb.cp/break-std-string.cc diff --git a/gdb/testsuite/gdb.cp/break-std-string.exp b/gdb/testsuite/gdb.cp/break-std-string.exp new file mode 100644 index 00000000000..edfe4fb92ae --- /dev/null +++ b/gdb/testsuite/gdb.cp/break-std-string.exp @@ -0,0 +1,108 @@ +# Copyright 2022 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 . */ + +# Test setting a breakpoint at "f(std::string)". +# +# GDB should be able to expand the std::string typedef, and then set +# the breakpoint using the full-qualified and expanded parameter name. +# std::string, std::istream, std::iostream, std::ostream are special +# for the demangler, though, they have corresponding "standard +# substitutions" elements. The libiberty demangler only expands the +# standard substitutions to their full non-typedef underlying type if +# the DMGL_VERBOSE option is requested. GDB didn't use to use that +# option, and would instead prevent expansion of the std::string +# typedefs at breakpoint-set type, such that the function name used +# for function lookup would match with the "std::string" present in +# the function's non-DMGL_VERBOSE demangled name. +# +# For example (DMGL_VERBOSE): +# +# $ echo "_Z1fSs" | c++filt +# f(std::basic_string, std::allocator >) +# +# vs (no DMGL_VERBOSE): +# +# $ echo "_Z1fSs" | c++filt --no-verbose +# f(std::string) +# +# This design however broke setting a breakpoint at std:string when +# the new libstdc++ C++11 ABI was introduced, as the "f(std::string)" +# function's mangled name no longer uses a standard substitution for +# std::string... +# +# I.e., with the libstdc++ C++11 ABI, we now have (and DMGL_VERBOSE +# makes no difference): +# +# $ echo _Z1fNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE | c++filt +# f(std::__cxx11::basic_string, std::allocator >) +# +# So nowadays, GDB always uses DMGL_VERBOSE and no longer prevents +# std::string (etc.) typedef expansion. This test exercises both old +# and new C++11 ABI for this reason. + +standard_testfile .cc + +if { [skip_cplus_tests] } { continue } + +# CXX11_ABI specifies the value to define _GLIBCXX_USE_CXX11_ABI as. + +proc test {cxx11_abi} { + global srcdir subdir srcfile binfile testfile + + set options \ + [list c++ debug additional_flags=-D_GLIBCXX_USE_CXX11_ABI=$cxx11_abi] + if { [gdb_compile \ + "${srcdir}/${subdir}/${srcfile}" "${binfile}-${cxx11_abi}.o" \ + object $options] != "" } { + untested "failed to compile" + return -1 + } + + clean_restart ${testfile}-${cxx11_abi}.o + + gdb_test_no_output "set breakpoint pending off" + + # So that "whatis" expands the C++ typedef. Since we're debugging + # an .o file, GDB doesn't figure out were debugging C++ code and + # the current language when auto, is guessed as C. + gdb_test_no_output "set language c++" + + # Get the type std::string is a typedef for. + set realtype "" + set type "std::string" + gdb_test_multiple "whatis /r $type" "" { + -re -wrap "type = (\[^\r\n\]+)" { + set realtype $expect_out(1,string) + gdb_assert \ + {![string eq "$realtype" "$type"] && ![string eq "$realtype" ""]} \ + $gdb_test_name + } + } + + # Setting the breakpoint should still work even if GDB guesses the + # current language is C. Make sure to exercise C and C++ even if + # GDB someday changes to autodetect the current language as C++. + foreach_with_prefix lang {"c" "c++" "auto"} { + gdb_breakpoint "f($type)" message + + if { $realtype != "" } { + gdb_breakpoint "f($realtype)" message + } + } +} + +foreach_with_prefix cxx11_abi {0 1} { + test $cxx11_abi +} diff --git a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp b/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp deleted file mode 100644 index 14f11ddcf04..00000000000 --- a/gdb/testsuite/gdb.cp/no-dmgl-verbose.exp +++ /dev/null @@ -1,35 +0,0 @@ -# Copyright 2011-2022 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 . */ - -# Test loading symbols from unrelocated C++ object files. - -standard_testfile .cc - -if { [skip_cplus_tests] } { continue } - -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {c++ debug}] != "" } { - untested "failed to compile" - return -1 -} - -clean_restart ${testfile}.o - -gdb_test_no_output "set breakpoint pending off" - -gdb_breakpoint {'f(std::string)'} - -gdb_test {break 'f(std::basic_string, std::allocator >)'} \ - {Function ".*" not defined\.} \ - "DMGL_VERBOSE-demangled f(std::string) is not defined" base-commit: 2e920d702b43c6d21ebd1e8a49c9e976a0d2cde6 -- 2.36.0