Do not dereference std::unique_ptr by default

[tberghammer]

Summary

Displaying the object pointed by the unique_ptr can cause an infinite
recursion when we have a pointer loop so this change stops that
behavior. Additionally it makes the unique_ptr act more like a class
containing a pointer (what is the underlying truth) instead of some
"magic" class.

[jingham]

This test compiles correctly on Darwin if you pass the --std=c++11 flag. lldbtest has a getstdFlag that returns the correct std value in this case. Can you get the test running on Darwin with this?

Other than that this looks fine to me.

[tberghammer]

I am trying to compile it with the following command on OSX but I wasn't able to get it working:

clang  -std=c++11  -g -O0 -fno-builtin -arch x86_64   -fno-limit-debug-info -I$LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/../../../../../include -include $LLVM_ROOT/lldb/packages/Python/lldbsuite/test/make/test_common.h  -stdlib=libstdc++ -DLLDB_USING_LIBSTDCPP --driver-mode=g++ -c -o main.o main.cpp

Compile error (first few):

main.cpp:12:8: error: no member named 'unique_ptr' in namespace 'std'
  std::unique_ptr<char> nup;
  ~~~~~^
main.cpp:12:23: error: expected '(' for function-style cast or type construction
  std::unique_ptr<char> nup;
                  ~~~~^
main.cpp:12:25: error: use of undeclared identifier 'nup'; did you mean 'dup'?
  std::unique_ptr<char> nup;
                        ^~~
                        dup
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/unistd.h:438:6: note: 'dup' declared here
int      dup(int);
         ^
main.cpp:13:8: error: no member named 'unique_ptr' in namespace 'std'
  std::unique_ptr<int> iup(new int{123});
  ~~~~~^
main.cpp:13:22: error: expected '(' for function-style cast or type construction
  std::unique_ptr<int> iup(new int{123});
                  ~~~^
main.cpp:13:24: error: use of undeclared identifier 'iup'; did you mean 'dup'?
  std::unique_ptr<int> iup(new int{123});
                       ^~~
                       dup
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/unistd.h:438:6: note: 'dup' declared here
int      dup(int);

I think the problem is that this is testing libstdc++ what is not available on OSX.

Clang version:

Apple LLVM version 7.3.0 (clang-703.0.31)
Target: x86_64-apple-darwin16.3.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

[jingham]

There's no reason you couldn't build the gnu libstdc++ on Darwin. Anyway, if that's the problem, I'm pretty sure the testsuite has a way to conditionalize on which stdlib(s) are available. That would be clearer than conditionalizing on platform.

[tberghammer]

My understanding (don't have an OSX machine at hand right now to try out) is that OSX ships with the libstdc++ belonging to gcc-4.2.1 and that version of libstdc++ is too old to support c++11 features such as std::unique_ptr.

Regarding skipping tests I am not aware of any way to skip a test based on the STL library we are using (Pavel is working on it for libc++ at https://reviews.llvm.org/D30984) and actually the problem here is that the version of libstdc++ shipping on OSX is too old so I think skipIfDarwin is the correct decorator (we do it in several other tests as well). Alternative option could be to try to compile the source code and skip the test if compilation fails but that seems a bit flaky and might cause false negatives on other systems where we expect the test to pass.

[tberghammer]

I tried it out on OSX and the problem is that version of libstdc++ shipping with every recent OSX version (don't know about old ones) is 4.2.1 (last version with GPL v2) what doesn't support std::unique_ptr (supported since 4.3). Because of this I think the correct skip condition would be something like "skip if libstdc++ is older then 4.3" but I don't think we have a good way to specify that.

[jingham]

MacOS hasn't shipped with gcc for a while now. If you were serious about using gcc & its STL implementation you would install your own copies of the tools & libraries. So what's on the system isn't really determinative... But this is a minor point, if there's no way to check the library version we can just leave the test off for Darwin.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py

def test_with_run_command(self):
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped', 'stop reason = breakpoint'])
 
         frame = self.frame()
         self.assertTrue(frame.IsValid())
 
         self.expect("frame variable nup", substrs=['nup = nullptr'])
-        self.expect("frame variable iup", substrs=['iup = 0x', 'object = 123'])
-        self.expect("frame variable sup", substrs=['sup = 0x', 'object = "foobar"'])
+        self.expect("frame variable iup", substrs=['iup = 0x'])
+        self.expect("frame variable sup", substrs=['sup = 0x'])
 
         self.expect("frame variable ndp", substrs=['ndp = nullptr'])
-        self.expect("frame variable idp", substrs=['idp = 0x', 'object = 456', 'deleter = ', 'a = 1', 'b = 2'])
-        self.expect("frame variable sdp", substrs=['sdp = 0x', 'object = "baz"', 'deleter = ', 'a = 3', 'b = 4'])
-        
+        self.expect("frame variable idp", substrs=['idp = 0x', 'deleter = ', 'a = 1', 'b = 2'])
+        self.expect("frame variable sdp", substrs=['sdp = 0x', 'deleter = ', 'a = 3', 'b = 4'])
         self.assertEqual(123, frame.GetValueForVariablePath("iup.object").GetValueAsUnsigned())
         self.assertFalse(frame.GetValueForVariablePath("iup.deleter").IsValid())
 
         self.assertEqual('"foobar"', frame.GetValueForVariablePath("sup.object").GetSummary())
         self.assertFalse(frame.GetValueForVariablePath("sup.deleter").IsValid())
 
         self.assertEqual(456, frame.GetValueForVariablePath("idp.object").GetValueAsUnsigned())
         self.assertEqual('"baz"', frame.GetValueForVariablePath("sdp.object").GetSummary())
 
         idp_deleter = frame.GetValueForVariablePath("idp.deleter")
         self.assertTrue(idp_deleter.IsValid())
         self.assertEqual(1, idp_deleter.GetChildMemberWithName("a").GetValueAsUnsigned())
         self.assertEqual(2, idp_deleter.GetChildMemberWithName("b").GetValueAsUnsigned())
 
         sdp_deleter = frame.GetValueForVariablePath("sdp.deleter")
         self.assertTrue(sdp_deleter.IsValid())
         self.assertEqual(3, sdp_deleter.GetChildMemberWithName("a").GetValueAsUnsigned())
         self.assertEqual(4, sdp_deleter.GetChildMemberWithName("b").GetValueAsUnsigned())
+
+    @skipIfFreeBSD
+    @skipIfWindows  # libstdcpp not ported to Windows
+    @skipIfDarwin  # doesn't compile on Darwin
+    def test_recursive_unique_ptr(self):
+        # Tests that LLDB can handle when we have a loop in the unique_ptr
+        # reference chain and that it correctly handles the different options
+        # for the frame variable command in this case.
+        self.build()
+        self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_source_regexp(
+            self, "Set break point at this line.")
+        self.runCmd("run", RUN_SUCCEEDED)
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stopped', 'stop reason = breakpoint'])
+
+        self.expect("frame variable f1->fp",
+                    substrs=['fp = 0x'])
+        self.expect("frame variable --ptr-depth=1 f1->fp",
+                    substrs=['data = 2', 'fp = 0x'])
+        self.expect("frame variable --ptr-depth=2 f1->fp",
+                    substrs=['data = 2', 'fp = 0x', 'data = 1'])
+
+        frame = self.frame()
+        self.assertTrue(frame.IsValid())
+        self.assertEqual(2, frame.GetValueForVariablePath("f1->fp.object.data").GetValueAsUnsigned())
+        self.assertEqual(1, frame.GetValueForVariablePath("f1->fp.object.fp.object.data").GetValueAsUnsigned())
+

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/main.cpp

 #include <memory>
 #include <string>
 
 struct Deleter {
   void operator()(void *) {}
 
   int a;
   int b;
 };
 
+struct Foo {
+  int data;
+  std::unique_ptr<Foo> fp;
+};
+
 int main() {
   std::unique_ptr<char> nup;
   std::unique_ptr<int> iup(new int{123});
   std::unique_ptr<std::string> sup(new std::string("foobar"));
 
   std::unique_ptr<char, Deleter> ndp;
   std::unique_ptr<int, Deleter> idp(new int{456}, Deleter{1, 2});
   std::unique_ptr<std::string, Deleter> sdp(new std::string("baz"),
                                             Deleter{3, 4});
 
+  std::unique_ptr<Foo> fp(new Foo{3});
+
+  // Set up a structure where we have a loop in the unique_ptr chain.
+  Foo* f1 = new Foo{1};
+  Foo* f2 = new Foo{2};
+  f1->fp.reset(f2);
+  f2->fp.reset(f1);
+
   return 0; // Set break point at this line.
 }

lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

bool LibStdcppUniquePtrSyntheticFrontEnd::Update() {
   return false;
 }
 
 bool LibStdcppUniquePtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 lldb::ValueObjectSP
 LibStdcppUniquePtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx == 0)
-    return m_obj_obj;
+    return m_ptr_obj;
   if (idx == 1)
     return m_del_obj;
   if (idx == 2)
-    return m_ptr_obj;
+    return m_obj_obj;
   return lldb::ValueObjectSP();
 }
 
 size_t LibStdcppUniquePtrSyntheticFrontEnd::CalculateNumChildren() {
   if (m_del_obj)
     return 2;
-  if (m_ptr_obj && m_ptr_obj->GetValueAsUnsigned(0) != 0)
-    return 1;
-  return 0;
 }
 
 size_t LibStdcppUniquePtrSyntheticFrontEnd::GetIndexOfChildWithName(
     const ConstString &name) {
-  if (name == ConstString("obj") || name == ConstString("object"))
+  if (name == ConstString("ptr") || name == ConstString("pointer"))
     return 0;
   if (name == ConstString("del") || name == ConstString("deleter"))
     return 1;
-  if (name == ConstString("ptr") || name == ConstString("pointer"))
+  if (name == ConstString("obj") || name == ConstString("object"))
     return 2;
   return UINT32_MAX;
 }
 
 bool LibStdcppUniquePtrSyntheticFrontEnd::GetSummary(
     Stream &stream, const TypeSummaryOptions &options) {
   if (!m_ptr_obj)
     return false;