From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Gt3kOeT8b2JN1gMAWB0awg (envelope-from ) for ; Mon, 02 May 2022 11:46:44 -0400 Received: by simark.ca (Postfix, from userid 112) id DE7D01E058; Mon, 2 May 2022 11:46:44 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, NICE_REPLY_A,RDNS_DYNAMIC 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 07F741E00D for ; Mon, 2 May 2022 11:46:44 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5C78E3857C48 for ; Mon, 2 May 2022 15:46:43 +0000 (GMT) Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by sourceware.org (Postfix) with ESMTPS id 186523858D3C for ; Mon, 2 May 2022 15:46:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 186523858D3C 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-wr1-f51.google.com with SMTP id t6so20098510wra.4 for ; Mon, 02 May 2022 08:46:31 -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=mIPWV8iABXk7xs+n+5UtmoFRD+xc6LWE6u+ZZ50Le0U=; b=W7b8aUBL/ia92xo3iMPoAkSpG/TFIW6y8l9tQFenQNvMERAYhBQRs2Zq39FATLr2/K kcfznS240RM8JPESrW8X9c5iZ0BcKWPOrrawxm1aDv5rlPVi7pRD6ePLYDn1IrKp7MPE cJWm1u1h04eN+9cCd8D7kjy/gcEDCYdhrxu/UjWtRObXhfpNguHQk3uY89e9gZVjDlMx bSQGqoxR7wAu0RCCE4ivSNvrSZ5ip8nLGxxmN0PxtU5jHtA88UT9QnIQxoYhqpmAVqlT K6g0RCdC7VLM2EsIlK2y4owytq8l4Nm3tOJ6rcyAKT8yKMDMfkP+zKBrgVS9WvbFp5Ow 5ELg== X-Gm-Message-State: AOAM532YZJ2A473twwAfylZGkVdMYOHSEXZtyxYPW2y0FiXySwxa0PaY MIWwVKOu2UkBQnueBSiER04= X-Google-Smtp-Source: ABdhPJxmmXXLuAZSZ1gqjUYl13NPJ/f71v88JwvYkVzzrl6+F6LeNuVpDkGqEX5TWSk+4gBDPLv8JA== X-Received: by 2002:adf:f4c7:0:b0:20a:de87:3003 with SMTP id h7-20020adff4c7000000b0020ade873003mr9629979wrp.632.1651506389854; Mon, 02 May 2022 08:46:29 -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 q22-20020a1cf316000000b003942a244f4csm6590369wmq.37.2022.05.02.08.46.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 May 2022 08:46:28 -0700 (PDT) Message-ID: Date: Mon, 2 May 2022 16:46:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [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: 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> <06aaa8fe-6800-492b-9c01-e2fd360304fb@palves.net> <20220430211043.cr2iqp6xgoeyws7z@octopus> From: Pedro Alves In-Reply-To: <20220430211043.cr2iqp6xgoeyws7z@octopus> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 , Rogerio Alves , gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2022-04-30 22:11, Lancelot SIX wrote: >> @@ -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 > > Not sure about comment conventions, but shouldn't type be upper case > here? Yes, fixed. > >> + 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); Also, here we can use the symbol's language instead of hardcoding C++. >> + >> 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) ... >> +# 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. > > s/full-qualified/fully-qualified/ maybe? Right, fixed. > >> +# 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 > ^ > One ':' is missing. Fixed. And this should say "f(std::string)" instead of std::string. Fix that too. > >> +# 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 > > Extra space at the end of the line. Fixed. > >> +# 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. > > I guess that this option is libstdc++ only, right? When using another > standard library (libc++ for example), does this option have an impact? > > It is probably not really an issue, though. If this has no effect, we > just end up running the test twice with whatever ABI is used. We still > want the tested behavior to be the same, it is just that we have test > names which do not really reflect what is tested. Right. I've now mentioned this in a comment, and changed the prefix used in the test names, to be the full name of the define to make this a bit more obvious: ... PASS: gdb.cp/break-std-string.exp: _GLIBCXX_USE_CXX11_ABI=0: lang=c: gdb_breakpoint: set breakpoint at f(std::string) ... PASS: gdb.cp/break-std-string.exp: _GLIBCXX_USE_CXX11_ABI=1: lang=c: gdb_breakpoint: set breakpoint at f(std::string) ... > > Or maybe we assume libstdc++ in gdb.cp, I do not know. We don't. > >> + >> +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" > > Is this forced to OFF just in case `gdb_breakpoint` change in the future > to have the "allow-pending" behavior by default? In the current state > of gdb_breakpoint, this is not necessary. I had just blindly copied this from gdb.cp/no-dmgl-verbose.exp. I removed it, as indeed it's not necessary. Actually, I stopped using gdb_breakpoint too. There's no real advantage to using it here, and gdb_breakpoint is a bit too lax with expected regexps to my liking when we're actually testing a break command rather than setting a breakpoint to run to it to set up for some other test. > >> + >> + # So that "whatis" expands the C++ typedef. Since we're debugging >> + # an .o file, GDB doesn't figure out were debugging C++ code and > > s/were/we’re/ > Thanks, fixed. I'll send a v2 with these fixes once I manage to write an actual commit log.