Inhibit global lookups for symbols in the IR dynamic checks

[spyffe]

Summary

The IR dynamic checks are self-contained functions whose job is to

• verify that pointers referenced in an expression are valid at runtime; and
• verify that selectors sent to Objective-C objects by an expression are actually supported by that object.

These dynamic checks forward-declare all the functions they use and should not require any external debug information. The way they ensure this is by marking all the names they use with a dollar sign ($). The expression parser recognizes such symbols and perform no lookups for them.

This patch fixes three issues surrounding the use of the dollar sign:

• to fix a MIPS issue, the name of the pointer checker was changed from starting with $ to starting with _$, but this was not properly ignored; and
• the Objective-C object checker used a temporary variable that did not start with $.
• the Objective-C object checker used an externally-defined struct (struct objc_selector) but didn't need to.

The patch also reformats the string containing the Objective-C object checker, which was mangled horribly when the code was transformed to a uniform width of 80 columns.

[clayborg]

source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

651–652 ↗

(On Diff #116261)

Should we use a function or macro here? This code is duped below.

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

812–814 ↗

(On Diff #116261)

Should we use a function or macro here? This code is duped above

source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

815–831 ↗

(On Diff #116261)

Since you are modifying this string switch to R"( to make it much clearer:

R"(
extern "C" void *gdb_object_getClass(void *);
extern "C"  int printf(const char *format, ...);
extern "C" void
%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)
  if ($__lldb_arg_obj == (void *)0)
    return; // nil is ok
  if (!gdb_object_getClass($__lldb_arg_obj)) {
    *((volatile int *)0) = 'ocgc';
  } else if ($__lldb_arg_selector != (void *)0) {
    signed char $responds = (signed char)
        [(id)$__lldb_arg_obj respondsToSelector:
            (void *) $__lldb_arg_selector];
    if ($responds == (signed char) 0)
      *((volatile int *)0) = 'ocgc';
  }
})";

834–852 ↗

(On Diff #116261)

Use R"( as above.

[zturner]

source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

635–652 ↗

(On Diff #116261)

I might be missing something here, but how about:

std::string name = context.m_decl_name.getAsString();
if (name.empty() || name == "id" || name == "Class")
  return;
if (name.startswith("$") || name.startswith("_$"))
  return;

5 lines instead of ~20.

[spyffe]

Thank you. I am testing a new revision with your suggestions.

[spyffe]

Applied Greg's and Zachary's suggestions:

• Used R"(...)" for multiline strings.
• Used StringRefs appropriately.
• Factored out symbol-ignoring into a function shared between ClangASTSource and ClangExpressionDeclMap.

[clayborg]

Looks good.

[spyffe]

Committed as LLDB r314225

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h

protected:
   /// @param[in] src_type
   ///     The source type.
   ///
   /// @return
   ///     The imported type.
   //------------------------------------------------------------------
   CompilerType GuardedCopyType(const CompilerType &src_type);
 
+  
+  //------------------------------------------------------------------
+  /// Returns true if a name should be ignored by name lookup.
+  ///
+  /// @param[in] name
+  ///     The name to be considered.
+  ///
+  /// @param[in] ignore_all_dollar_nmmes
+  ///     True if $-names of all sorts should be ignored.
+  ///
+  /// @return
+  ///     True if the name is one of a class of names that are ignored by
+  ///     global lookup for performance reasons.
+  //------------------------------------------------------------------
+  bool IgnoreName(const ConstString name, bool ignore_all_dollar_names);
+
   friend struct NameSearchContext;
 
   bool m_import_in_progress;
   bool m_lookups_enabled;
 
   const lldb::TargetSP
       m_target; ///< The target to use in finding variables and types.
   clang::ASTContext

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

if (!context.m_namespace_map->empty()) {
     NamespaceDecl *clang_namespace_decl =
         AddNamespace(context, context.m_namespace_map);
 
     if (clang_namespace_decl)
       clang_namespace_decl->setHasExternalVisibleStorage();
   }
 }
 
+bool ClangASTSource::IgnoreName(const ConstString name,
+                                bool ignore_all_dollar_names) {
+  static const ConstString id_name("id");
+  static const ConstString Class_name("Class");
+
+  if (name == id_name || name == Class_name)
+    return true;
+
+  StringRef name_string_ref = name.GetStringRef();
+
+  // The ClangASTSource is not responsible for finding $-names.
+  if (name_string_ref.empty() ||
+      (ignore_all_dollar_names && name_string_ref.startswith("$")) ||
+      name_string_ref.startswith("_$"))
+    return true;
+
+  return false;
+}
+
 void ClangASTSource::FindExternalVisibleDecls(
     NameSearchContext &context, lldb::ModuleSP module_sp,
     CompilerDeclContext &namespace_decl, unsigned int current_id) {
   assert(m_ast_context);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   SymbolContextList sc_list;
 
   const ConstString name(context.m_decl_name.getAsString().c_str());
-
-  const char *name_unique_cstr = name.GetCString();
-
-  static ConstString id_name("id");
-  static ConstString Class_name("Class");
-
-  if (name == id_name || name == Class_name)
-    return;
-
-  if (name_unique_cstr == NULL)
-    return;
-
-  // The ClangASTSource is not responsible for finding $-names.
-  if (name_unique_cstr[0] == '$')
+  if (IgnoreName(name, true))
     return;
 
   if (module_sp && namespace_decl) {
     CompilerDeclContext found_namespace_decl;
 
     SymbolVendor *symbol_vendor = module_sp->GetSymbolVendor();
 
     if (symbol_vendor) {

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

std::function<void(clang::FunctionDecl *)> MaybeRegisterFunctionBody =
         }
       };
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   SymbolContextList sc_list;
 
   const ConstString name(context.m_decl_name.getAsString().c_str());
-
-  const char *name_unique_cstr = name.GetCString();
-
-  if (name_unique_cstr == NULL)
-    return;
-
-  static ConstString id_name("id");
-  static ConstString Class_name("Class");
-
-  if (name == id_name || name == Class_name)
+  if (IgnoreName(name, false))
     return;
 
   // Only look for functions by name out in our symbols if the function
   // doesn't start with our phony prefix of '$'
   Target *target = m_parser_vars->m_exe_ctx.GetTargetPtr();
   StackFrame *frame = m_parser_vars->m_exe_ctx.GetFramePtr();
   SymbolContext sym_ctx;
   if (frame != nullptr)
do {
       if (log)
         log->Printf("  CEDM::FEVD[%u] Found persistent decl %s", current_id,
                     name.GetCString());
 
       context.AddNamedDecl(parser_named_decl);
     } while (0);
   }
 
-  if (name_unique_cstr[0] == '$' && !namespace_decl) {
+  if (name.GetCString()[0] == '$' && !namespace_decl) {
     static ConstString g_lldb_class_name("$__lldb_class");
 
     if (name == g_lldb_class_name) {
       // Clang is looking for the type of "this"
 
       if (frame == NULL)
         return;
 
if (name == ConstString(g_lldb_local_vars_namespace_cstr)) {
 
       if (frame_decl_context) {
         ClangASTContext *ast = llvm::dyn_cast_or_null<ClangASTContext>(
             frame_decl_context.GetTypeSystem());
 
         if (ast) {
           clang::NamespaceDecl *namespace_decl =
               ClangASTContext::GetUniqueNamespaceDeclaration(
-                  m_ast_context, name_unique_cstr, nullptr);
+                  m_ast_context, name.GetCString(), nullptr);
           if (namespace_decl) {
             context.AddNamedDecl(namespace_decl);
             clang::DeclContext *clang_decl_ctx =
                 clang::Decl::castToDeclContext(namespace_decl);
             clang_decl_ctx->setHasExternalVisibleStorage(true);
             context.m_found.local_vars_nsp = true;
           }
         }
       }
 
       return;
     }
 
     // any other $__lldb names should be weeded out now
-    if (!::strncmp(name_unique_cstr, "$__lldb", sizeof("$__lldb") - 1))
+    if (name.GetStringRef().startswith("$__lldb"))
       return;
 
     ExpressionVariableSP pvar_sp(
         m_parser_vars->m_persistent_vars->GetVariable(name));
 
     if (pvar_sp) {
       AddOneVariable(context, pvar_sp, current_id);
       return;

lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

AppleObjCRuntimeV2::CreateExceptionResolver(Breakpoint *bkpt, bool catch_bp,
   return resolver_sp;
 }
 
 UtilityFunction *AppleObjCRuntimeV2::CreateObjectChecker(const char *name) {
   char check_function_code[2048];
 
   int len = 0;
   if (m_has_object_getClass) {
-    len = ::snprintf(check_function_code, sizeof(check_function_code),
-                     "extern \"C\" void *gdb_object_getClass(void *);          "
-                     "                                \n"
-                     "extern \"C\"  int printf(const char *format, ...);       "
-                     "                                \n"
-                     "extern \"C\" void                                        "
-                     "                                \n"
-                     "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)    "
-                     "                                \n"
-                     "{                                                        "
-                     "                                \n"
-                     "   if ($__lldb_arg_obj == (void *)0)                     "
-                     "                                \n"
-                     "       return; // nil is ok                              "
-                     "                                \n"
-                     "   if (!gdb_object_getClass($__lldb_arg_obj))            "
-                     "                                \n"
-                     "       *((volatile int *)0) = 'ocgc';                    "
-                     "                                \n"
-                     "   else if ($__lldb_arg_selector != (void *)0)           "
-                     "                                \n"
-                     "   {                                                     "
-                     "                                \n"
-                     "        signed char responds = (signed char) [(id) "
-                     "$__lldb_arg_obj                       \n"
-                     "                                                "
-                     "respondsToSelector:                      \n"
-                     "                                       (struct "
-                     "objc_selector *) $__lldb_arg_selector];   \n"
-                     "       if (responds == (signed char) 0)                  "
-                     "                                \n"
-                     "           *((volatile int *)0) = 'ocgc';                "
-                     "                                \n"
-                     "   }                                                     "
-                     "                                \n"
-                     "}                                                        "
-                     "                                \n",
-                     name);
-  } else {
-    len = ::snprintf(check_function_code, sizeof(check_function_code),
-                     "extern \"C\" void *gdb_class_getClass(void *);           "
-                     "                                \n"
-                     "extern \"C\"  int printf(const char *format, ...);       "
-                     "                                \n"
-                     "extern \"C\"  void                                       "
-                     "                                \n"
-                     "%s(void *$__lldb_arg_obj, void *$__lldb_arg_selector)    "
-                     "                                \n"
-                     "{                                                        "
-                     "                                \n"
-                     "   if ($__lldb_arg_obj == (void *)0)                     "
-                     "                                \n"
-                     "       return; // nil is ok                              "
-                     "                                \n"
-                     "    void **$isa_ptr = (void **)$__lldb_arg_obj;          "
-                     "                                \n"
-                     "    if (*$isa_ptr == (void *)0 || "
-                     "!gdb_class_getClass(*$isa_ptr))                        \n"
-                     "       *((volatile int *)0) = 'ocgc';                    "
-                     "                                \n"
-                     "   else if ($__lldb_arg_selector != (void *)0)           "
-                     "                                \n"
-                     "   {                                                     "
-                     "                                \n"
-                     "        signed char responds = (signed char) [(id) "
-                     "$__lldb_arg_obj                       \n"
-                     "                                                "
-                     "respondsToSelector:                      \n"
-                     "                                        (struct "
-                     "objc_selector *) $__lldb_arg_selector];  \n"
-                     "       if (responds == (signed char) 0)                  "
-                     "                                \n"
-                     "           *((volatile int *)0) = 'ocgc';                "
-                     "                                \n"
-                     "   }                                                     "
-                     "                                \n"
-                     "}                                                        "
-                     "                                \n",
-                     name);
+    len = ::snprintf(check_function_code, sizeof(check_function_code), R"(
+                     extern "C" void *gdb_object_getClass(void *);
+                     extern "C" int printf(const char *format, ...);
+                     extern "C" void
+                     %s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) {
+                       if ($__lldb_arg_obj == (void *)0)
+                         return; // nil is ok
+                       if (!gdb_object_getClass($__lldb_arg_obj)) {
+                         *((volatile int *)0) = 'ocgc';
+                       } else if ($__lldb_arg_selector != (void *)0) {
+                         signed char $responds = (signed char)
+                             [(id)$__lldb_arg_obj respondsToSelector:
+                                 (void *) $__lldb_arg_selector];
+                         if ($responds == (signed char) 0)
+                           *((volatile int *)0) = 'ocgc';
+                       }
+                     })", name);
+  } else {
+    len = ::snprintf(check_function_code, sizeof(check_function_code), R"(
+                     extern "C" void *gdb_class_getClass(void *);
+                     extern "C" int printf(const char *format, ...);
+                     extern "C" void
+                     %s(void *$__lldb_arg_obj, void *$__lldb_arg_selector) {
+                       if ($__lldb_arg_obj == (void *)0)
+                         return; // nil is ok
+                       void **$isa_ptr = (void **)$__lldb_arg_obj;
+                       if (*$isa_ptr == (void *)0 ||
+                           !gdb_class_getClass(*$isa_ptr))
+                         *((volatile int *)0) = 'ocgc';
+                       else if ($__lldb_arg_selector != (void *)0) {
+                         signed char $responds = (signed char)
+                             [(id)$__lldb_arg_obj respondsToSelector:
+                                 (void *) $__lldb_arg_selector];
+                         if ($responds == (signed char) 0)
+                           *((volatile int *)0) = 'ocgc';
+                       }
+                     })", name);
   }
 
   assert(len < (int)sizeof(check_function_code));
   UNUSED_IF_ASSERT_DISABLED(len);
 
   Status error;
   return GetTargetRef().GetUtilityFunctionForLanguage(
       check_function_code, eLanguageTypeObjC, name, error);