[lldb] Disable looking at pointee types to find synthetic value for non-ObjC

[aeubanks]

Summary

After D134378, we started seeing crashes with incomplete types (in the
context of shared libraries).

When trying to print a std::vector<int> & with only debug info for a
declaration, we now try to use the formatter after D134378. With an
incomplete type, this somehow goes into infinite recursion with the
frames

lldb_private::ValueObject::Dereference
lldb_private::ValueObjectSynthetic::CreateSynthFilter
lldb_private::ValueObjectSynthetic::ValueObjectSynthetic
lldb_private::ValueObject::CalculateSyntheticValue
lldb_private::ValueObject::HasSyntheticValue

This has to do with FrontEndWantsDereference that some STL formatters
set, causing recursion between the formatter (which tries to dereference),
and dereferencing (which wants to know if there's a formatter to avoid dereferencing).

The reason this only started appearing after D134378 was because
previously with incomplete types, for names with <, lldb would attempt
to parse template parameter DIEs, which were empty, then create an empty
ClassTemplateSpecializationDecl which overrode the name used to lookup
a formatter in FormattersMatchData() to not include template
parameters (e.g. std::vector<> &). After D134378 we don't create a
ClassTemplateSpecializationDecl when there are no template parameters
and the name to lookup a formatter is the original name (e.g.
std::vector<int> &).

The code to try harder with incomplete child compiler types was added in
D79554 for ObjC purposes.

[aeubanks]

lldb crash repro

$ cat /tmp/a.cc
#include <vector>

void f(std::vector<int>& v) {
        *(volatile int*) nullptr = 0;
}
$ cat /tmp/main.cc
#include <vector>

void f(std::vector<int>& v);

int main() {
        std::vector<int> v ;
        f(v);
}
$ ./build/bin/clang++ -g /tmp/a.cc -o /tmp/a.so -shared -stdlib=libc++ -fuse-ld=lld -Wl,-rpath,$PWD/build/lib
$ ./build/bin/clang++ -g /tmp/main.cc /tmp/a.so -o /tmp/a -stdlib=libc++ -fuse-ld=lld -Wl,-rpath,$PWD/build/lib
$ ./build/bin/lldb /tmp/a -b -o run
# crash

any help with adding a test case for this would be appreciated, I'm not sure if there's any testing for shared libraries in lldb?

[aeubanks]

also I'm don't understand how this code doesn't infinite recurse on ObjC and can't trace it because I don't have a mac

if anybody has an actual way of fixing this I'd be happy with that too

[dblaikie]

lldb/source/Core/ValueObject.cpp

2676–2680

Maybe worth filing a bug and referencing it here?

Is this limitation still necessary if the incomplete type has template parameter DIEs? (I guess probably yes, because it'll be missing member descriptions, etc)

& does this path get hit if the type is declared in one CU but defined in another? (& does the inf recurse/crash loop still get hit in that case, without this patch?)

[aeubanks]

add bug

[aeubanks]

lldb/source/Core/ValueObject.cpp

2676–2680

Maybe worth filing a bug and referencing it here?

Filed https://github.com/llvm/llvm-project/issues/59012, added here

Is this limitation still necessary if the incomplete type has template parameter DIEs? (I guess probably yes, because it'll be missing member descriptions, etc)

yes (I incorrectly mentioned in person that this works with -gsimple-template-names, it actually still infinite recurses)

& does this path get hit if the type is declared in one CU but defined in another? (& does the inf recurse/crash loop still get hit in that case, without this patch?)

if the declaration is in a shared library and the main binary has the definition, we hit this
if we have two CUs, one with a declaration, one with a definition, but both linked into the same binary, we don't hit the issue
AFAICT lldb restricts looking up debug info to the binary/shared library, but otherwise prefers definitions over declarations?

[labath]

We have tests for shared libraries (grep for DYLIB_C(XX)_SOURCES)), but it's easier to reproduce this problem by compiling one CU with -g0 -- see inline comment. (If you are creating an "API" test for this, beware that their default is to build everything with -fstandalone-debug, and you'll need to explicitly change that (see LIMIT_DEBUG_INFO_FLAGS))

This behavior (bug) is triggered by the FrontEndWantsDereference formatter flag, which is why it manifests itself for (libc++) vector and friends. The ObjC formatters don't set that flag so they should be safe. The libstdc++ formatters don't set it either. Furthermore there doesn't seem to be a way to set this flag by the user, so it is not possible to create a libc++-independent reproducer (which is what I was going to suggest).

I've been looking for a better way to fix this today (sorry about the delay), but I haven't figured out anything better. There is this fundamental recursion between the pretty printer (which wants to dereference a value) and the dereferencing code (which wants to know if it has a pretty printer -- so it can avoid dereferencing). Just removing the HasSyntheticValue()fixed the bug for me -- but then we were able to "dereference" incomplete structs. It's possible we may be able to allow the dereference to succeed here, but then refuse to print the value somewhere down the line. I would like to hear what @jingham things about all this.

lldb/source/Core/ValueObject.cpp

2676–2680

Yes, but if one of those CUs (the one that was supposed to contain the definition) is built without debug info, then we end up exactly in the same situation (a binary without a definition of a type), but without the complications that shared libraries bring.

[aeubanks]

add test

[aeubanks]

In D137983#3930973, @labath wrote:

We have tests for shared libraries (grep for DYLIB_C(XX)_SOURCES)), but it's easier to reproduce this problem by compiling one CU with -g0 -- see inline comment. (If you are creating an "API" test for this, beware that their default is to build everything with -fstandalone-debug, and you'll need to explicitly change that (see LIMIT_DEBUG_INFO_FLAGS))

lldb/test/API/lang/cpp/incomplete-types/ looks like exactly what I want, I've added a test based on that (and verified that lldb crashes in that test without the source change)

This behavior (bug) is triggered by the FrontEndWantsDereference formatter flag, which is why it manifests itself for (libc++) vector and friends. The ObjC formatters don't set that flag so they should be safe. The libstdc++ formatters don't set it either. Furthermore there doesn't seem to be a way to set this flag by the user, so it is not possible to create a libc++-independent reproducer (which is what I was going to suggest).

I've been looking for a better way to fix this today (sorry about the delay), but I haven't figured out anything better. There is this fundamental recursion between the pretty printer (which wants to dereference a value) and the dereferencing code (which wants to know if it has a pretty printer -- so it can avoid dereferencing). Just removing the HasSyntheticValue()fixed the bug for me -- but then we were able to "dereference" incomplete structs. It's possible we may be able to allow the dereference to succeed here, but then refuse to print the value somewhere down the line. I would like to hear what @jingham things about all this.

Thanks for the investigation!
Actually some libstdc++ formatters do also set FrontEndWantsDereference, so this is reproable with

$ cat /tmp/main.cc
#include <set>

void f(std::set<int>& v);

int main() {
        std::set<int> v;
        f(v);
}
$ cat /tmp/a.cc
#include <set>

void f(std::set<int>& v) {
        // break
}
$ bin/clang++ -g0 /tmp/main.cc -o /tmp/main.o  -c && bin/clang++ -g /tmp/a.cc -o /tmp/a.o  -c && bin/clang++ /tmp/a.o /tmp/main.o -o /tmp/a
$ bin/lldb /tmp/a -b -o 'br set -n f'  -o run

with and without -stdlib=libc++

maybe a more targeted solution would be to change the added condition to when FrontEndWantsDereference is set, but that's more annoying to thread through the code. I think this should be ok as a temporary workaround if we don't have a proper fix soonish?

[aeubanks]

would somebody be willing to lgtm the workaround while we investigate further since this is currently breaking a fairly typical debugging session?

[labath]

Okay, here goes nothing.

lldb/source/Core/ValueObject.cpp

if (child_compiler_type && child_byte_size) {
           child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
           child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
           language_flags);
     }
 
     // In case of incomplete child compiler type, use the pointee type and try
     // to recreate a new ValueObjectChild using it.
     if (!m_deref_valobj) {
-      if (HasSyntheticValue()) {
+      // FIXME(#59012): C++ stdlib formatters break with incomplete types (e.g.
+      // `std::vector<int> &`). Remove ObjC restriction once that's resolved.
+      if (Language::LanguageIsObjC(GetPreferredDisplayLanguage()) &&
+          HasSyntheticValue()) {
         child_compiler_type = compiler_type.GetPointeeType();
 
         if (child_compiler_type) {
           ConstString child_name;
           if (!child_name_str.empty())
             child_name.SetCString(child_name_str.c_str());
 
           m_deref_valobj = new ValueObjectChild(
               *this, child_compiler_type, child_name, child_byte_size,

lldb/test/API/lang/cpp/incomplete-stl-types/Makefile

+CXX_SOURCES := main.cpp f.cpp
+
+include Makefile.rules
+
+# Force main.cpp to be built with no debug information
+main.o: CFLAGS = $(CFLAGS_NO_DEBUG)
+
+# And force -flimit-debug-info on the rest.
+f.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS)

lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py

+"""
+Test situations where the debug info only has a declaration, no definition, for
+an STL container with a formatter.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestStlIncompleteTypes(TestBase):
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("f.cpp"))
+
+        var = self.frame().GetValueForVariablePath("v")
+        self.assertIn("set", var.GetDisplayTypeName())

lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp

+#include <set>
+
+void f(std::set<int> &v) {
+  // break here
+}

lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp

+#include <set>
+
+void f(std::set<int> &v);
+
+int main() {
+  std::set<int> v;
+  f(v);
+}