Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jan Kratochvil]

On Fri, 16 Dec 2016 14:17:32 +0100, Jonathan Wakely wrote:
> On 16/12/16 14:06 +0100, Jan Kratochvil wrote:
> > (gdb) p bb.c.d
> > $4 = (D &) @0x601028: {e = 0x601060 }
> 
> Wat?
> 
> bb.c.d is not a valid expression.
> 
> B::c is a pointer, it should be bb.c->d
> 
> Is it GDB policy to make invalid expressions like that "work"?

Yes, this is a GDB extension from the times of plain C.  In C++ it became
a problem.  That was not the topic of this example for the initial
dereferencing operator.


> I had no idea this even worked, I'd have used bb.c->d.e->i because
> that's the correct expression for accessing that variable.

There was a plan to make the 'compile' project backward compatible with these
confusing GDB universal dot operators.


Jan

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jonathan Wakely]

On 16/12/16 14:06 +0100, Jan Kratochvil wrote:

On Fri, 16 Dec 2016 13:33:52 +0100, Jonathan Wakely wrote:

We don't do auto-deref for std::shared_ptr or std::unique_ptr, even
though we know the object they point to definitely is live and safe to
access, and that's because those types have pointer semantics not
reference semantics.


This is wrong std::shared_ptr or std::unique_ptr is not auto-dereferenced for
GDB printing.  But it may be more a GDB problem, not libstdc++ pretty printers
problem.

For example glib pretty printers already auto-dereference data structures:
5  GList* list = NULL;
(gdb) p/r list
$1 = (GList *) 0x607a00
(gdb) p list
$2 = 0x607a00 = {0x400810}
/usr/share/glib-2.0/gdb/glib.py
if type.code == gdb.TYPE_CODE_PTR:
type = type.target().unqualified()
t = str(type)
if t == "GList":
return GListPrinter(val, "GList")

But that is more a GDB bug that should be solved even for generic pointers.
Currently while traversing through data structures one has to randomly add or
remove '*' from the beginning of the GDB print expression:

1   class E { int a[1000]; int i=42; } ee;
2   class D { E *e= } dd;
3   class C { D =dd; } cc;
4   class B { C *c= } bb;
5   int main() {}
(gdb) p bb
$1 = {c = 0x601030 }
(gdb) p bb.c
$2 = (C *) 0x601030 
Oops, I need to add a dereference:
(gdb) p *bb.c
$3 = {d = @0x601028}
(gdb) p *bb.c.d
No symbol "operator*" in current context.
Oops, I need to remove a dereference:
(gdb) p bb.c.d
$4 = (D &) @0x601028: {e = 0x601060 }


Wat?

bb.c.d is not a valid expression.

B::c is a pointer, it should be bb.c->d

Is it GDB policy to make invalid expressions like that "work"?

Because I'm not comfortable emulating that in the libstdc++ printers.



(gdb) p bb.c.d.e
$5 = (E *) 0x601060 
Oops, I need to add a dereference:
(gdb) p *bb.c.d.e
$6 = {a = {0 }, i = 42}
(gdb) p *bb.c.d.e.i
Cannot access memory at address 0x2a
Oops, I need to remove a dereference:
(gdb) p bb.c.d.e.i
$7 = 42


I had no idea this even worked, I'd have used bb.c->d.e->i because
that's the correct expression for accessing that variable.

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jan Kratochvil]

On Fri, 16 Dec 2016 13:33:52 +0100, Jonathan Wakely wrote:
> We don't do auto-deref for std::shared_ptr or std::unique_ptr, even
> though we know the object they point to definitely is live and safe to
> access, and that's because those types have pointer semantics not
> reference semantics.

This is wrong std::shared_ptr or std::unique_ptr is not auto-dereferenced for
GDB printing.  But it may be more a GDB problem, not libstdc++ pretty printers
problem.

For example glib pretty printers already auto-dereference data structures:
5  GList* list = NULL;
(gdb) p/r list
$1 = (GList *) 0x607a00
(gdb) p list
$2 = 0x607a00 = {0x400810}
/usr/share/glib-2.0/gdb/glib.py
if type.code == gdb.TYPE_CODE_PTR:
type = type.target().unqualified()
t = str(type)
if t == "GList":
return GListPrinter(val, "GList")

But that is more a GDB bug that should be solved even for generic pointers.
Currently while traversing through data structures one has to randomly add or
remove '*' from the beginning of the GDB print expression:

1   class E { int a[1000]; int i=42; } ee;
2   class D { E *e= } dd;
3   class C { D =dd; } cc;
4   class B { C *c= } bb;
5   int main() {}
(gdb) p bb
$1 = {c = 0x601030 }
(gdb) p bb.c
$2 = (C *) 0x601030 
Oops, I need to add a dereference:
(gdb) p *bb.c
$3 = {d = @0x601028}
(gdb) p *bb.c.d
No symbol "operator*" in current context.
Oops, I need to remove a dereference:
(gdb) p bb.c.d
$4 = (D &) @0x601028: {e = 0x601060 }
(gdb) p bb.c.d.e
$5 = (E *) 0x601060 
Oops, I need to add a dereference:
(gdb) p *bb.c.d.e
$6 = {a = {0 }, i = 42}
(gdb) p *bb.c.d.e.i
Cannot access memory at address 0x2a
Oops, I need to remove a dereference:
(gdb) p bb.c.d.e.i
$7 = 42

This is probably solved in some clickable front-end interfaces.


Jan

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jonathan Wakely]

On 16/12/16 08:51 +0100, Jan Kratochvil wrote:

On Fri, 16 Dec 2016 02:07:07 +0100, Jonathan Wakely wrote:

On 15/12/16 22:19 +0100, Jan Kratochvil wrote:
> Just with the GDB 'compile' project (libcc1) which is planned to be used for
> all GDB expressions evalation the Xmethods will no longer work.

But then *it can just get compiled, so it will still work, right?

The only reason it doesn't work today is that the definition for
operator* might not be in the executable, but if you can compile a new
definition that doesn't matter.


Currently it cannot as the source for gcc (via libcc1) is (re)generated from
DWARF.  Currently GDB does not provide original sources as a context for the
compiled expression.


Then maybe Xmethods should continue to work ;-)

The reason I'm so strongly against auto-dereferencing iterators is
that it's inconsistent, and dangerous. It only happens for *some*
iterators, the small set of containers from the C++03 std::lib that we
have printers for, and not for other iterators (such as pointers, or
iterators from C++11 containers, or user-defined iterators).

It's fundamentally wrong. An iterator is not the value it points at,
because it might not even point at anything (it could have a garbage
pointer that points to deallocated memory, or be past-the-end, which
isn't detectable in general, or perform some additional work to obtain
a value, like reverse_iterator does).

We don't do auto-deref for std::shared_ptr or std::unique_ptr, even
though we know the object they point to definitely is live and safe to
access, and that's because those types have pointer semantics not
reference semantics. Like iterators.

And if we can make 'print *it' always work then we don't need that
inconsistent auto-deref behaviour. It's up to the person driving GDB
to decide whether to try and dereference an iterator, we don't make
that decision for them.


Besides that there is a C++17 modules feature which should solve that for
LLDB as pointed out by Pedro before:
http://lists.llvm.org/pipermail/lldb-dev/2016-August/010870.html
Unaware if/when GDB 'compile' will handle C++ modules as a sources substitute
or if/when it will do the original source recompilation.


I think we're still a long way from being able to rely on that in the
GNU toolchain.

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jan Kratochvil]

On Fri, 16 Dec 2016 02:07:07 +0100, Jonathan Wakely wrote:
> On 15/12/16 22:19 +0100, Jan Kratochvil wrote:
> > Just with the GDB 'compile' project (libcc1) which is planned to be used for
> > all GDB expressions evalation the Xmethods will no longer work.
> 
> But then *it can just get compiled, so it will still work, right?
> 
> The only reason it doesn't work today is that the definition for
> operator* might not be in the executable, but if you can compile a new
> definition that doesn't matter.

Currently it cannot as the source for gcc (via libcc1) is (re)generated from
DWARF.  Currently GDB does not provide original sources as a context for the
compiled expression.

Besides that there is a C++17 modules feature which should solve that for
LLDB as pointed out by Pedro before:
http://lists.llvm.org/pipermail/lldb-dev/2016-August/010870.html
Unaware if/when GDB 'compile' will handle C++ modules as a sources substitute
or if/when it will do the original source recompilation.


Jan

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jonathan Wakely]

On 15/12/16 22:19 +0100, Jan Kratochvil wrote:

On Thu, 15 Dec 2016 15:18:17 +0100, Jonathan Wakely wrote:

I'm going to add Xmethods for all our iterator types so that it will
always be possible to do "print *iter", so if GDB supports Xmethods
then we don't need to register the iterator printers.


Just with the GDB 'compile' project (libcc1) which is planned to be used for
all GDB expressions evalation the Xmethods will no longer work.


Ah.

But then *it can just get compiled, so it will still work, right?

The only reason it doesn't work today is that the definition for
operator* might not be in the executable, but if you can compile a new
definition that doesn't matter.

Re: [PATCH] PR59170 make pretty printers check for singular iterators

[Jan Kratochvil]

On Thu, 15 Dec 2016 15:18:17 +0100, Jonathan Wakely wrote:
> I'm going to add Xmethods for all our iterator types so that it will
> always be possible to do "print *iter", so if GDB supports Xmethods
> then we don't need to register the iterator printers.

Just with the GDB 'compile' project (libcc1) which is planned to be used for
all GDB expressions evalation the Xmethods will no longer work.


Jan

[PATCH] PR59170 make pretty printers check for singular iterators

[Jonathan Wakely]

This is another partial fix for PR 59170, this time adding checks for
normal mode iterators that are default-constructed, so we don't try to
dereference null pointers.

We still auto-dereference past-the-end iterators, and iterators that
have been invalidated by container mutation. The former can be
detected in debug mode (with some work to teach each iterator type how
to compare itself to the container's end()) and the latter is already
handled for debug mode. Neither case can be derected in normal mode.

I feel quite strongly that we should disable the printers for
iterators (the ones that make "print iter" automatically dereference
the iterator and print what it points to ... or garbage ... or crash).
I'm going to add Xmethods for all our iterator types so that it will
always be possible to do "print *iter", so if GDB supports Xmethods
then we don't need to register the iterator printers.

PR libstdc++/59170
* python/libstdcxx/v6/printers.py (StdListIteratorPrinter.to_string)
(StdSlistIteratorPrinter.to_string, StdVectorIteratorPrinter.to_string)
(StdRbtreeIteratorPrinter.to_string)
(StdDequeIteratorPrinter.to_string): Add check for value-initialized
iterators.
* testsuite/libstdc++-prettyprinters/simple.cc: Test them.
* testsuite/libstdc++-prettyprinters/simple11.cc: Likewise.

Tested x86_64-linux, committed to trunk.
commit 7b73bc563a5b4828b80e18d34cc06c0cbdae12ef
Author: Jonathan Wakely 
Date:   Thu Dec 15 14:05:24 2016 +

PR59170 make pretty printers check for singular iterators

PR libstdc++/59170
* python/libstdcxx/v6/printers.py (StdListIteratorPrinter.to_string)
(StdSlistIteratorPrinter.to_string, StdVectorIteratorPrinter.to_string)
(StdRbtreeIteratorPrinter.to_string)
(StdDequeIteratorPrinter.to_string): Add check for value-initialized
iterators.
* testsuite/libstdc++-prettyprinters/simple.cc: Test them.
* testsuite/libstdc++-prettyprinters/simple11.cc: Likewise.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py 
b/libstdc++-v3/python/libstdcxx/v6/printers.py
index ab3592a..86de1ca 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -200,6 +200,8 @@ class StdListIteratorPrinter:
 self.typename = typename
 
 def to_string(self):
+if not self.val['_M_node']:
+return 'non-dereferenceable iterator for std::list'
 nodetype = find_type(self.val.type, '_Node')
 nodetype = nodetype.strip_typedefs().pointer()
 node = self.val['_M_node'].cast(nodetype).dereference()
@@ -246,6 +248,8 @@ class StdSlistIteratorPrinter:
 self.val = val
 
 def to_string(self):
+if not self.val['_M_node']:
+return 'non-dereferenceable iterator for __gnu_cxx::slist'
 nodetype = find_type(self.val.type, '_Node')
 nodetype = nodetype.strip_typedefs().pointer()
 return str(self.val['_M_node'].cast(nodetype).dereference()['_M_data'])
@@ -333,6 +337,8 @@ class StdVectorIteratorPrinter:
 self.val = val
 
 def to_string(self):
+if not self.val['_M_current']:
+return 'non-dereferenceable iterator for std::vector'
 return str(self.val['_M_current'].dereference())
 
 class StdTuplePrinter:
@@ -494,6 +500,8 @@ class StdRbtreeIteratorPrinter:
 self.link_type = nodetype.strip_typedefs().pointer()
 
 def to_string (self):
+if not self.val['_M_node']:
+return 'non-dereferenceable iterator for associative container'
 node = self.val['_M_node'].cast(self.link_type).dereference()
 return str(get_value_from_Rb_tree_node(node))
 
@@ -708,6 +716,8 @@ class StdDequeIteratorPrinter:
 self.val = val
 
 def to_string(self):
+if not self.val['_M_cur']:
+return 'non-dereferenceable iterator for std::deque'
 return str(self.val['_M_cur'].dereference())
 
 class StdStringPrinter:
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc 
b/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc
index fb8e0d7..35fbb90 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/simple.cc
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 int
@@ -50,6 +51,9 @@ main()
   deq.push_back("two");
 // { dg-final { note-test deq {std::deque with 2 elements = {"one", "two"}} } }
 
+  std::deque::iterator deqiter0;
+// { dg-final { note-test deqiter0 {non-dereferenceable iterator for 
std::deque} } }
+
   std::deque::iterator deqiter = deq.begin();
 // { dg-final { note-test deqiter {"one"} } }
 
@@ -58,6 +62,9 @@ main()
   lst.push_back("two");
 // { dg-final { note-test lst {std::list = {[0] = "one", [1] = "two"}} } }
 
+  std::list::iterator lstiter0;
+// { dg-final { note-test lstiter0