From patchwork Wed Dec 13 10:36:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 24910 Received: (qmail 7470 invoked by alias); 13 Dec 2017 10:37:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7457 invoked by uid 89); 13 Dec 2017 10:37:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=mandated X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Dec 2017 10:37:02 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 6B7AE116DF8; Wed, 13 Dec 2017 05:37:00 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id T8GI59bmqLB7; Wed, 13 Dec 2017 05:37:00 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id E2D1C116DF7; Wed, 13 Dec 2017 05:36:59 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 7E8B689D47; Wed, 13 Dec 2017 14:36:55 +0400 (+04) Date: Wed, 13 Dec 2017 14:36:55 +0400 From: Joel Brobecker To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: [RFC] regresssion(internal-error) printing subprogram argument Message-ID: <20171213103655.msbaxfrykc36f4a7@adacore.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: NeoMutt/20170113 (1.7.2) Hi Pedro, I don't know if you remember about the patch series that introduces wild matching for C++ as well? I havent' verified this in details, because there is a series of patches, and they are fairly long, but I have a feeling that they may have introduced a regression. I can bisect tomorrow if needed. In any case, consider the following code which first declares a tagged type (the equivalent of a class in Ada), and then a procedure which takes a pointer (access) to this type's 'Class. package Pck is type Top_T is tagged record N : Integer := 1; end record; procedure Inspect (Obj: access Top_T'Class); end Pck; Putting a breakpoint in that procedure and then running to it triggers an internal error: (gdb) break inspect (gdb) continue Breakpoint 1, pck.inspect (obj=0x63e010 /[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed. What's special about this subprogram is that it takes an access to what we call a 'Class type, and for implementation reasons, the compiler adds an extra argument named "objL". If you are curious why, it allows the compiler for perform dynamic accessibility checks that are mandated by the language. If we look at the location where we get the internal error (in stack.c), we find that we are looping over the symbol of each parameter, and for each parameter, we do: /* We have to look up the symbol because arguments can have two entries (one a parameter, one a local) and the one we want is the local, which lookup_symbol will find for us. [...] nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL).symbol; gdb_assert (nsym != NULL); The lookup_symbol goes through the lookup structure, which means the symbol's linkage name ("objL") gets transformed into a lookup_name_info object (in block_lookup_symbol), before it gets fed to the block symbol dictionary iterators. This, in turn, triggers the symbol matching by comparing the "lookup" name which, for Ada, means among other things, lowercasing the given name to "objl". It is this transformation that causes the lookup find no matches, and therefore trip this assertion. Going back to the "offending" call to lookup_symbol in stack.c, what we are trying to do, here, is do a lookup by linkage name. So, I think what we mean to be doing is a completely litteral symbol lookup, so maybe not even strcmp_iw, but actually just plain strcmp??? There doesn't seem to be a way to do just that anymore, unfortunately. While this wasn't necessarily part of the spec, in the past, in practice, you could get that effect by doing a lookup using the C language. But that doesn't work, because we still end up somehow using Ada's lookup_name routine which transforms "objL". So, ideally, as I hinted before, I think what we need is a way to perform a litteral lookup so that searches by linkage names like the above can be performed. But this might be a fairly involved change (maybe adding a new kind of lookup, and then making adjustments so we know which kind of name to use for name matching). Failing that, the attached prototype takes the easy approach of just making Ada special, and adjusting the name used for the lookup to encode in the name itself the fact that the lookup should be litteral. I tested it with both the official testsuite as well as AdaCore's testsuite, and it fixed the issue without regression that I could see. It's not ideal (not clean, IMO), but gets us back with minimal fuss. We could have that while we work on extending the current framework to allow litteral lookups -- I would just clean it up and post an official RFA for it. Can you tell me what you think? I'm sorry I didn't notice this when I did my review and round of testing. I did the best I could, but the current version of GDB showing 300+ failures in AdaCore's testsuite at that time, I used manual report comparison, and I must have missed these differences (7 failures). Note also that I consider this issue blocking for 8.1, as these are not just limited to access to 'Class parameters. These kinds of internal parameters can also be generated in other situations, and in particular when using tasking (the equivalent of threads). Tasking is fairly prevalent in Ada programs. We might even want to defer branching if we elect to take the more general fix of adding litteral lookup support... Thanks! From c48fd2d065cddafba771c6bead7ee16806505089 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Wed, 13 Dec 2017 03:53:13 -0500 Subject: [PATCH] WIP: `nsym != NULL' assert failure in stack.c::print_frame_args --- gdb/stack.c | 6 +++- gdb/testsuite/gdb.ada/access_tagged_param.exp | 37 +++++++++++++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/foo.adb | 20 ++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.ads | 21 +++++++++++++ 5 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param.exp create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/foo.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.ads diff --git a/gdb/stack.c b/gdb/stack.c index 6bd0d45..521cef1 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -615,8 +615,12 @@ print_frame_args (struct symbol *func, struct frame_info *frame, if (*SYMBOL_LINKAGE_NAME (sym)) { struct symbol *nsym; + std::string sym_linkage_name (SYMBOL_LINKAGE_NAME (sym)); - nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), + if (SYMBOL_LANGUAGE (sym) == language_ada) + sym_linkage_name = std::string("<") + sym_linkage_name + '>'; + + nsym = lookup_symbol (sym_linkage_name.c_str (), b, VAR_DOMAIN, NULL).symbol; gdb_assert (nsym != NULL); if (SYMBOL_CLASS (nsym) == LOC_REGISTER diff --git a/gdb/testsuite/gdb.ada/access_tagged_param.exp b/gdb/testsuite/gdb.ada/access_tagged_param.exp new file mode 100644 index 0000000..a5e399f --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param.exp @@ -0,0 +1,37 @@ +# Copyright 2017 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 . + +load_lib "ada.exp" + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } { + return -1 +} + +clean_restart ${testfile} + +if ![runto "foo"] then { + perror "Couldn't run ${testfile}" + return +} + +gdb_breakpoint "pck.inspect" + +# Continue until we reach the breakpoint, and verify that we can print +# the value of all the parameters. + +gdb_test "continue" \ + ".*Breakpoint $decimal, pck\\.inspect \\(obj=$hex, =2\\) at .*" diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb new file mode 100644 index 0000000..bd7e641 --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb @@ -0,0 +1,20 @@ +-- Copyright 2017 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 . + +with Pck; use Pck; +procedure Foo is +begin + Inspect (new Top_T'(N => 2)); +end Foo; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb new file mode 100644 index 0000000..88fa970 --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2017 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 . + +package body Pck is + procedure Inspect (Obj: access Top_T'Class) is + begin + null; + end Inspect; +end Pck; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads new file mode 100644 index 0000000..bbb5c8d --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads @@ -0,0 +1,21 @@ +-- Copyright 2017 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 . + +package Pck is + type Top_T is tagged record + N : Integer := 1; + end record; + procedure Inspect (Obj: access Top_T'Class); +end Pck; -- 2.1.4