Add libc++ data formatter for std::function

[shafik]

Summary

Adding formatter summary for std::function.

- Added LibcxxFunctionSummaryProvider
- Removed LibcxxFunctionFrontEnd
- Modified data formatter tests to test new summary functionality

[vsk]

source/Plugins/Language/CPlusPlus/LibCxx.cpp

46 ↗

(On Diff #161108)

'or' -> 'or hold a'
'will hold' -> 'points to'

47 ↗

(On Diff #161108)

'need' -> 'needed'

49 ↗

(On Diff #161108)

Could you explicitly pass in a DynamicValueType into GetChildMemberWithName instead of true? You should be able to find one from valobj->GetManager()->GetUseDynamic().

55 ↗

(On Diff #161108)

The address size is a generically useful thing to have around, there's no need to comment about defining this.

58 ↗

(On Diff #161108)

Please use an early exit here, and everywhere else in the function where it's possible to do so. If the process has really disappeared, there's no need to print anything. In general there's no need to present an incomplete result from this formatter.

71 ↗

(On Diff #161108)

When you say "possible", what do you mean? Could you add a comment?

72 ↗

(On Diff #161108)

Do you need to check status.Fail() here?

76 ↗

(On Diff #161108)

Before doing anything related to SectionLoadList, it would help to have a short comment explaining the intent of the code. Looking through it, it seems like it'd be even better if split out into a helper function.

80 ↗

(On Diff #161108)

Instead of attempting to parse symbol names, which is inherently brittle and complicated, why not get the base address of the callable and look up it's location via the symbol table? You'd lose some precision in being able to disambiguate lambdas vs. regular functions, but that's a great tradeoff to make, because the code would be simpler and more reliable.

[jingham]

For the most part this is fine. There are two bits to work on:

1. I think you could do all of this work on a static std::function object from the data section even if you don't have a process. It might be worth seeing whether you can do that. It looks like you can make static std::function objects...
2. I think this code would be a little easier to understand if you first stated clearly the kinds of std::function variants that you've discovered at the beginning of the provider. Then when you get to the section of code that handles each of the cases, start the code with: "This covers the case...". Be kind to two years in the future you when you will have forgotten how all this stuff works...

source/Plugins/Language/CPlusPlus/LibCxx.cpp

58 ↗

(On Diff #161108)

Data formatters can get called in the absence of a process. For simple C structs, for instance, they might be used to look at an initialized static object in a DATA section. For instance, I could do something like:

#include <functional>
auto g_func = [](int x, int y) {return x + y; };
int
main(int argc, char ** argv)
{

int ret = g_func(argc, 100);
return ret;

}

and I should be able to see g_func using "target var" before running the program. I think the only place you care about the process here is whether you should resolve a load address or not. Target->ReadMemory will read from a running process if that target has one (but preferring data from the underlying files if that's faster.) So I think this formatter should be able to do its job with or without a process.

But I agree with Vedant that in cases where you can't get useful information (for instance if you can't get the vtable address), then you should return false immediately and not try to present a summary. That way instead of showing an incomplete summary, lldb will show the raw printing of the type, which is more useful than a place-holder summary string.

You already do that for the case where you can't read the process memory. Using early return when you discover an error will make it easier to ensure you are doing that right every time you get an error.

80 ↗

(On Diff #161108)

I'm not sure I agree. There were some cases where the only way Shafik could find the target was looking at the demangled name. There's nothing here that's so complex that it isn't worth doing, and we can test the cases to ensure that it keeps working. The alternative is leaving folks with no idea what their std::function holds and no way of knowing why this works sometimes but not others.

129 ↗

(On Diff #161108)

This ";" is not doing anything, is it?

136–140 ↗

(On Diff #161108)

In the first bit of code, you put the explaining comment before the code that handles the condition. This time it's in the middle of the code. Maybe move this comment before "if (symbol_name.contains(...)"

202–204 ↗

(On Diff #161108)

I don't think this is valuable. You are just reporting the value of __f_ here? If that's all you can do, I think it's better to return false indicating that you really couldn't make sense of this object, and showing the raw value.

[jingham]

Sounds like std::function objects need to run static initializers before they are useful. So trying to format them w/o a process won't do any good. Probably better to just return false directly if you don't have a process in that case.

[shafik]

Fixes based on comments

• Switched to early exits
• Added a lot more comments to explain all the cases being dealt with and noting which cases different sections are dealing with

[shafik]

@jingham @vsk I believe I have addressed most of your comments

source/Plugins/Language/CPlusPlus/LibCxx.cpp

58 ↗

(On Diff #161108)

AFAIT we can't derive useful information for the static case.

std::function does not have constexpr constructors so it can't be initialized during constant initialization. I confirmed in lldb that all the fields are uninitialized therefore we have no pointers to examine.

136–140 ↗

(On Diff #161108)

I updated the commenting a big, hopefully it clears things up.

[jingham]

This looks fine. There was one place where you referred back to case 1 by name not ordinal, but otherwise this is quite clear. Don't need another round of review, just fix that nit and its good to go.

[shafik]

Updating comment

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py

def test(self):
 
         self.runCmd("run", RUN_SUCCEEDED)
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'stop reason = breakpoint'])
 
-        f1 = self.get_variable('f1')
-        f2 = self.get_variable('f2')
+        self.expect("frame variable f1",
+                    substrs=['f1 =  Function = foo(int, int)'])
 
-        if self.TraceOn():
-            print(f1)
-        if self.TraceOn():
-            print(f2)
+        self.expect("frame variable f2",
+                    substrs=['f2 =  Lambda in File main.cpp at Line 27'])
 
-        self.assertTrue(f1.GetValueAsUnsigned(0) != 0, 'f1 has a valid value')
-        self.assertTrue(f2.GetValueAsUnsigned(0) != 0, 'f2 has a valid value')
+        self.expect("frame variable f3",
+                    substrs=['f3 =  Lambda in File main.cpp at Line 31'])
+
+        self.expect("frame variable f4",
+                    substrs=['f4 =  Function in File main.cpp at Line 17'])
+
+        self.expect("frame variable f5",
+                    substrs=['f5 =  Function = Bar::add_num(int) const'])

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp

 //===-- main.cpp --------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include <functional>
 
 int foo(int x, int y) {
   return x + y - 1;
 }
 
-int main ()
+struct Bar {
+   int operator()() {
+       return 66 ;
+   }
+   int add_num(int i) const { return i + 3 ; }
+} ;
+
+int main (int argc, char *argv[])
 {
   int acc = 42;
   std::function<int (int,int)> f1 = foo;
   std::function<int (int)> f2 = [acc,f1] (int x) -> int {
     return x+f1(acc,x);
   };
-    return f1(acc,acc) + f2(acc); // Set break point at this line.
-}
 
+  auto f = [](int x, int y) { return x + y; };
+  auto g = [](int x, int y) { return x * y; } ;
+  std::function<int (int,int)> f3 =  argc %2 ? f : g ;
+
+  Bar bar1 ;
+  std::function<int ()> f4( bar1 ) ;
+  std::function<int (const Bar&, int)> f5 = &Bar::add_num;
+
+  return f1(acc,acc) + f2(acc) + f3(acc+1,acc+2) + f4() + f5(bar1, 10); // Set break point at this line.
+}

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

AddCXXSynthetic(
       stl_synth_flags, true);
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEndCreator,
       "weak_ptr synthetic children",
       ConstString("^(std::__(ndk)?1::)weak_ptr<.+>(( )?&)?$"), stl_synth_flags,
       true);
 
+  AddCXXSummary(
+      cpp_category_sp, lldb_private::formatters::LibcxxFunctionSummaryProvider,
+      "libc++ std::function summary provider",
+      ConstString("^std::__(ndk)?1::function<.+>$"), stl_summary_flags, true);
+
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibcxxContainerSummaryProvider,
                 "libc++ std::bitset summary provider",
                 ConstString("^std::__(ndk)?1::bitset<.+>(( )?&)?$"),
                 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
AddCXXSynthetic(
       ConstString("^std::__(ndk)?1::__wrap_iter<.+>$"), stl_synth_flags, true);
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEndCreator,
       "std::map iterator synthetic children",
       ConstString("^std::__(ndk)?1::__map_iterator<.+>$"), stl_synth_flags,
       true);
-
-  AddCXXSynthetic(
-      cpp_category_sp, lldb_private::formatters::LibcxxFunctionFrontEndCreator,
-      "std::function synthetic value provider",
-      ConstString("^std::__(ndk)?1::function<.+>$"), stl_synth_flags, true);
 #endif
 }
 
 static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
     return;
 
   TypeSummaryImpl::Flags stl_summary_flags;

lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h

bool LibcxxOptionalSummaryProvider(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &options); // libc++ std::optional<>
 
 bool LibcxxSmartPointerSummaryProvider(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions
         &options); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
+bool LibcxxFunctionSummaryProvider(
+    ValueObject &valobj, Stream &stream,
+    const TypeSummaryOptions &options); // libc++ std::function<>
+
 SyntheticChildrenFrontEnd *
 LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                          lldb::ValueObjectSP);
 
 bool LibcxxContainerSummaryProvider(ValueObject &valobj, Stream &stream,
                                     const TypeSummaryOptions &options);
 
 class LibCxxMapIteratorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {

 SyntheticChildrenFrontEnd *
 LibcxxStdUnorderedMapSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                               lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
 LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                               lldb::ValueObjectSP);
 
-SyntheticChildrenFrontEnd *LibcxxFunctionFrontEndCreator(CXXSyntheticChildren *,
-                                                         lldb::ValueObjectSP);
-
 SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *,
                                                       lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
                                                       lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
 LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
                               lldb::ValueObjectSP valobj_sp);
 
 } // namespace formatters
 } // namespace lldb_private
 
 #endif // liblldb_LibCxx_h_

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

 //===-- LibCxx.cpp ----------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "LibCxx.h"
 
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "llvm/ADT/ScopeExit.h"
 // Project includes
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/DataFormatters/VectorIterator.h"
 #include "lldb/Symbol/ClangASTContext.h"
 #include "lldb/Target/ProcessStructReader.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
 using namespace lldb_private;
bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
   llvm::StringRef engaged_as_cstring(
       engaged_sp->GetValueAsUnsigned(0) == 1 ? "true" : "false");
 
   stream.Printf(" Has Value=%s ", engaged_as_cstring.data());
 
   return true;
 }
 
+bool lldb_private::formatters::LibcxxFunctionSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+
+  if (!valobj_sp)
+    return false;
+
+  // Member __f_ has type __base*, the contents of which will hold:
+  // 1) a vtable entry which may hold type information needed to discover the
+  //    lambda being called
+  // 2) possibly hold a pointer to the callable object
+  // e.g.
+  //
+  // (lldb) frame var -R  f_display
+  // (std::__1::function<void (int)>) f_display = {
+  //  __buf_ = {
+  //  …
+  // }
+  //  __f_ = 0x00007ffeefbffa00
+  // }
+  // (lldb) memory read -fA 0x00007ffeefbffa00
+  // 0x7ffeefbffa00: ... `vtable for std::__1::__function::__func<void (*) ...
+  // 0x7ffeefbffa08: ... `print_num(int) at std_function_cppreference_exam ...
+  //
+  // We will be handling five cases below, std::function is wrapping:
+  //
+  // 1) a lambda we know at compile time. We will obtain the name of the lambda
+  //    from the first template pameter from __func's vtable. We will look up
+  //    the lambda's operator()() and obtain the line table entry.
+  // 2) a lambda we know at runtime. A pointer to the lambdas __invoke method
+  //    will be stored after the vtable. We will obtain the lambdas name from
+  //    this entry and lookup operator()() and obtain the line table entry.
+  // 3) a callable object via operator()(). We will obtain the name of the
+  //    object from the first template parameter from __func's vtable. We will
+  //    look up the objectc operator()() and obtain the line table entry.
+  // 4) a member function. A pointer to the function will stored after the
+  //    we will obtain the name from this pointer.
+  // 5) a free function. A pointer to the function will stored after the vtable
+  //    we will obtain the name from this pointer.
+  ValueObjectSP member__f_(
+      valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
+  lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
+
+  ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+
+  if (process == nullptr)
+    return false;
+
+  uint32_t address_size = process->GetAddressByteSize();
+  Status status;
+
+  // First item pointed to by __f_ should be the pointer to the vtable for
+  // a __base object.
+  lldb::addr_t vtable_address =
+      process->ReadPointerFromMemory(member__f_pointer_value, status);
+
+  if (status.Fail())
+    return false;
+
+  bool found_wrapped_function = false;
+
+  // Using scoped exit so we can use early return and still execute the default
+  // action in case we don't find the wrapper function. Otherwise we can't use
+  // early exit without duplicating code.
+  auto default_print_on_exit = llvm::make_scope_exit(
+      [&found_wrapped_function, &stream, &member__f_pointer_value]() {
+        if (!found_wrapped_function)
+          stream.Printf(" __f_ = %llu", member__f_pointer_value);
+      });
+
+  lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+  // As commened above we may not have a function pointer but if we do we will
+  // need it.
+  lldb::addr_t possible_function_address =
+      process->ReadPointerFromMemory(address_after_vtable, status);
+
+  if (status.Fail())
+    return false;
+
+  Target &target = process->GetTarget();
+
+  if (target.GetSectionLoadList().IsEmpty())
+    return false;
+
+  Address vtable_addr_resolved;
+  SymbolContext sc;
+  Symbol *symbol;
+
+  if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
+                                                      vtable_addr_resolved))
+    return false;
+
+  target.GetImages().ResolveSymbolContextForAddress(
+      vtable_addr_resolved, eSymbolContextEverything, sc);
+  symbol = sc.symbol;
+
+  if (symbol == NULL)
+    return false;
+
+  llvm::StringRef vtable_name(symbol->GetName().GetCString());
+  bool found_expected_start_string =
+      vtable_name.startswith("vtable for std::__1::__function::__func<");
+
+  if (!found_expected_start_string)
+    return false;
+
+  // Given case 1 or 3 we have a vtable name, we are want to extract the first
+  // template parameter
+  //
+  //  ... __func<main::$_0, std::__1::allocator<main::$_0> ...
+  //             ^^^^^^^^^
+  //
+  // We do this by find the first < and , and extracting in between.
+  //
+  // This covers the case of the lambda known at compile time.
+  //
+  size_t first_open_angle_bracket = vtable_name.find('<') + 1;
+  size_t first_comma = vtable_name.find_first_of(',');
+
+  llvm::StringRef first_template_parameter =
+      vtable_name.slice(first_open_angle_bracket, first_comma);
+
+  Address function_address_resolved;
+
+  // Setup for cases 2, 4 and 5 we have a pointer to a function after the
+  // vtable. We will use a process of elimination to drop through each case
+  // and obtain the data we need.
+  if (target.GetSectionLoadList().ResolveLoadAddress(
+          possible_function_address, function_address_resolved)) {
+    target.GetImages().ResolveSymbolContextForAddress(
+        function_address_resolved, eSymbolContextEverything, sc);
+    symbol = sc.symbol;
+  }
+
+  auto get_name = [&first_template_parameter, &symbol]() {
+    // Given case 1:
+    //
+    //    main::$_0
+    //
+    // we want to append ::operator()()
+    if (first_template_parameter.contains("$_"))
+      return llvm::Regex::escape(first_template_parameter.str()) +
+             R"(::operator\(\)\(.*\))";
+
+    if (symbol != NULL &&
+        symbol->GetName().GetStringRef().contains("__invoke")) {
+
+      llvm::StringRef symbol_name = symbol->GetName().GetStringRef();
+      size_t pos2 = symbol_name.find_last_of(':');
+
+      // Given case 2:
+      //
+      //    main::$_1::__invoke(...)
+      //
+      // We want to slice off __invoke(...) and append operator()()
+      std::string lambda_operator =
+          llvm::Regex::escape(symbol_name.slice(0, pos2 + 1).str()) +
+          R"(operator\(\)\(.*\))";
+
+      return lambda_operator;
+    }
+
+    // Case 3
+    return first_template_parameter.str() + R"(::operator\(\)\(.*\))";
+    ;
+  };
+
+  std::string func_to_match = get_name();
+
+  SymbolContextList scl;
+
+  target.GetImages().FindFunctions(RegularExpression{func_to_match}, true, true,
+                                   true, scl);
+
+  // Case 1,2 or 3
+  if (scl.GetSize() >= 1) {
+    SymbolContext sc2 = scl[0];
+
+    AddressRange range;
+    sc2.GetAddressRange(eSymbolContextEverything, 0, false, range);
+
+    Address address = range.GetBaseAddress();
+
+    Address addr;
+    if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
+                                  addr)) {
+      LineEntry line_entry;
+      addr.CalculateSymbolContextLineEntry(line_entry);
+
+      found_wrapped_function = true;
+      if (first_template_parameter.contains("$_") ||
+          (symbol != NULL &&
+           symbol->GetName().GetStringRef().contains("__invoke"))) {
+        // Case 1 and 2
+        stream.Printf(" Lambda in File %s at Line %u",
+                      line_entry.file.GetFilename().GetCString(),
+                      line_entry.line);
+      } else {
+        // Case 3
+        stream.Printf(" Function in File %s at Line %u",
+                      line_entry.file.GetFilename().GetCString(),
+                      line_entry.line);
+      }
+
+      return true;
+    }
+  }
+
+  // Case 4 or 5
+  if (!symbol->GetName().GetStringRef().startswith("vtable for")) {
+    found_wrapped_function = true;
+    stream.Printf(" Function = %s ", symbol->GetName().GetCString());
+    return true;
+  }
+
+  return false;
+}
+
 bool lldb_private::formatters::LibcxxSmartPointerSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
   if (!valobj_sp)
     return false;
   ValueObjectSP ptr_sp(
       valobj_sp->GetChildMemberWithName(ConstString("__ptr_"), true));
   ValueObjectSP count_sp(valobj_sp->GetChildAtNamePath(
bool lldb_private::formatters::LibcxxStringSummaryProvider(
   options.SetQuote('"');
   options.SetSourceSize(size);
   options.SetBinaryZeroIsTerminator(false);
   StringPrinter::ReadBufferAndDumpToStream<
       StringPrinter::StringElementType::ASCII>(options);
 
   return true;
 }
-
-class LibcxxFunctionFrontEnd : public SyntheticValueProviderFrontEnd {
-public:
-  LibcxxFunctionFrontEnd(ValueObject &backend)
-      : SyntheticValueProviderFrontEnd(backend) {}
-
-  lldb::ValueObjectSP GetSyntheticValue() override {
-    static ConstString g___f_("__f_");
-    return m_backend.GetChildMemberWithName(g___f_, true);
-  }
-};
-
-SyntheticChildrenFrontEnd *
-lldb_private::formatters::LibcxxFunctionFrontEndCreator(
-    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
-  if (valobj_sp)
-    return new LibcxxFunctionFrontEnd(*valobj_sp);
-  return nullptr;
-}