From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by sourceware.org (Postfix) with ESMTPS id 9A7F93858C51 for ; Thu, 9 Jun 2022 11:36:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9A7F93858C51 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-f44.google.com with SMTP id l126-20020a1c2584000000b0039c1a10507fso6615352wml.1 for ; Thu, 09 Jun 2022 04:36:44 -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:from:to:cc:references:in-reply-to :content-transfer-encoding; bh=2ShCrhmWNR550Vl0t6KsMhS8GlPI1nnCxZ6Cxf/FTrk=; b=jMbdtQvLotQvbvveZYbcgCLT16jdWyL9DxIVEo4qNmsL3a2tPdQIKxQz0EXtYyJUna 1Mw297t1SuCyj4EUhPLnhXx3Hf7dqzanLN6BcB1G6jMx1KckswMCZrjxuWusGp6nLJYw 4olHtb9isaVKs/uPO9kUVBmfQxZMpYW/nkrGd6J29hEaD3wSoUER5uttvVEawdMVyzp6 wRzEgDT2wbRobeHt1E8I0YGorn6ufMK9J0wB7qRydFY+qlrIW2ZbFx7AC7sZeEoxJKWU UZj6iAWlKvFnopOQ+CXdL/VcmfA9g2BHqP3/idvuscCXEGHikp09jAzisnQreLDREpJG 74VQ== X-Gm-Message-State: AOAM532h0RLHk5GoQRnk2KPG2oUphWrUJAOOxOSD0qLUwWIdDn42Blf/ XnRmFVwSndwVUr5tCKhEJTwxxFkSCfZD4A== X-Google-Smtp-Source: ABdhPJyxgtsOscBaM6Sv+cTMi419pl4EcAExcuZm+txOwMWN7rdibl0d65nhRizXFUuv9OHzO4bPmg== X-Received: by 2002:a7b:c755:0:b0:39c:3beb:32aa with SMTP id w21-20020a7bc755000000b0039c3beb32aamr2997107wmk.30.1654774603338; Thu, 09 Jun 2022 04:36:43 -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 z14-20020adfd0ce000000b00215bd1680a8sm17003582wrh.79.2022.06.09.04.36.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Jun 2022 04:36:42 -0700 (PDT) Message-ID: <26e8bc61-4833-e4f2-5938-d2ba52a8e3cb@palves.net> Date: Thu, 9 Jun 2022 12:36:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] Debug support for global alias variable Content-Language: en-US From: Pedro Alves To: "Natarajan, Kavitha" , Tom Tromey , "Natarajan, Kavitha via Gdb-patches" Cc: "George, Jini Susan" , "Parasuraman, Hariharan" References: <66ca57f9-c3b8-4aeb-f07c-c3c254dfdea6@redhat.com> <87ilra1gcx.fsf@tromey.com> <87sfqawi0v.fsf@tromey.com> <871qxsisr7.fsf@tromey.com> <498102ba-9a88-5ca7-ebe7-7120b655ee9c@palves.net> <41b119d8-0321-23de-212b-33a328720455@palves.net> In-Reply-To: <41b119d8-0321-23de-212b-33a328720455@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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-List-Received-Date: Thu, 09 Jun 2022 11:36:47 -0000 On 2022-06-08 17:29, Pedro Alves wrote: > Hi! > > I'm building current llvm to give this a try, and I'll merge it after some testing. > > Thanks again for the patch, and sorry for the delays. > I've now given it some testing, and I have some questions. >> /* Return the using directives repository (global or local?) to use in the >> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp >> index 289f49bbc3f..564b43086bc 100644 >> --- a/gdb/testsuite/gdb.base/symbol-alias.exp >> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp >> @@ -15,6 +15,9 @@ >> # along with this program. If not, see . >> >> standard_testfile symbol-alias.c symbol-alias2.c >> +set using_clang [test_compiler_info clang-*] >> +set old_clang [expr [test_compiler_info {clang-1[0-3]-*-*}] \ >> + || [test_compiler_info {clang-[1-9]-*}]] These could use a short comment (see bottom of this email). Also, this assumes the fix will be in Clang 14, but Clang 14 has been released recently, and your fix will only be in Clang 15, right? I caught this because in addition to testing against current main LLVM, I tested against a ROCm bundled Clang I had handy, which happens to be based on a LLVM that identifies as 14, and the tests FAILed, instead of XFAILing. I think that above we want "clang-1[0-4]" instead of "clang-1[0-3]", right? >> >> if { [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $srcfile2]] } { >> return -1 >> @@ -31,6 +34,62 @@ foreach f {"func" "func_alias"} { >> } >> >> # Variables. >> -foreach v {"g_var_s" "g_var_s_alias"} { >> - gdb_test "p $v" "= {field1 = 1, field2 = 2}" >> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}" >> +foreach v {"g_var_s_alias" "g_var_s_alias2"} { >> + gdb_test_multiple "p $v" "print alias variable $v" { >> + -re -wrap " = {field1 = 1, field2 = 2}" { >> + pass $gdb_test_name >> + } >> + -re -wrap "has unknown type; cast it to its declared type" { >> + if { $old_clang } { >> + xfail $gdb_test_name >> + } else { >> + fail $gdb_test_name >> + } >> + } >> + } >> +} >> + >> +# Static Variable. >> +gdb_test "p g_var" " = 1" >> +gdb_test_multiple "p g_var_alias" "print alias of static variable" { >> + -re -wrap " = 1" { >> + pass $gdb_test_name >> + } >> + -re -wrap "has unknown type; cast it to its declared type" { >> + if { $old_clang } { >> + xfail $gdb_test_name >> + } else { >> + fail $gdb_test_name >> + } >> + } >> +} >> + >> +# Deferred Variable. I had never heard the term "deferred variable" before. IIUC, it refers to the case of the alias aliasing a name that has not been seen in scope yet. Is this term "deferred variable" a term you invented, or did you find it elsewhere? I'm just curious, and pondering adding a comment. >> +gdb_test "p g_def_var" " = 2" >> +gdb_test_multiple "p g_def_var_alias" "print alias of deferred variable" { >> + -re -wrap " = 2" { >> + pass $gdb_test_name >> + } >> + -re -wrap "has unknown type; cast it to its declared type" { >> + if { $using_clang } { So all other tests pass with your fixed Clang, but this one will still xfail. Do you have more information about this? Strange offhand that "alias of alias" works, but this one doesn't. Is there some Clang bug open about this we could add a reference to here? If not, could one be created? If instead you think this is a GDB bug, then this should be a kfail ("known gdb fail") instead of an xfail, in which case we should have a bug open in gdb's bugzilla about it so we can reference it here. >> + xfail $gdb_test_name >> + } else { >> + fail $gdb_test_name >> + } >> + } >> +} >> + >> +# Alias of deferred Variable alias. >> +gdb_test_multiple "p g_def_var2_alias2" "print alias of alias of deferred variable" { >> + -re -wrap " = 3" { >> + pass $gdb_test_name >> + } >> + -re -wrap "has unknown type; cast it to its declared type" { >> + if { $old_clang } { >> + xfail $gdb_test_name >> + } else { >> + fail $gdb_test_name >> + } >> + } >> } >> diff --git a/gdb/testsuite/gdb.base/symbol-alias2.c b/gdb/testsuite/gdb.base/symbol-alias2.c >> index 34f4e121e25..41d4b6a53e5 100644 >> --- a/gdb/testsuite/gdb.base/symbol-alias2.c >> +++ b/gdb/testsuite/gdb.base/symbol-alias2.c >> @@ -23,6 +23,8 @@ struct S >> >> struct S g_var_s = { 1, 2 }; >> >> +static int g_var = 1; >> + >> #ifdef __cplusplus >> /* So that the alias attribute below work without having to figure out >> this function's mangled name. */ >> @@ -38,3 +40,17 @@ func (void) >> struct S *func_alias (void) __attribute__ ((alias ("func"))); >> >> extern struct S g_var_s_alias __attribute__ ((alias ("g_var_s"))); >> + >> +extern struct S g_var_s_alias2 __attribute__ ((alias ("g_var_s_alias"))); >> + >> +extern int g_var_alias __attribute__ ((alias ("g_var"))); >> + >> +extern int g_def_var_alias __attribute__ ((alias ("g_def_var"))); >> + >> +int g_def_var = 2; >> + >> +extern int g_def_var2_alias __attribute__ ((alias ("g_def_var2"))); >> + >> +int g_def_var2 = 3; Offhand, there doesn't seem to be any difference between g_def_var_alias and g_def_var2_alias? If we really need both, then I'm thinking this could use some comment. >> + >> +extern int g_def_var2_alias2 __attribute__ ((alias ("g_def_var2_alias"))); >> -- >> >> > Here's a slightly tweaked version of your patch, that I am aiming at merging, once we resolve the nits pointed out above. It just tweaks/adds comments in the code and the commit log, and tweaks formatting to be a bit more in line with GDB conventions. Please double check. >From 6a2748a6af5ed9ea266f115f73d603e4d0536db7 Mon Sep 17 00:00:00 2001 From: Kavitha Natarajan Date: Mon, 18 Apr 2022 14:53:55 +0530 Subject: [PATCH] Debug support for global alias variable Starting with (future) Clang 15 (since https://reviews.llvm.org/D120989), Clang emits the DWARF information of global alias variables as DW_TAG_imported_declaration. However, GDB does not handle it. It incorrectly always reads this tag as C++/Fortran imported declaration (type alias, namespace alias and Fortran module). This commit adds support to handle this tag as an alias variable. This change fixes the failures in the gdb.base/symbol-alias.exp testcase with current git Clang. This testcase is also updated to test nested (recursive) aliases. Change-Id: Ief83123f8c77b31751c00404c7521c9b86de74dd --- gdb/dwarf2/read.c | 63 +++++++++++++++-------- gdb/testsuite/gdb.base/symbol-alias.exp | 66 +++++++++++++++++++++++-- gdb/testsuite/gdb.base/symbol-alias2.c | 16 ++++++ 3 files changed, 121 insertions(+), 24 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 696277d931a..66af99df083 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -894,6 +894,10 @@ static void build_type_psymtabs_reader (cutu_reader *reader, static void dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile); +static void var_decode_location (struct attribute *attr, + struct symbol *sym, + struct dwarf2_cu *cu); + static unsigned int peek_abbrev_code (bfd *, const gdb_byte *); static const gdb_byte *read_attribute (const struct die_reader_specs *, @@ -1071,7 +1075,7 @@ static struct using_direct **using_directives (struct dwarf2_cu *cu); static void read_import_statement (struct die_info *die, struct dwarf2_cu *); -static int read_namespace_alias (struct die_info *die, struct dwarf2_cu *cu); +static bool read_alias (struct die_info *die, struct dwarf2_cu *cu); static struct type *read_module_type (struct die_info *die, struct dwarf2_cu *cu); @@ -8676,9 +8680,10 @@ process_die (struct die_info *die, struct dwarf2_cu *cu) break; case DW_TAG_imported_declaration: cu->processing_has_namespace_info = true; - if (read_namespace_alias (die, cu)) + if (read_alias (die, cu)) break; - /* The declaration is not a global namespace alias. */ + /* The declaration is neither a global namespace nor a variable + alias. */ /* Fall through. */ case DW_TAG_imported_module: cu->processing_has_namespace_info = true; @@ -9146,18 +9151,18 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) return retval; } -/* Inspect DIE in CU for a namespace alias. If one exists, record - a new symbol for it. +/* Inspect DIE in CU for a namespace alias or a variable with alias + attribute. If one exists, record a new symbol for it. - Returns 1 if a namespace alias was recorded, 0 otherwise. */ + Returns true if an alias was recorded, false otherwise. */ -static int -read_namespace_alias (struct die_info *die, struct dwarf2_cu *cu) +static bool +read_alias (struct die_info *die, struct dwarf2_cu *cu) { struct attribute *attr; - /* If the die does not have a name, this is not a namespace - alias. */ + /* If the die does not have a name, this is neither a namespace + alias nor a variable alias. */ attr = dwarf2_attr (die, DW_AT_name, cu); if (attr != NULL) { @@ -9183,26 +9188,42 @@ read_namespace_alias (struct die_info *die, struct dwarf2_cu *cu) { complaint (_("DIE at %s has too many recursively imported " "declarations"), sect_offset_str (d->sect_off)); - return 0; + return false; } if (attr != NULL) { struct type *type; - sect_offset sect_off = attr->get_ref_die_offset (); - - type = get_die_type_at_offset (sect_off, cu->per_cu, cu->per_objfile); - if (type != NULL && type->code () == TYPE_CODE_NAMESPACE) + if (d->tag == DW_TAG_variable) { - /* This declaration is a global namespace alias. Add - a symbol for it whose type is the aliased namespace. */ - new_symbol (die, type, cu); - return 1; + /* This declaration is a C/C++ global variable alias. + Add a symbol for it whose type is the same as the + aliased variable's. */ + type = die_type (d, imported_cu); + struct symbol *sym = new_symbol (die, type, cu); + attr = dwarf2_attr (d, DW_AT_location, imported_cu); + sym->set_aclass_index (LOC_UNRESOLVED); + if (attr != nullptr) + var_decode_location (attr, sym, cu); + return true; + } + else + { + sect_offset sect_off = attr->get_ref_die_offset (); + type = get_die_type_at_offset (sect_off, cu->per_cu, + cu->per_objfile); + if (type != nullptr && type->code () == TYPE_CODE_NAMESPACE) + { + /* This declaration is a global namespace alias. Add + a symbol for it whose type is the aliased + namespace. */ + new_symbol (die, type, cu); + return true; + } } } } - - return 0; + return false; } /* Return the using directives repository (global or local?) to use in the diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp index 289f49bbc3f..a944928b415 100644 --- a/gdb/testsuite/gdb.base/symbol-alias.exp +++ b/gdb/testsuite/gdb.base/symbol-alias.exp @@ -15,6 +15,10 @@ # along with this program. If not, see . standard_testfile symbol-alias.c symbol-alias2.c +set using_clang [test_compiler_info clang-*] +# Clang versions prior to v15 do not emit debug info for aliases. +set old_clang [expr [test_compiler_info {clang-1[0-4]-*-*}] \ + || [test_compiler_info {clang-[1-9]-*}]] if { [prepare_for_testing "failed to prepare" ${testfile} [list $srcfile $srcfile2]] } { return -1 @@ -30,7 +34,63 @@ foreach f {"func" "func_alias"} { gdb_test "p *${f}()" "= {field1 = 1, field2 = 2}" } -# Variables. -foreach v {"g_var_s" "g_var_s_alias"} { - gdb_test "p $v" "= {field1 = 1, field2 = 2}" +# Extern global variable. +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}" +foreach v {"g_var_s_alias" "g_var_s_alias2"} { + gdb_test_multiple "p $v" "print alias variable $v" { + -re -wrap " = {field1 = 1, field2 = 2}" { + pass $gdb_test_name + } + -re -wrap "has unknown type; cast it to its declared type" { + if { $old_clang } { + xfail $gdb_test_name + } else { + fail $gdb_test_name + } + } + } +} + +# Static global variable. +gdb_test "p g_var" " = 1" +gdb_test_multiple "p g_var_alias" "print alias of static variable" { + -re -wrap " = 1" { + pass $gdb_test_name + } + -re -wrap "has unknown type; cast it to its declared type" { + if { $old_clang } { + xfail $gdb_test_name + } else { + fail $gdb_test_name + } + } +} + +# Deferred variable. +gdb_test "p g_def_var" " = 2" +gdb_test_multiple "p g_def_var_alias" "print alias of deferred variable" { + -re -wrap " = 2" { + pass $gdb_test_name + } + -re -wrap "has unknown type; cast it to its declared type" { + if { $using_clang } { + xfail $gdb_test_name + } else { + fail $gdb_test_name + } + } +} + +# Alias of deferred variable alias. +gdb_test_multiple "p g_def_var2_alias2" "print alias of alias of deferred variable" { + -re -wrap " = 3" { + pass $gdb_test_name + } + -re -wrap "has unknown type; cast it to its declared type" { + if { $old_clang } { + xfail $gdb_test_name + } else { + fail $gdb_test_name + } + } } diff --git a/gdb/testsuite/gdb.base/symbol-alias2.c b/gdb/testsuite/gdb.base/symbol-alias2.c index 34f4e121e25..41d4b6a53e5 100644 --- a/gdb/testsuite/gdb.base/symbol-alias2.c +++ b/gdb/testsuite/gdb.base/symbol-alias2.c @@ -23,6 +23,8 @@ struct S struct S g_var_s = { 1, 2 }; +static int g_var = 1; + #ifdef __cplusplus /* So that the alias attribute below work without having to figure out this function's mangled name. */ @@ -38,3 +40,17 @@ func (void) struct S *func_alias (void) __attribute__ ((alias ("func"))); extern struct S g_var_s_alias __attribute__ ((alias ("g_var_s"))); + +extern struct S g_var_s_alias2 __attribute__ ((alias ("g_var_s_alias"))); + +extern int g_var_alias __attribute__ ((alias ("g_var"))); + +extern int g_def_var_alias __attribute__ ((alias ("g_def_var"))); + +int g_def_var = 2; + +extern int g_def_var2_alias __attribute__ ((alias ("g_def_var2"))); + +int g_def_var2 = 3; + +extern int g_def_var2_alias2 __attribute__ ((alias ("g_def_var2_alias"))); base-commit: 5ca5b31d6381ba6d0309e6ae0b1527a852d08a4b -- 2.36.0