public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
* [expr-cumulative] RFA fix PR 9708
@ 2010-01-26 18:02 Tom Tromey
  2010-01-26 20:54 ` Keith Seitz
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2010-01-26 18:02 UTC (permalink / raw)
  To: Project Archer

Consider this code:

    struct K {
      int m () {
        static bool themagicstatic = false;

        return themagicstatic ? 23 : 24;
      }
    };

To my surprise, `themagicstatic' is actually marked extern and given a
mangled name:

    08049738 V K::m()::themagicstatic

I imagine this is so that K::m can be inlined, though I'm not sure.

In any case, PR 9708 is that if the inferior is stopped in K::m, then
"print themagicstatic" will not work.

I tracked this down to two problems.  First, one case in new_symbol was
unconditionally using the global scope for such symbols.  Second,
die_needs_namespace needed a tweak.

Built and regtested on x86-64 (compile farm).
New test included.  Please review.

I'll send this upstream once the physname patches go in there.

Tom

2010-01-26  Tom Tromey  <tromey@redhat.com>

	PR c++/9708:
	* dwarf2read.c (die_needs_namespace) <DW_TAG_variable>: A variable
	in a lexical block does not need a namespace.
	(new_symbol) <DW_TAG_variable>: Put extern variables on
	list_in_scope in all cases.

2010-01-26  Tom Tromey  <tromey@redhat.com>

	PR c++/9708:
	* gdb.cp/m-static.exp: Add regression test.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 09c1cae..ccecc3d 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2644,6 +2644,13 @@ die_needs_namespace (struct die_info *die, struct dwarf2_cu *cu)
 	attr = dwarf2_attr (die, DW_AT_external, cu);
 	if (attr == NULL && die->parent->tag != DW_TAG_namespace)
 	  return 0;
+	/* A variable in a lexical block of some kind does not need a
+	   namespace, even though in C++ such variables may be
+	   external and have a mangled name.  */
+	if (die->parent->tag ==  DW_TAG_lexical_block
+	    || die->parent->tag ==  DW_TAG_try_block
+	    || die->parent->tag ==  DW_TAG_catch_block)
+	  return 0;
 	return 1;
       }
       break;
@@ -8498,7 +8505,15 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 	      var_decode_location (attr, sym, cu);
 	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
 	      if (attr2 && (DW_UNSND (attr2) != 0))
-		add_symbol_to_list (sym, &global_symbols);
+		{
+		  struct pending **list_to_add;
+
+		  /* A variable with DW_AT_external is never static, but it
+		     may be block-scoped.  */
+		  list_to_add = (cu->list_in_scope == &file_symbols
+				 ? &global_symbols : cu->list_in_scope);
+		  add_symbol_to_list (sym, list_to_add);
+		}
 	      else
 		add_symbol_to_list (sym, cu->list_in_scope);
 	    }
diff --git a/gdb/testsuite/gdb.cp/m-static.cc b/gdb/testsuite/gdb.cp/m-static.cc
index 2a0b61c..7f997ef 100644
--- a/gdb/testsuite/gdb.cp/m-static.cc
+++ b/gdb/testsuite/gdb.cp/m-static.cc
@@ -15,6 +15,12 @@ protected:
 
 public:
   gnu_obj_1(antiquities a, long l) {}
+
+  long method ()
+  {
+    static bool svar = true;
+    return key2;
+  }
 };
 
 const bool gnu_obj_1::test;
@@ -70,5 +76,8 @@ int main()
 
   test4.dummy = test4.elsewhere;
   test4.dummy = 0;
-  return test4.dummy;	// breakpoint: constructs-done
+
+  test1.method (); // breakpoint: constructs-done
+
+  return test4.dummy;
 }
diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
index f207462..7b4e0ca 100644
--- a/gdb/testsuite/gdb.cp/m-static.exp
+++ b/gdb/testsuite/gdb.cp/m-static.exp
@@ -132,5 +132,10 @@ gdb_test "print test4.nowhere" "field nowhere is nonexistent or has been optimis
 # that GDB's current behavior in such situations is either consistent
 # across platforms or optimal, so I'm not including one now.
 
+# Step into test1.method and examine the method-scoped static.
+# This is a regression test for PR 9708.
+gdb_test "step" "gnu_obj_1::method.*"
+gdb_test "print svar" " = true"
+
 gdb_exit
 return 0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [expr-cumulative] RFA fix PR 9708
  2010-01-26 18:02 [expr-cumulative] RFA fix PR 9708 Tom Tromey
@ 2010-01-26 20:54 ` Keith Seitz
  2010-01-26 21:08   ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: Keith Seitz @ 2010-01-26 20:54 UTC (permalink / raw)
  To: tromey; +Cc: Project Archer

On 01/26/2010 10:02 AM, Tom Tromey wrote:
> @@ -2644,6 +2644,13 @@ die_needs_namespace (struct die_info *die, struct dwarf2_cu *cu)
>   	attr = dwarf2_attr (die, DW_AT_external, cu);
>   	if (attr == NULL&&  die->parent->tag != DW_TAG_namespace)
>   	  return 0;
> +	/* A variable in a lexical block of some kind does not need a
> +	   namespace, even though in C++ such variables may be
> +	   external and have a mangled name.  */
> +	if (die->parent->tag ==  DW_TAG_lexical_block
> +	    || die->parent->tag ==  DW_TAG_try_block
> +	    || die->parent->tag ==  DW_TAG_catch_block)
> +	  return 0;
>   	return 1;
>         }
>         break;

One question: should one be able to print the value of this static 
OUTSIDE the specific block in which it is defined? "print 
K::m::themagicstatic"?

This would almost certainly be a contrivance on our part for the sake of 
debugging, but it seems a reasonable thing to be able to do.

In any case, I don't have a problem with your patch.

Keith

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [expr-cumulative] RFA fix PR 9708
  2010-01-26 20:54 ` Keith Seitz
@ 2010-01-26 21:08   ` Tom Tromey
  0 siblings, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2010-01-26 21:08 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Project Archer

Keith> One question: should one be able to print the value of this static
Keith> OUTSIDE the specific block in which it is defined? "print
Keith> K::m::themagicstatic"?

It would have to be some other syntax, because this is valid:

    struct K {
      int m () {
        static bool themagicstatic = false;

        return themagicstatic ? 23 : 24;
      }

      class m {
        static bool themagicstatic;
      };
    };


... and K::m::themagicstatic unambiguously refers to the member of
class m.

I thought about trying to make K::m()::themagicstatic work, but that
also seems like a pain.

Keith> This would almost certainly be a contrivance on our part for the sake
Keith> of debugging, but it seems a reasonable thing to be able to do.

I agree.  I think it is a separate problem, though, because it would be
nice to also make it work for C, where there is no demangled form, etc.
Maybe if we ever get "HPD-like" in our symbol naming, or something like
that.

Keith> In any case, I don't have a problem with your patch.

Thanks, I'll commit shortly.

Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-01-26 21:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-26 18:02 [expr-cumulative] RFA fix PR 9708 Tom Tromey
2010-01-26 20:54 ` Keith Seitz
2010-01-26 21:08   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).