[Reproducers] Tool to insert SBReproducer macros

[JDevlieghere]

Summary

Simple tool to automate/simplify the workflow of inserting SB_RECORD and SB_REGSITER macros.

Because the tool won't be part of the build process I don't wan't to over-complicate it. Currently it prints a list of SB_REGISTER macros followed by the source file with the SB_RECORD macro's inserted. This means you'd have to copy over the SB_REGISTER macros manually per source file.

See example output here: https://reviews.llvm.org/P8125

[JDevlieghere]

• Update files in place.
• Check for existing macros.

[JDevlieghere]

• Skip unsupported signatures (function pointers and void*)

[JDevlieghere]

Fixed printing of booleans; the tool would show _Bool instead of bool.

I ran this on a few files and it appears to work with the prototype.

[labath]

Is this ready for review now?

tools/sbrepro/SBRepro.cpp

98 ↗

(On Diff #183584)

MacroMacro?

[JDevlieghere]

Fix MacroMacro and other code cleanups.

[labath]

tools/sbrepro/SBRepro.cpp

22–31 ↗

(On Diff #183888)

There's a join in ADT/StringExtras.h which should do what you need here.

194–197 ↗

(On Diff #183888)

What exactly does this skip? Would e.g. a constructor with an empty body still be considered for macro-ization?

[JDevlieghere]

• Use join from StringExtras.
• Rename Skip to ShoupdSkip and document its cases.

[labath]

So, if I understand this correctly, a constructor like SBFoo::SBFoo(){} will not be considered for macroization because it's empty? That doesn't sound right to me. Even an empty constructor has a side effect of constructing the object, which is something that I would expect to see recorded in the trace file. Is there some reason why it is hard/impossible to insert macros into these kinds of functions? If so, it may be worth just warning the user about their existence.

A function with an empty body probably doesn't do anything interesting, so it may be possible to skip recording those, but I think it would make sense to record them nonetheless.

[JDevlieghere]

In D56822#1376770, @labath wrote:

So, if I understand this correctly, a constructor like SBFoo::SBFoo(){} will not be considered for macroization because it's empty? That doesn't sound right to me. Even an empty constructor has a side effect of constructing the object, which is something that I would expect to see recorded in the trace file. Is there some reason why it is hard/impossible to insert macros into these kinds of functions? If so, it may be worth just warning the user about their existence.

A function with an empty body probably doesn't do anything interesting, so it may be possible to skip recording those, but I think it would make sense to record them nonetheless.

Ugh, I was confusing empty bodies with methods without a body (e.g. declarations). Empty bodies are definitely handled! I'll update the comment.

[labath]

ok, looks good then

[JDevlieghere]

• Skip decls that have a canonical decl in the implementation file.

[JDevlieghere]

It doesn't really make sense to land this without the SBReproducer framework, so I'll leave it open until that gets accepted as well.

[JDevlieghere]

• Use new LLDB_ prefix
• Insert two newlines at the end of the macro, so clang-format fixes it up consistently.

lit/Reproducer/Inputs/foo.cpp

+struct Foo {
+  Foo();
+  Foo(int i);
+
+  void A();
+  void B(int i);
+  int C(int i);
+  int D(bool b) const;
+  static void E();
+  static int F(int i);
+};
+
+void Foo::A() {}
+void Foo::B(int i) {}
+int Foo::C(int i) { return i; }
+int Foo::D(bool b) const { return 1; }
+void Foo::E() {}
+int Foo::F(int i) { return i; }

lit/Reproducer/TestInstrumentationRecord.test

+# RUN: cp %p/Inputs/foo.cpp %t.cpp
+# RUN: lldb-instr %t.cpp
+# RUN: cat %t.cpp | FileCheck %s
+
+# CHECK: LLDB_RECORD_METHOD_NO_ARGS(void, Foo, A);
+# CHECK: LLDB_RECORD_METHOD(void, Foo, B, (int), i);
+# CHECK: LLDB_RECORD_METHOD(int, Foo, C, (int), i);
+# CHECK: LLDB_RECORD_METHOD_CONST(int, Foo, D, (bool), b);
+# CHECK: LLDB_RECORD_STATIC_METHOD_NO_ARGS(void, Foo, E);
+# CHECK: LLDB_RECORD_STATIC_METHOD(int, Foo, F, (int), i);

lit/Reproducer/TestInstrumentationRegister.test

+# RUN: cp %p/Inputs/foo.cpp %t.cpp
+# RUN: lldb-instr %t.cpp | FileCheck %s
+
+# CHECK: LLDB_REGISTER_METHOD(void, Foo, A, ());
+# CHECK: LLDB_REGISTER_METHOD(void, Foo, B, (int));
+# CHECK: LLDB_REGISTER_METHOD(int, Foo, C, (int));
+# CHECK: LLDB_REGISTER_METHOD_CONST(int, Foo, D, (bool));
+# CHECK: LLDB_REGISTER_STATIC_METHOD(void, Foo, E, ());
+# CHECK: LLDB_REGISTER_STATIC_METHOD(int, Foo, F, (int));

lit/helper/toolchain.py

def use_lldb_substitutions(config):
     lldbmi = ToolSubst('%lldbmi',
                        command=FindTool('lldb-mi'),
                        extra_args=['--synchronous'],
                        unresolved='ignore')
 
 
     build_script = os.path.dirname(__file__)
     build_script = os.path.join(build_script, 'build.py')
-    build_script_args = [build_script, 
+    build_script_args = [build_script,
                         '--compiler=any', # Default to best compiler
                         '--arch=' + str(config.lldb_bitness)]
     if config.lldb_lit_tools_dir:
         build_script_args.append('--tools-dir={0}'.format(config.lldb_lit_tools_dir))
     if config.lldb_tools_dir:
         build_script_args.append('--tools-dir={0}'.format(config.lldb_tools_dir))
 
     primary_tools = [
         ToolSubst('%lldb',
                   command=FindTool('lldb'),
                   extra_args=['-S',
                               os.path.join(config.test_source_root,
                                            'lit-lldb-init')]),
         lldbmi,
         ToolSubst('%debugserver',
                   command=FindTool(dsname),
                   extra_args=dsargs,
                   unresolved='ignore'),
         'lldb-test',
+        'lldb-instr',
         ToolSubst('%build',
                   command="'" + sys.executable + "'",
                   extra_args=build_script_args)
         ]
 
     llvm_config.add_tool_substitutions(primary_tools,
                                        [config.lldb_tools_dir])
     if lldbmi.was_resolved:

tools/CMakeLists.txt

 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
+add_subdirectory(lldb-instr)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
 add_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
   add_subdirectory(debugserver)
 endif()
 
 if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD)
   add_subdirectory(lldb-server)
 endif()

tools/lldb-instr/CMakeLists.txt

+add_lldb_tool(lldb-instr
+  Instrument.cpp
+
+  LINK_LIBS
+    clangAST
+    clangBasic
+    clangFrontend
+    clangTooling
+
+  LINK_COMPONENTS
+    Support
+  )

tools/lldb-instr/Instrument.cpp

+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTConsumers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <sstream>
+#include <string>
+
+using namespace clang;
+using namespace clang::driver;
+using namespace clang::tooling;
+
+static llvm::cl::OptionCategory InstrCategory("LLDB Instrumentation Generator");
+
+/// Get the macro name for recording method calls.
+///
+/// LLDB_RECORD_METHOD
+/// LLDB_RECORD_METHOD_CONST
+/// LLDB_RECORD_METHOD_NO_ARGS
+/// LLDB_RECORD_METHOD_CONST_NO_ARGS
+/// LLDB_RECORD_STATIC_METHOD
+/// LLDB_RECORD_STATIC_METHOD_NO_ARGS
+static std::string GetRecordMethodMacroName(bool Static, bool Const,
+                                            bool NoArgs) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_RECORD";
+  if (Static)
+    OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+    OS << "_CONST";
+  if (NoArgs)
+    OS << "_NO_ARGS";
+
+  return OS.str();
+}
+
+/// Get the macro name for register methods.
+///
+/// LLDB_REGISTER_CONSTRUCTOR
+/// LLDB_REGISTER_METHOD
+/// LLDB_REGISTER_METHOD_CONST
+/// LLDB_REGISTER_STATIC_METHOD
+static std::string GetRegisterMethodMacroName(bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "LLDB_REGISTER";
+  if (Static)
+    OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+    OS << "_CONST";
+
+  return OS.str();
+}
+
+static std::string GetRecordMethodMacro(StringRef Result, StringRef Class,
+                                        StringRef Method, StringRef Signature,
+                                        StringRef Values, bool Static,
+                                        bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << GetRecordMethodMacroName(Static, Const, Values.empty());
+  OS << "(" << Result << ", " << Class << ", " << Method;
+
+  if (!Values.empty()) {
+    OS << ", (" << Signature << "), " << Values << ");\n\n";
+  } else {
+    OS << ");\n\n";
+  }
+
+  return OS.str();
+}
+
+static std::string GetRecordConstructorMacro(StringRef Class,
+                                             StringRef Signature,
+                                             StringRef Values) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  if (!Values.empty()) {
+    OS << "LLDB_RECORD_CONSTRUCTOR(" << Class << ", (" << Signature << "), "
+       << Values << ");\n\n";
+  } else {
+    OS << "LLDB_RECORD_CONSTRUCTOR_NO_ARGS(" << Class << ");\n\n";
+  }
+  return OS.str();
+}
+
+static std::string GetRegisterConstructorMacro(StringRef Class,
+                                               StringRef Signature) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << "LLDB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature
+     << "));\n\n";
+  return OS.str();
+}
+
+static std::string GetRegisterMethodMacro(StringRef Result, StringRef Class,
+                                          StringRef Method, StringRef Signature,
+                                          bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << GetRegisterMethodMacroName(Static, Const);
+  OS << "(" << Result << ", " << Class << ", " << Method << ", (" << Signature
+     << "));\n";
+  return OS.str();
+}
+
+class SBVisitor : public RecursiveASTVisitor<SBVisitor> {
+public:
+  SBVisitor(Rewriter &R, ASTContext &Context)
+      : MyRewriter(R), Context(Context) {}
+
+  bool VisitCXXMethodDecl(CXXMethodDecl *Decl) {
+    // Not all decls should be registered. Please refer to that method's
+    // comment for details.
+    if (ShouldSkip(Decl))
+      return false;
+
+    // Print 'bool' instead of '_Bool'.
+    PrintingPolicy Policy(Context.getLangOpts());
+    Policy.Bool = true;
+
+    // Collect the functions parameter types and names.
+    std::vector<std::string> ParamTypes;
+    std::vector<std::string> ParamNames;
+    for (auto *P : Decl->parameters()) {
+      QualType T = P->getType();
+
+      // Currently we don't support functions that have function pointers as an
+      // argument.
+      if (T->isFunctionPointerType())
+        return false;
+
+      // Currently we don't support functions that have void pointers as an
+      // argument.
+      if (T->isVoidPointerType())
+        return false;
+
+      ParamTypes.push_back(T.getAsString(Policy));
+      ParamNames.push_back(P->getNameAsString());
+    }
+
+    // Convert the two lists to string for the macros.
+    std::string ParamTypesStr = llvm::join(ParamTypes, ", ");
+    std::string ParamNamesStr = llvm::join(ParamNames, ", ");
+
+    CXXRecordDecl *Record = Decl->getParent();
+    QualType ReturnType = Decl->getReturnType();
+
+    // Construct the macros.
+    std::string Macro;
+    if (isa<CXXConstructorDecl>(Decl)) {
+      llvm::outs() << GetRegisterConstructorMacro(Record->getNameAsString(),
+                                                  ParamTypesStr);
+
+      Macro = GetRecordConstructorMacro(Record->getNameAsString(),
+                                        ParamTypesStr, ParamNamesStr);
+    } else {
+      llvm::outs() << GetRegisterMethodMacro(
+          ReturnType.getAsString(Policy), Record->getNameAsString(),
+          Decl->getNameAsString(), ParamTypesStr, Decl->isStatic(),
+          Decl->isConst());
+
+      Macro = GetRecordMethodMacro(
+          ReturnType.getAsString(Policy), Record->getNameAsString(),
+          Decl->getNameAsString(), ParamTypesStr, ParamNamesStr,
+          Decl->isStatic(), Decl->isConst());
+    }
+
+    // If this CXXMethodDecl already starts with a macro we're done.
+    Stmt *Body = Decl->getBody();
+    for (auto &C : Body->children()) {
+      if (C->getBeginLoc().isMacroID())
+        return false;
+      break;
+    }
+
+    // Insert the macro at the beginning of the function. We don't attempt to
+    // fix the formatting and instead rely on clang-format to fix it after the
+    // tool has run. This is also the reason that the macros end with two
+    // newlines, counting on clang-format to normalize this in case the macro
+    // got inserted before an existing newline.
+    SourceLocation InsertLoc = Lexer::getLocForEndOfToken(
+        Body->getBeginLoc(), 0, MyRewriter.getSourceMgr(),
+        MyRewriter.getLangOpts());
+    MyRewriter.InsertTextAfter(InsertLoc, Macro);
+
+    return true;
+  }
+
+private:
+  /// Determine whether we need to consider the given CXXMethodDecl.
+  ///
+  /// Currently we skip the following cases:
+  ///  1. Decls outside the main source file,
+  ///  2. Decls that are only present in the source file,
+  ///  3. Decls that are not definitions,
+  ///  4. Non-public decls,
+  ///  5. Destructors.
+  bool ShouldSkip(CXXMethodDecl *Decl) {
+    // Skip anything outside the main file.
+    if (!MyRewriter.getSourceMgr().isInMainFile(Decl->getBeginLoc()))
+      return true;
+
+    // Skip if the canonical decl in the current decl. It means that the method
+    // is declared in the implementation and is therefore not exposed as part
+    // of the API.
+    if (Decl == Decl->getCanonicalDecl())
+      return true;
+
+    // Skip decls that have no body, i.e. are just declarations.
+    Stmt *Body = Decl->getBody();
+    if (!Body)
+      return true;
+
+    // Skip non-public decls.
+    AccessSpecifier AS = Decl->getAccess();
+    if (AS != AccessSpecifier::AS_public)
+      return true;
+
+    // Skip destructors.
+    if (isa<CXXDestructorDecl>(Decl))
+      return true;
+
+    return false;
+  }
+
+  Rewriter &MyRewriter;
+  ASTContext &Context;
+};
+
+class SBConsumer : public ASTConsumer {
+public:
+  SBConsumer(Rewriter &R, ASTContext &Context) : Visitor(R, Context) {}
+
+  // Override the method that gets called for each parsed top-level
+  // declaration.
+  bool HandleTopLevelDecl(DeclGroupRef DR) override {
+    for (DeclGroupRef::iterator b = DR.begin(), e = DR.end(); b != e; ++b) {
+      Visitor.TraverseDecl(*b);
+    }
+    return true;
+  }
+
+private:
+  SBVisitor Visitor;
+};
+
+class SBAction : public ASTFrontendAction {
+public:
+  SBAction() = default;
+
+  void EndSourceFileAction() override { MyRewriter.overwriteChangedFiles(); }
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef file) override {
+    MyRewriter.setSourceMgr(CI.getSourceManager(), CI.getLangOpts());
+    return llvm::make_unique<SBConsumer>(MyRewriter, CI.getASTContext());
+  }
+
+private:
+  Rewriter MyRewriter;
+};
+
+int main(int argc, const char **argv) {
+  CommonOptionsParser OP(argc, argv, InstrCategory,
+                         "Utility for generating the macros for LLDB's "
+                         "instrumentation framework.");
+  ClangTool T(OP.getCompilations(), OP.getSourcePathList());
+  return T.run(newFrontendActionFactory<SBAction>().get());
+}