This is an archive of the discontinued LLVM Phabricator instance.

Inhibit global lookups for symbols in the IR dynamic checks
ClosedPublic

Authored by spyffe on Sep 21 2017, 1:54 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

spyffe created this revision.Sep 21 2017, 1:54 PM
clayborg requested changes to this revision.Sep 21 2017, 2:03 PM
clayborg added a subscriber: clayborg.
clayborg added inline comments.
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.

This revision now requires changes to proceed.Sep 21 2017, 2:03 PM
zturner added inline comments.
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 marked 5 inline comments as done.Sep 22 2017, 4:20 PM

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

spyffe updated this revision to Diff 116444.EditedSep 22 2017, 5:18 PM
spyffe edited edge metadata.

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.
spyffe abandoned this revision.Sep 26 2017, 10:30 AM

Committed as LLDB r314225

This revision was automatically updated to reflect the committed changes.