Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v2 11/13] Handle "p 'S::method()::static_var'" (quoted) in symbol lookup
Date: Thu, 13 Jul 2017 15:32:00 -0000	[thread overview]
Message-ID: <1499959929-29497-12-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1499959929-29497-1-git-send-email-palves@redhat.com>

While the previous commit made "p method()::static_var" (no
single-quotes) Just Work, if users (or frontends) try wrapping the
expression with quotes, they'll get:

  (gdb) p 'S::method()::static_var'
  'S::method()::static_var' has unknown type; cast it to its declared type

even if we _do_ have debug info for that variable.  That's better than
the bogus/confusing value what GDB would print before the
stop-assuming-int patch:

  (gdb) p 'S::method()::static_var'
  $1 = 1

but I think it'd still be nice to make this case Just Work too.

In this case, due to the quoting, the C/C++ parser (c-exp.y)
interprets the whole expression/string as a single symbol name, and we
end up calling lookup_symbol on that name.  There's no debug symbol
with that fully-qualified name, but since the compiler gives the
static variable a mangled linkage name exactly like the above, it
appears in the mininal symbols:

  $ nm -A local-static | c++filt | grep static_var
  local-static:0000000000601040 d S::method()::static_var

... and that's what GDB happens to find/print.  This only happens in
C++, note, since for C the compiler uses different linkage names:

  local-static-c:0000000000601040 d static_var.1848

So while (in C++, not C) function local static variables are given a
mangled name that demangles to the same syntax that GDB
documents/expects as the way to access function local statics, there's
no global symbol in the debug info with that name at all.  The debug
info for a static local variable for a non-inline function looks like
this:

 <1><2a1>: Abbrev Number: 19 (DW_TAG_subprogram)
 ...
 <2><2f7>: Abbrev Number: 20 (DW_TAG_variable)
    <2f8>   DW_AT_name        : (indirect string, offset: 0x4e9): static_var
    <2fc>   DW_AT_decl_file   : 1
    <2fd>   DW_AT_decl_line   : 64
    <2fe>   DW_AT_type        : <0x25>
    <302>   DW_AT_location    : 9 byte block: 3 40 10 60 0 0 0 0 0      (DW_OP_addr: 601040)

and for an inline function, it looks like this (linkage name run
through c++filt for convenience):

 <2><21b>: Abbrev Number: 16 (DW_TAG_variable)
    <21c>   DW_AT_name        : (indirect string, offset: 0x21a): static_var
    <220>   DW_AT_decl_file   : 1
    <221>   DW_AT_decl_line   : 48
    <222>   DW_AT_linkage_name: (indirect string, offset: 0x200): S::inline_method()::static_var
    <226>   DW_AT_type        : <0x25>
    <22a>   DW_AT_external    : 1
    <22a>   DW_AT_location    : 9 byte block: 3 a0 10 60 0 0 0 0 0      (DW_OP_addr: 6010a0)

(The inline case makes the variable external so that the linker can
merge the different inlined copies.  It seems like GCC never outputs
the linkage name for non-extern globals.)

When we read the DWARF, we record the static_var variable as a regular
variable of the containing function's block.  This makes stopping in
the function and printing the variable as usual.  The variable just so
happens to have a memory address as location.

So one way to make "p 'S::method()::static_var'" work would be to
record _two_ copies of the symbols for these variables.  One in the
function's scope/block, with "static_var" as name, as we currently do,
and another in the static or global blocks (depending on whether the
symbol is external), with a fully-qualified name.  I wrote a prototype
patch for that, and it works.  For the non-inline case above, since
the debug info doesn't point to the linkage same, that patch built the
physname of the static local variable as the concat of the physname of
the containing function, plus "::", plus the variable's name.  We
could make that approach work for C too, though it kind of feels
awkward to record fake symbol names like that in C.

The other approach I tried is to change the C++ symbol lookup routines
instead.  This is the approach this commit takes.  We can already
lookup up symbol in namespaces and classes, so this feels like a good
fit, and was easy enough.  The advantage is that this doesn't require
recording extra symbols.

The test in gdb.cp/m-static.exp that exposed the need for this is
removed, since the same functionality is now covered by
gdb.cp/local-static.exp.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* cp-namespace.c (cp_search_static_and_baseclasses): Handle
	function/method scopes; lookup the nested name as a function local
	static variable.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/local-static.exp: Also test with
	class::method::variable wholly quoted.
	* gdb.cp/m-static.exp (class::method::variable): Remove test.
---
 gdb/cp-namespace.c                    | 50 +++++++++++++++++++----------------
 gdb/std-operator.def                  | 11 +++++++-
 gdb/testsuite/gdb.cp/local-static.exp | 12 +++++++++
 gdb/testsuite/gdb.cp/m-static.exp     |  5 ----
 4 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index b96c421..c7b5aa8 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -246,40 +246,44 @@ cp_search_static_and_baseclasses (const char *name,
 				  unsigned int prefix_len,
 				  int is_in_anonymous)
 {
-  struct block_symbol sym;
-  struct block_symbol klass_sym;
-  struct type *klass_type;
-
   /* Check for malformed input.  */
   if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
     return null_block_symbol;
 
-  /* Find the name of the class and the name of the method, variable, etc.  */
-
-  /* The class name is everything up to and including PREFIX_LEN.  */
-  std::string klass (name, prefix_len);
+  /* The class, namespace or function name is everything up to and
+     including PREFIX_LEN.  */
+  std::string scope (name, prefix_len);
 
   /* The rest of the name is everything else past the initial scope
      operator.  */
-  std::string nested (name + prefix_len + 2);
-
-  /* Lookup a class named KLASS.  If none is found, there is nothing
-     more that can be done.  KLASS could be a namespace, so always look
-     in VAR_DOMAIN.  This works for classes too because of
-     symbol_matches_domain (which should be replaced with something else,
-     but it's what we have today).  */
-  klass_sym = lookup_global_symbol (klass.c_str (), block, VAR_DOMAIN);
-  if (klass_sym.symbol == NULL)
+  const char *nested = name + prefix_len + 2;
+
+  /* Lookup the scope symbol.  If none is found, there is nothing more
+     that can be done.  SCOPE could be a namespace, so always look in
+     VAR_DOMAIN.  This works for classes too because of
+     symbol_matches_domain (which should be replaced with something
+     else, but it's what we have today).  */
+  block_symbol scope_sym = lookup_symbol_in_static_block (scope.c_str (),
+							  block, VAR_DOMAIN);
+  if (scope_sym.symbol == NULL)
+    scope_sym = lookup_global_symbol (scope.c_str (), block, VAR_DOMAIN);
+  if (scope_sym.symbol == NULL)
     return null_block_symbol;
-  klass_type = SYMBOL_TYPE (klass_sym.symbol);
 
-  /* Look for a symbol named NESTED in this class.
+  struct type *scope_type = SYMBOL_TYPE (scope_sym.symbol);
+
+  /* If the scope is a function/method, then look up NESTED as a local
+     static variable.  E.g., "print 'function()::static_var'".  */
+  if (TYPE_CODE (scope_type) == TYPE_CODE_FUNC
+      || TYPE_CODE (scope_type) == TYPE_CODE_METHOD)
+    return lookup_symbol (nested, SYMBOL_BLOCK_VALUE (scope_sym.symbol),
+			  VAR_DOMAIN, NULL);
+
+  /* Look for a symbol named NESTED in this class/namespace.
      The caller is assumed to have already have done a basic lookup of NAME.
      So we pass zero for BASIC_LOOKUP to cp_lookup_nested_symbol_1 here.  */
-  sym = cp_lookup_nested_symbol_1 (klass_type, nested.c_str (), name,
-				   block, domain, 0, is_in_anonymous);
-
-  return sym;
+  return cp_lookup_nested_symbol_1 (scope_type, nested, name,
+				    block, domain, 0, is_in_anonymous);
 }
 
 /* Look up NAME in the C++ namespace NAMESPACE.  Other arguments are
diff --git a/gdb/std-operator.def b/gdb/std-operator.def
index 56a9af9..344ba25 100644
--- a/gdb/std-operator.def
+++ b/gdb/std-operator.def
@@ -298,7 +298,16 @@ OP (OP_SCOPE)
      p 'S:method() const'::var
 
    then the C-specific handling directly in the parser takes over (see
-   "block/variable productions).  */
+   "block/variable productions).
+
+   Also, if the whole function+var is quoted like this:
+
+     p 'S:method() const::var'
+
+   then the whole quoted expression is interpreted as a single symbol
+   name and we don't use OP_FUNC_STATIC_VAR either.  In that case, the
+   C++-specific symbol lookup routines take care of the
+   function-local-static search.  */
 OP (OP_FUNC_STATIC_VAR)
 
 /* OP_TYPE is for parsing types, and used with the "ptype" command
diff --git a/gdb/testsuite/gdb.cp/local-static.exp b/gdb/testsuite/gdb.cp/local-static.exp
index ba0c5ef..581efa3 100644
--- a/gdb/testsuite/gdb.cp/local-static.exp
+++ b/gdb/testsuite/gdb.cp/local-static.exp
@@ -95,6 +95,11 @@ proc do_test {lang} {
     #
     #  'func()'::var
     #  func()::var
+    #  'func()::var'
+    #
+    # In C++, the latter case makes sure that symbol lookup finds the
+    # debug symbol instead of the minimal symbol with that exact same
+    # name.
 
     foreach scope_line $scopes_list  {
 	set scope [lindex $scope_line 0]
@@ -105,6 +110,13 @@ proc do_test {lang} {
 
 	    gdb_test "print '${scope}'::${var_prefix}_${var}" $print_re
 	    gdb_test "print ${scope}::${var_prefix}_${var}" $print_re
+
+	    set sym "${scope}::${var_prefix}_${var}"
+	    if {$lang == "c++"} {
+		gdb_test "print '${sym}'" $print_re
+	    } else {
+		gdb_test "print '${sym}'" "No symbol \"$sym\" in current context\\."
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
index eeb88e9..10239a3 100644
--- a/gdb/testsuite/gdb.cp/m-static.exp
+++ b/gdb/testsuite/gdb.cp/m-static.exp
@@ -52,11 +52,6 @@ gdb_continue_to_breakpoint "end of constructors"
 
 # One.
 
-# simple object, static const int, accessing via 'class::method::variable'
-# Regression test for PR c++/15203 and PR c++/15210
-gdb_test "print (int) 'gnu_obj_1::method()::sintvar'" "\\$\[0-9\]+ = 4" \
-    "simple object, static int, accessing via 'class::method::variable'"
-
 # simple object, static const bool
 gdb_test "print test1.test" "\\$\[0-9\]* = true" "simple object, static const bool"
 
-- 
2.5.5


  parent reply	other threads:[~2017-07-13 15:32 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-13 15:32 [PATCH v2 00/13] No-debug-info debugging improvements Pedro Alves
2017-07-13 15:32 ` [PATCH v2 09/13] Eliminate UNOP_MEMVAL_TLS Pedro Alves
2017-07-13 15:32 ` [PATCH v2 03/13] Introduce OP_VAR_MSYM_VALUE Pedro Alves
2017-07-13 15:32 ` [PATCH v2 06/13] evaluate_subexp_standard: Remove useless assignments Pedro Alves
2017-07-13 15:32 ` [PATCH v2 01/13] Fix calling prototyped functions via function pointers Pedro Alves
2017-07-13 15:32 ` Pedro Alves [this message]
2017-07-13 15:32 ` [PATCH v2 10/13] Handle "p S::method()::static_var" in the C++ parser Pedro Alves
2017-07-13 15:32 ` [PATCH v2 05/13] evaluate_subexp_standard: Eliminate one goto Pedro Alves
2017-07-13 15:32 ` [PATCH v2 02/13] Stop assuming no-debug-info functions return int Pedro Alves
2017-07-13 15:37 ` [PATCH v2 12/13] Make "p S::method() const::static_var" work too Pedro Alves
2017-07-13 15:38 ` [PATCH v2 07/13] evaluate_subexp_standard: Factor out OP_VAR_VALUE handling Pedro Alves
2017-07-13 15:38 ` [PATCH v2 13/13] Document "no debug info debugging" improvements Pedro Alves
2017-07-13 16:11   ` Eli Zaretskii
2017-07-13 16:31     ` Pedro Alves
2017-07-13 17:54       ` Eli Zaretskii
2017-07-13 15:38 ` [PATCH v2 08/13] Stop assuming no-debug-info variables have type int Pedro Alves
2017-12-07 23:30   ` [RFA] Adjust gdb.arch/i386-sse-stack-align.exp print statement Sergio Durigan Junior
2017-12-08 10:47     ` Pedro Alves
2017-12-08 16:28       ` Sergio Durigan Junior
2017-07-13 15:39 ` [PATCH v2 04/13] Make ptype/whatis print function name of functions with no debug info too Pedro Alves
2017-09-04 19:21 ` [PATCH v2 00/13] No-debug-info debugging improvements Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1499959929-29497-12-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox