[lldb] Let 'v' command directly access ivars of _any_ self/this

[kastiglione]

Summary

The v (frame variable) command can access ivars/fields of this or self. This
change relaxes the criteria for finding this/self.

There are cases where a this/self variable does exist, but are cases in which v did not
support direct access of ivars -- meaning the user would have to type this->field or
self->_ivar to access. This change allows those cases to also work without explicitly
dereferencing this/self.

For example:

__weak Type *weakSelf = self;
void (^block)() = ^{
   Type *self = weakSelf; // Re-establish strong reference.
   // ...
};

In this case, self exists but v wouldn't use it.

[kastiglione]

Add tests; rebase

[aprantl]

From an end-user perspective, I like this.

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

9819

Couple of questions for my understanding:

1. Do you see any opportunities for this to spectacularly do the wrong thing in an ObjectiveC++ program by guessing the wrong language? I suppose this just tries to do a best effort?

2. We're not using the DW_AT_APPLE_runtime_language attribute because it's only attached to Objective-C classes?

3. We're not using the DW_AT_language of the DW_TAG_compile_unit because it won't help us in an ObjC++ program?

[kastiglione]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

9819

We're not using the DW_AT_APPLE_runtime_language attribute because it's only attached to Objective-C classes?
We're not using the DW_AT_language of the DW_TAG_compile_unit because it won't help us in an ObjC++ program?

I am not sure how to answer these too. While this function is new, the logic and this code path which calls GetObjectPtrLanguage, is taken from the existing implementation. In other words, how the debug info is made use of, hasn't changed.

[kastiglione]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

9819

Do you see any opportunities for this to spectacularly do the wrong thing in an ObjectiveC++ program by guessing the wrong language? I suppose this just tries to do a best effort?

I don't see any obvious ways to get it wrong, but can't rule it out entirely. The way this change is implemented, there should be no way to have a regression – everything that used to work should still work. There could be some case out there where, in objc++, the wrong choice is made between this/self, but that would result in the same "variable not found" message you'd get today in that case (since the implementation should have no regressions).

[aprantl]

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

9819

Sorry I didn't realize this was existing code! While there might be an opportunity the improve the existing implementation, that is orthogonal to this change. Carry on.

lldb/include/lldb/Symbol/CompilerDeclContext.h

bool IsValid() const {
     return m_type_system != nullptr && m_opaque_decl_ctx != nullptr;
   }
 
   std::vector<CompilerDecl> FindDeclByName(ConstString name,
                                            const bool ignore_using_decls);
 
   /// Checks if this decl context represents a method of a class.
   ///
-  /// \param[out] language_object_name_ptr
-  ///     If non NULL and \b true is returned from this function,
-  ///     this will indicate if implicit object name for the language
-  ///     like "this" for C++, and "self" for Objective C.
-  ///
   /// \return
   ///     Returns true if this is a decl context that represents a method
   ///     in a struct, union or class.
-  bool IsClassMethod(ConstString *language_object_name_ptr = nullptr);
+  bool IsClassMethod();
+
+  /// Determines the original language of the decl context.
+  lldb::LanguageType GetLanguage();
+
+  /// Determines the name of the instance variable for the this decl context.
+  ///
+  /// For C++ the name is "this", for Objective-C the name is "self".
+  ///
+  /// \return
+  ///     Returns a string for the name of the instance variable.
+  ConstString GetInstanceVariableName(lldb::LanguageType language);
 
   /// Check if the given other decl context is contained in the lookup
   /// of this decl context (for example because the other context is a nested
   /// inline namespace).
   ///
   /// @param[in] other
   ///     The other decl context for which we should check if it is contained
   ///     in the lookoup of this context.

lldb/include/lldb/Symbol/SymbolContext.h

public:
   /// represented by this symbol context (whether the function is an inline
   /// function or not).
   ///
   /// \return
   ///     The block object pointer that defines the function that is
   ///     represented by this symbol context object, nullptr otherwise.
   Block *GetFunctionBlock();
 
-  /// If this symbol context represents a function that is a method, return
-  /// true and provide information about the method.
+  /// Determines the name of the instance variable for the this decl context.
   ///
-  /// \param[out] language_object_name
-  ///     If \b true is returned, the name of the artificial variable
-  ///     for the language ("this" for C++, "self" for ObjC).
+  /// For C++ the name is "this", for Objective-C the name is "self".
   ///
   /// \return
-  ///     \b True if this symbol context represents a function that
-  ///     is a method of a class, \b false otherwise.
-  bool GetFunctionMethodInfo(ConstString &language_object_name);
+  ///     Returns a string for the name of the instance variable.
+  ConstString GetInstanceVariableName();
 
   /// Sorts the types in TypeMap according to SymbolContext to TypeList
   ///
   void SortTypeList(TypeMap &type_map, TypeList &type_list) const;
 
   /// Find a name of the innermost function for the symbol context.
   ///
   /// For instance, if the symbol context contains an inlined block, it will

lldb/include/lldb/Symbol/TypeSystem.h

public:
   DeclContextFindDeclByName(void *opaque_decl_ctx, ConstString name,
                             const bool ignore_imported_decls);
 
   virtual ConstString DeclContextGetName(void *opaque_decl_ctx) = 0;
 
   virtual ConstString
   DeclContextGetScopeQualifiedName(void *opaque_decl_ctx) = 0;
 
-  virtual bool
-  DeclContextIsClassMethod(void *opaque_decl_ctx,
-                           ConstString *language_object_name_ptr) = 0;
+  virtual bool DeclContextIsClassMethod(void *opaque_decl_ctx) = 0;
 
   virtual bool DeclContextIsContainedInLookup(void *opaque_decl_ctx,
                                               void *other_opaque_decl_ctx) = 0;
 
+  virtual lldb::LanguageType DeclContextGetLanguage(void *opaque_decl_ctx) = 0;
+
   // Tests
 #ifndef NDEBUG
   /// Verify the integrity of the type to catch CompilerTypes that mix
   /// and match invalid TypeSystem/Opaque type pairs.
   virtual bool Verify(lldb::opaque_compiler_type_t type) = 0;
 #endif
 
   virtual bool IsArrayType(lldb::opaque_compiler_type_t type,
#endif
 
   virtual bool IsVoidType(lldb::opaque_compiler_type_t type) = 0;
 
   virtual bool CanPassInRegisters(const CompilerType &type) = 0;
 
   // TypeSystems can support more than one language
   virtual bool SupportsLanguage(lldb::LanguageType language) = 0;
 
+  /// The name of the variable used for explicitly accessing data scoped to the
+  /// current instance (or type). C++ uses "this", ObjC uses "self".
+  virtual ConstString GetInstanceVariableName(lldb::LanguageType language) = 0;
+
   // Type Completion
 
   virtual bool GetCompleteType(lldb::opaque_compiler_type_t type) = 0;
 
   virtual bool IsForcefullyCompleted(lldb::opaque_compiler_type_t type) {
     return false;
   }
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

public:
   std::vector<CompilerDecl>
   DeclContextFindDeclByName(void *opaque_decl_ctx, ConstString name,
                             const bool ignore_using_decls) override;
 
   ConstString DeclContextGetName(void *opaque_decl_ctx) override;
 
   ConstString DeclContextGetScopeQualifiedName(void *opaque_decl_ctx) override;
 
-  bool DeclContextIsClassMethod(void *opaque_decl_ctx,
-                                ConstString *language_object_name_ptr) override;
+  bool DeclContextIsClassMethod(void *opaque_decl_ctx) override;
 
   bool DeclContextIsContainedInLookup(void *opaque_decl_ctx,
                                       void *other_opaque_decl_ctx) override;
 
+  lldb::LanguageType DeclContextGetLanguage(void *opaque_decl_ctx) override;
+
   // Clang specific clang::DeclContext functions
 
   static clang::DeclContext *
   DeclContextGetAsDeclContext(const CompilerDeclContext &dc);
 
   static clang::ObjCMethodDecl *
   DeclContextGetAsObjCMethodDecl(const CompilerDeclContext &dc);
 
#endif
   bool IsTypedefType(lldb::opaque_compiler_type_t type) override;
 
   bool IsVoidType(lldb::opaque_compiler_type_t type) override;
 
   bool CanPassInRegisters(const CompilerType &type) override;
 
   bool SupportsLanguage(lldb::LanguageType language) override;
 
+  ConstString GetInstanceVariableName(lldb::LanguageType language) override;
+
   static std::optional<std::string> GetCXXClassName(const CompilerType &type);
 
   // Type Completion
 
   bool GetCompleteType(lldb::opaque_compiler_type_t type) override;
 
   bool IsForcefullyCompleted(lldb::opaque_compiler_type_t type) override;
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

 //===-- TypeSystemClang.cpp -----------------------------------------------==='//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TypeSystemClang.h"
 
+#include "clang/AST/DeclBase.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/FormatVariadic.h"
 
 #include <mutex>
 #include <memory>
 #include <string>
 #include <vector>
 
bool TypeSystemClang::CanPassInRegisters(const CompilerType &type) {
   }
   return false;
 }
 
 bool TypeSystemClang::SupportsLanguage(lldb::LanguageType language) {
   return TypeSystemClangSupportsLanguage(language);
 }
 
+ConstString
+TypeSystemClang::GetInstanceVariableName(lldb::LanguageType language) {
+  switch (language) {
+  case LanguageType::eLanguageTypeC_plus_plus:
+  case LanguageType::eLanguageTypeC_plus_plus_03:
+  case LanguageType::eLanguageTypeC_plus_plus_11:
+  case LanguageType::eLanguageTypeC_plus_plus_14:
+    return ConstString("this");
+  case LanguageType::eLanguageTypeObjC:
+  case LanguageType::eLanguageTypeObjC_plus_plus:
+    return ConstString("self");
+  default:
+    return {};
+  }
+}
+
 std::optional<std::string>
 TypeSystemClang::GetCXXClassName(const CompilerType &type) {
   if (!type)
     return std::nullopt;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
   if (qual_type.isNull())
     return std::nullopt;
if (opaque_decl_ctx) {
     clang::NamedDecl *named_decl =
         llvm::dyn_cast<clang::NamedDecl>((clang::DeclContext *)opaque_decl_ctx);
     if (named_decl)
       return ConstString(GetTypeNameForDecl(named_decl));
   }
   return ConstString();
 }
 
-bool TypeSystemClang::DeclContextIsClassMethod(
-    void *opaque_decl_ctx, ConstString *language_object_name_ptr) {
-  if (opaque_decl_ctx) {
-    clang::DeclContext *decl_ctx = (clang::DeclContext *)opaque_decl_ctx;
-    if (ObjCMethodDecl *objc_method =
-            llvm::dyn_cast<clang::ObjCMethodDecl>(decl_ctx)) {
-      if (objc_method->isInstanceMethod())
-        if (language_object_name_ptr)
-          language_object_name_ptr->SetCString("self");
-      return true;
-    } else if (CXXMethodDecl *cxx_method =
-                   llvm::dyn_cast<clang::CXXMethodDecl>(decl_ctx)) {
-      if (cxx_method->isInstance())
-        if (language_object_name_ptr)
-          language_object_name_ptr->SetCString("this");
-      return true;
-    } else if (clang::FunctionDecl *function_decl =
-                   llvm::dyn_cast<clang::FunctionDecl>(decl_ctx)) {
-      ClangASTMetadata *metadata = GetMetadata(function_decl);
-      if (metadata && metadata->HasObjectPtr()) {
-        if (language_object_name_ptr)
-          language_object_name_ptr->SetCString(metadata->GetObjectPtrName());
-        return true;
-      }
-    }
-  }
+bool TypeSystemClang::DeclContextIsClassMethod(void *opaque_decl_ctx) {
+  if (!opaque_decl_ctx)
+    return false;
+
+  if (llvm::isa<clang::ObjCMethodDecl>(decl_ctx)) {
+  } else if (llvm::isa<clang::CXXMethodDecl>(decl_ctx)) {
+  } else if (clang::FunctionDecl *fun_decl =
+                 llvm::dyn_cast<clang::FunctionDecl>(decl_ctx)) {
+    if (ClangASTMetadata *metadata = GetMetadata(fun_decl))
+      return metadata->HasObjectPtr();
+
   return false;
 }
 
 bool TypeSystemClang::DeclContextIsContainedInLookup(
     void *opaque_decl_ctx, void *other_opaque_decl_ctx) {
   auto *decl_ctx = (clang::DeclContext *)opaque_decl_ctx;
   auto *other = (clang::DeclContext *)other_opaque_decl_ctx;
 
   do {
     // A decl context always includes its own contents in its lookup.
     if (decl_ctx == other)
       return true;
 
     // If we have an inline namespace, then the lookup of the parent context
     // also includes the inline namespace contents.
   } while (other->isInlineNamespace() && (other = other->getParent()));
 
   return false;
 }
 
+lldb::LanguageType
+TypeSystemClang::DeclContextGetLanguage(void *opaque_decl_ctx) {
+  if (!opaque_decl_ctx)
+    return eLanguageTypeUnknown;
+
+  auto *decl_ctx = (clang::DeclContext *)opaque_decl_ctx;
+  if (llvm::isa<clang::ObjCMethodDecl>(decl_ctx)) {
+    return eLanguageTypeObjC;
+  } else if (llvm::isa<clang::CXXMethodDecl>(decl_ctx)) {
+    return eLanguageTypeC_plus_plus;
+  } else if (auto *fun_decl = llvm::dyn_cast<clang::FunctionDecl>(decl_ctx)) {
+    if (ClangASTMetadata *metadata = GetMetadata(fun_decl))
+      return metadata->GetObjectPtrLanguage();
+  }
+
+  return eLanguageTypeUnknown;
+}
+
 static bool IsClangDeclContext(const CompilerDeclContext &dc) {
   return dc.IsValid() && isa<TypeSystemClang>(dc.GetTypeSystem());
 }
 
 clang::DeclContext *
 TypeSystemClang::DeclContextGetAsDeclContext(const CompilerDeclContext &dc) {
   if (IsClangDeclContext(dc))
     return (clang::DeclContext *)dc.GetOpaqueDeclContext();

lldb/source/Symbol/CompilerDeclContext.cpp

 }
 
 ConstString CompilerDeclContext::GetScopeQualifiedName() const {
   if (IsValid())
     return m_type_system->DeclContextGetScopeQualifiedName(m_opaque_decl_ctx);
   return ConstString();
 }
 
-bool CompilerDeclContext::IsClassMethod(ConstString *language_object_name_ptr) {
+bool CompilerDeclContext::IsClassMethod() {
   if (IsValid())
-    return m_type_system->DeclContextIsClassMethod(m_opaque_decl_ctx,
-                                                   language_object_name_ptr);
+    return m_type_system->DeclContextIsClassMethod(m_opaque_decl_ctx);
   return false;
 }
 
+lldb::LanguageType CompilerDeclContext::GetLanguage() {
+  if (IsValid())
+    return m_type_system->DeclContextGetLanguage(m_opaque_decl_ctx);
+  return {};
+}
+
+ConstString
+CompilerDeclContext::GetInstanceVariableName(lldb::LanguageType language) {
+  if (IsValid())
+    return m_type_system->GetInstanceVariableName(language);
+  return {};
+}
+
 bool CompilerDeclContext::IsContainedInLookup(CompilerDeclContext other) const {
   if (!IsValid())
     return false;
 
   // If the other context is just the current context, we don't need to go
   // over the type system to know that the lookup is identical.
   if (this == &other)
     return true;

lldb/source/Symbol/SymbolContext.cpp

if (function) {
     // There is no block information in this symbol context, so we must assume
     // that the block that is desired is the top level block of the function
     // itself.
     return &function->GetBlock(true);
   }
   return nullptr;
 }
 
-bool SymbolContext::GetFunctionMethodInfo(ConstString &language_object_name) {
-  Block *function_block = GetFunctionBlock();
-  if (function_block) {
-    CompilerDeclContext decl_ctx = function_block->GetDeclContext();
-    if (decl_ctx)
-      return decl_ctx.IsClassMethod(&language_object_name);
+ConstString SymbolContext::GetInstanceVariableName() {
+  if (Block *function_block = GetFunctionBlock())
+    if (CompilerDeclContext decl_ctx = function_block->GetDeclContext()) {
+      auto language = decl_ctx.GetLanguage();
+      if (language == eLanguageTypeUnknown)
+        language = GetLanguage();
+      return decl_ctx.GetInstanceVariableName(language);
     }
-  return false;
+
+  return {};
 }
 
 void SymbolContext::SortTypeList(TypeMap &type_map, TypeList &type_list) const {
   Block *curr_block = block;
   bool isInlinedblock = false;
   if (curr_block != nullptr &&
       curr_block->GetContainingInlinedBlock() != nullptr)
     isInlinedblock = true;

lldb/source/Target/StackFrame.cpp

ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
   if (var_sp) {
     var_expr = var_expr.drop_front(name_const_string.GetLength());
   }
 
   if (!var_sp && (options & eExpressionPathOptionsAllowDirectIVarAccess)) {
     // Check for direct ivars access which helps us with implicit access to
     // ivars using "this" or "self".
     GetSymbolContext(eSymbolContextFunction | eSymbolContextBlock);
-    ConstString method_object_name;
-    if (m_sc.GetFunctionMethodInfo(method_object_name)) {
-      if (method_object_name) {
-        var_sp = variable_list->FindVariable(method_object_name);
-        if (var_sp) {
-          separator_idx = 0;
-          if (Type *var_type = var_sp->GetType())
-            if (auto compiler_type = var_type->GetForwardCompilerType())
-              if (!compiler_type.IsPointerType())
-                var_expr_storage = ".";
+    if (auto instance_var_name = m_sc.GetInstanceVariableName()) {
+      var_sp = variable_list->FindVariable(instance_var_name);
 
-          if (var_expr_storage.empty())
-            var_expr_storage = "->";
-          var_expr_storage += var_expr;
-          var_expr = var_expr_storage;
-          synthetically_added_instance_object = true;
-        }
-      }
-    }
-  }
 
   if (!var_sp && (options & eExpressionPathOptionsInspectAnonymousUnions)) {
     // Check if any anonymous unions are there which contain a variable with
     // the name we need
     for (const VariableSP &variable_sp : *variable_list) {
       if (!variable_sp)
         continue;
       if (!variable_sp->GetName().IsEmpty())

lldb/test/API/commands/frame/var/direct-ivar/objc/Makefile

 OBJC_SOURCES := main.m
+CFLAGS_EXTRAS := -fblocks -fobjc-arc
+LD_EXTRAS := -lobjc
 include Makefile.rules

lldb/test/API/commands/frame/var/direct-ivar/objc/TestFrameVarDirectIvarObjC.py

 import lldb
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
 from lldbsuite.test import lldbutil
 
 
 class TestCase(TestBase):
     @skipUnlessDarwin
     def test_objc_self(self):
         self.build()
         lldbutil.run_to_source_breakpoint(self, "// check self", lldb.SBFileSpec("main.m"))
         self.expect("frame variable _ivar", startstr="(int) _ivar = 30")
+
+    @skipUnlessDarwin
+    def test_objc_self_capture_idiom(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// check idiomatic self", lldb.SBFileSpec("main.m"))
+        self.expect("frame variable weakSelf", startstr="(Classic *) weakSelf = 0x")
+        self.expect("frame variable self", startstr="(Classic *) self = 0x")
+        self.expect("frame variable _ivar", startstr="(int) _ivar = 30")

lldb/test/API/commands/frame/var/direct-ivar/objc/main.m

 #include <objc/NSObject.h>
 
 @interface Classic : NSObject {
 @public
   int _ivar;
 }
 @end
 
 @implementation Classic
-- (int)fun {
+- (void)fun {
   // check self
 }
+
+- (void)run {
+  __weak Classic *weakSelf = self;
+  ^{
+    Classic *self = weakSelf;
+    // check idiomatic self
+
+    // Use `self` to extend its lifetime (for lldb to inspect the variable).
+    [self copy];
+  }();
+}
 @end
 
 int main() {
   Classic *c = [Classic new];
   c->_ivar = 30;
   [c fun];
+  [c run];
 }