Load compiler plugins in ASTUnit, too

[kfunk]

Summary

This change makes it possible to load plugins by passing extra clang
compiler arguments to libclang's clang_parseTranslation entry point.

Example: -Xclang -load -Xclang $LLVM_ROOT/lib/ClangLazy.so -Xclang
-add-plugin -Xclang clang-lazy

This makes it possible to introduce additional diagnostics, which can
then be shown by IDEs

See discussion on cfe-dev: http://lists.llvm.org/pipermail/cfe-dev/2014-October/039380.html

Original patch by Stefan Kemnitz <kemnitz.stefan at gmail.com>

[kfunk]

Doing as requested by http://lists.llvm.org/pipermail/cfe-dev/2014-October/039381.html, let's put this up for review in its initial form, so we can discuss a better approach.

[kfunk]

By the way; prove that it really works: http://wstaw.org/m/2015/12/23/kdevelop-clazy.png (loving it!) :)

[milianw]

lgtm, but someone else should approve

[doug.gregor]

It looks good. Are you able to include a test for this?

[kfunk]

Update, add (non-working) test

Just uploading my WIP patch, now that the branching comes close. I added a test, unfortunately it doesn't do what I expect.
Please see the FIXME.

Didn't have the time to investigate yet, so any hints welcome!

[realincubus]

Hi.

I am finally back at work and happy to see that some effort already went into the patch that i initially wrote some time ago (D5611)

I just looked through the files of the new patch and noticed that the libclang.exports file is missing.
As far as i understand this, the exports file defines which symbols will be visible to the linker when linking to the
libclang.so library.

Since some of the symbols that are needed for the plugin mechanism to work are not exported, i wrote a list of these symbols and appended theses to the libclang.exports file.

If i don't add these symbols it's impossible to load any plugin from libclang.so

I assume that if you write a test for loading plugins via libclang you should write a test that acually uses libclang for loading plugins via its C interface (clang_parseTranslationUnit).

@kfunk you said that this works for you without the additional symbols. it might be that something changed in clangs codebase and i dont have to export the symbols anymore. In the following days i will try to run my code without the symbols and write a test that uses libclangs C interface.

all the best stefan kemnitz

[kfunk]

@realincubus: Sorry, I didn't know you had put this up for review already.

So back to this patch. Yes, it works fine for me *without* amending libclang.exports. I'm injecting the arguments to clang_parseTranslationUnit2.

See this patch here for reference: https://quickgit.kde.org/?p=kdevelop.git&a=commit&h=b299e550f6753667d415c6aea8c0b0806e954e36
(when you view the whole blob of *parsesession.cpp*, you can also see the call to clang_parseTranslationUnit2)

[realincubus]

ok after i checked out a recent version of clang and applied the patch D15729 i can verify, that one does not have to add the symbols to the libclang.exports file.

i believe the plugin mechanism was changed between the first version of my plugin and how it works right now.
i will now try to find out why the test doesn't work like it should or write another one.

[milianw]

Still good from my side. @klimek, @rsmith: Could you please review this as well?

Thanks

[kfunk]

Note: Unit test is still not functional. I'll have another look at it now.

[kfunk]

Remove TODO statement

[kfunk]

Okay, just gave this another look. Unit test seems fine as-is. Let's call loadPlugins there explicitly.

@LLVM/Clang devs: Please review!

I'm especially interested in whether you think there's a better location for calling CompilerInstance::loadPlugins than in ASTUnit::Parse.

[kfunk]

@bkramer: Thoughts about my patch? I wonder if this makes your "work-around" in D17808 redundant.

[kfunk]

Remove unrelated hunk

[bkramer]

This looks good, thanks for the patch. When this goes in please watch the windows buildbots as plugin support on windows is somewhat broken and we may have to disable tests there.

Also, while this is a strict improvement and should go in I'm not so sure that it solves the "plugins in libclang" issue. I couldn't get it to work even with this patch due to the way we build libclang in Release mode.

[realincubus]

@bkramer I can verify that after building this with clang 3.9 in release mode it does not work anymore like expected. everything is fine in debug mode

if i look at the output generated from

llvm::sys::DynamicLibrary::LoadLibraryPermanently

in 1761 CompilerInstance::loadPlugins()

i get errors like

/home/incubus/llvm_patch_test/build/lib/ClangPlugin.so /home/incubus/llvm_patch_test/build/lib/ClangPlugin.so: undefined symbol: _ZN5clang17ASTFrontendAction13ExecuteActionEv

In my original patch D5611 I added a list of symbols that had to be exported in order to make libclang work with plugins like expected.
After adding the symbols from the original patch and 2 additional ones it is ok now with clang version 3.9 in release mode.

Symbols needed:

_ZN5clang15PluginASTAction6anchorEv
_ZN4llvm11raw_ostream5writeEPKcm
_ZN4llvm4errsEv
_ZN5clang11ASTConsumer33HandleTopLevelDeclInObjCContainerENS_12DeclGroupRefE
_ZN5clang14FrontendActionD2Ev
_ZN5clang13DiagnosticIDs15getCustomDiagIDENS0_5LevelEN4llvm9StringRefE
_ZN5clang17ASTFrontendAction13ExecuteActionEv
_ZN5clang11ASTConsumer24HandleImplicitImportDeclEPNS_10ImportDeclE
_ZN5clang14FrontendAction22shouldEraseOutputFilesEv
_ZNK5clang15DeclarationName11getAsStringEv
_ZN5clang17DiagnosticsEngine21EmitCurrentDiagnosticEb
_ZN5clang11ASTConsumer21HandleInterestingDeclENS_12DeclGroupRefE
_ZN5clang14FrontendActionC2Ev
_ZN4llvm8RegistryIN5clang15PluginASTActionENS_14RegistryTraitsIS2_EEE4nodeC1ERKNS_19SimpleRegistryEntryIS2_EE
_ZTVN5clang15PluginASTActionE
_ZTVN5clang17ASTFrontendActionE
_ZTVN5clang11ASTConsumerE
_ZN4llvm8RegistryIN5clang15PluginASTActionEE4HeadE
_ZN4llvm8RegistryIN5clang15PluginASTActionEE4TailE

Please tell me if I am wrong, but "tools/libclang/libclang.exports" is kind of a filter.
Everything that is not listed in this file is not exported as a symbol in the resulting libclang.so file.

Is there a way to automatically add the needed symbols to the exports file ?

[bkramer]

If we really want to go down this route we should drop the exports file, putting random symbols in there is not going to fly. I'm not sure what that would break for libclang users though.

Also make sure that your building without BUILD_SHARED_LIBS, otherwise you'll get a very distorted view of how libclang works.

[realincubus]

would it be possible to add some cmake flag that would allow the user to choose whether to respect the libclang.exports file or not ?

[kfunk]

Marking this Diff properly -- it needs more work.

include/clang/Frontend/CompilerInstance.h

public:
 
   GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc) override;
 
   bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override;
 
   void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {
     DependencyCollectors.push_back(std::move(Listener));
   }
+
+  /// load plugins that were added by -load
+  void loadPlugins();
 };
 
 } // end namespace clang
 
 #endif

lib/Frontend/ASTUnit.cpp

bool ASTUnit::Parse(std::shared_ptr<PCHContainerOperations> PCHContainerOps,
 
   std::unique_ptr<TopLevelDeclTrackerAction> Act(
       new TopLevelDeclTrackerAction(*this));
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar<TopLevelDeclTrackerAction>
     ActCleanup(Act.get());
 
+  Clang->loadPlugins();
+
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
     goto error;
 
   if (SavedMainFileBuffer) {
     std::string ModName = getPreambleFile(this);
     TranslateStoredDiagnostics(getFileManager(), getSourceManager(),
                                PreambleDiagnostics, StoredDiagnostics);
   }

lib/Frontend/CompilerInstance.cpp

 #include "llvm/Support/Host.h"
 #include "llvm/Support/LockFileManager.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include <sys/stat.h>
 #include <system_error>
 #include <time.h>
 
 using namespace clang;
 
 CompilerInstance::CompilerInstance(
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
if (GlobalIndex) {
       if (GlobalIndex->lookupIdentifier(Name, FoundModules))
         return true;
     }
   }
 
   return false;
 }
 void CompilerInstance::resetAndLeakSema() { BuryPointer(takeSema()); }
+
+void
+CompilerInstance::loadPlugins(){
+  // Load any requested plugins.
+  for (unsigned i = 0, e = getFrontendOpts().Plugins.size(); i != e; ++i) {
+    const std::string &Path = getFrontendOpts().Plugins[i];
+    std::string Error;
+    if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
+      getDiagnostics().Report(diag::err_fe_unable_to_load_plugin)
+        << Path << Error;
+  }
+}

lib/FrontendTool/ExecuteCompilerInvocation.cpp

bool clang::ExecuteCompilerInvocation(CompilerInstance *Clang) {
   //
   // FIXME: Use a better -version message?
   if (Clang->getFrontendOpts().ShowVersion) {
     llvm::cl::PrintVersionMessage();
     return true;
   }
 
   // Load any requested plugins.
-  for (unsigned i = 0,
-         e = Clang->getFrontendOpts().Plugins.size(); i != e; ++i) {
-    const std::string &Path = Clang->getFrontendOpts().Plugins[i];
-    std::string Error;
-    if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
-      Clang->getDiagnostics().Report(diag::err_fe_unable_to_load_plugin)
-        << Path << Error;
-  }
+  Clang->loadPlugins();
+
 
   // Honor -mllvm.
   //
   // FIXME: Remove this, one day.
   // This should happen AFTER plugins have been loaded!
   if (!Clang->getFrontendOpts().LLVMArgs.empty()) {
     unsigned NumArgs = Clang->getFrontendOpts().LLVMArgs.size();
     auto Args = llvm::make_unique<const char*[]>(NumArgs + 2);

unittests/Frontend/CMakeLists.txt

+add_subdirectory(PrintFunctionNamesTestPlugin)
+
 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(FrontendTests
   FrontendActionTest.cpp
   CodeGenActionTest.cpp
   )
 target_link_libraries(FrontendTests
   clangAST
   clangBasic
   clangFrontend
   clangLex
   clangSema
   clangCodeGen
   )
+add_dependencies(FrontendTests PrintFunctionNamesTestPlugin)

unittests/Frontend/FrontendActionTest.cpp

 //===- unittests/Frontend/FrontendActionTest.cpp - FrontendAction tests ---===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "gtest/gtest.h"
 
+#include <iostream>
+
 using namespace llvm;
 using namespace clang;
 
 namespace {
 
 class TestASTFrontendAction : public ASTFrontendAction {
 public:
   TestASTFrontendAction(bool enableIncrementalProcessing = false,
TEST(ASTFrontendAction, LateTemplateIncrementalParsing) {
   TestASTFrontendAction test_action(/*enableIncrementalProcessing=*/true,
                                     /*actOnEndOfTranslationUnit=*/true);
   ASSERT_TRUE(compiler.ExecuteAction(test_action));
   ASSERT_EQ(13U, test_action.decl_names.size());
   EXPECT_EQ("A", test_action.decl_names[0]);
   EXPECT_EQ("c", test_action.decl_names[12]);
 }
 
+bool pluginIsLoaded(const std::string &pluginName) {
+  for (auto it = FrontendPluginRegistry::begin(),
+       ie = FrontendPluginRegistry::end();
+       it != ie; ++it) {
+    if (it->getName() == pluginName) {
+      return true;
+    }
+  }
+  return false;
+}
+
+TEST(ASTFrontendAction, PluginLoadTest) {
+  CompilerInvocation *invocation = new CompilerInvocation;
+  invocation->getLangOpts()->CPlusPlus = true;
+  invocation->getPreprocessorOpts().addRemappedFile(
+      "test.cc",
+      MemoryBuffer::getMemBuffer("int main() { }").release());
+  invocation->getFrontendOpts().Inputs.push_back(FrontendInputFile("test.cc",
+                                                                   IK_CXX));
+  invocation->getFrontendOpts().ProgramAction = frontend::ParseSyntaxOnly;
+  invocation->getFrontendOpts().Plugins.push_back("PrintFunctionNamesTestPlugin.so");
+  invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+  CompilerInstance compiler;
+  compiler.setInvocation(invocation);
+  compiler.createDiagnostics();
+  compiler.loadPlugins();
+
+  TestASTFrontendAction test_action(/*enableIncrementalProcessing=*/true,
+                                    /*actOnEndOfTranslationUnit=*/true);
+  ASSERT_TRUE(compiler.ExecuteAction(test_action));
+  ASSERT_TRUE(pluginIsLoaded("print-fns"));
+}
+
 struct TestPPCallbacks : public PPCallbacks {
   TestPPCallbacks() : SeenEnd(false) {}
 
   void EndOfMainFile() override { SeenEnd = true; }
 
   bool SeenEnd;
 };
 

unittests/Frontend/PrintFunctionNamesTestPlugin/CMakeLists.txt

+add_llvm_loadable_module(PrintFunctionNamesTestPlugin PrintFunctionNamesTestPlugin.cpp)
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  target_link_libraries(PrintFunctionNamesTestPlugin ${cmake_2_8_12_PRIVATE}
+    clangAST
+    clangBasic
+    clangFrontend
+    LLVMSupport
+    )
+endif()

unittests/Frontend/PrintFunctionNamesTestPlugin/PrintFunctionNamesTestPlugin.cpp

+//===- PrintFunctionNames.cpp ---------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Example clang plugin which simply prints the names of all the top-level decls
+// in the input file.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Sema/Sema.h"
+#include "llvm/Support/raw_ostream.h"
+using namespace clang;
+
+namespace {
+
+class PrintFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  std::set<std::string> ParsedTemplates;
+
+public:
+  PrintFunctionsConsumer(CompilerInstance &Instance,
+                         std::set<std::string> ParsedTemplates)
+      : Instance(Instance), ParsedTemplates(ParsedTemplates) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+    for (DeclGroupRef::iterator i = DG.begin(), e = DG.end(); i != e; ++i) {
+      const Decl *D = *i;
+      if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
+        llvm::errs() << "top-level-decl: \"" << ND->getNameAsString() << "\"\n";
+    }
+
+    return true;
+  }
+
+  void HandleTranslationUnit(ASTContext& context) override {
+    if (!Instance.getLangOpts().DelayedTemplateParsing)
+      return;
+
+    // This demonstrates how to force instantiation of some templates in
+    // -fdelayed-template-parsing mode. (Note: Doing this unconditionally for
+    // all templates is similar to not using -fdelayed-template-parsig in the
+    // first place.)
+    // The advantage of doing this in HandleTranslationUnit() is that all
+    // codegen (when using -add-plugin) is completely finished and this can't
+    // affect the compiler output.
+    struct Visitor : public RecursiveASTVisitor<Visitor> {
+      const std::set<std::string> &ParsedTemplates;
+      Visitor(const std::set<std::string> &ParsedTemplates)
+          : ParsedTemplates(ParsedTemplates) {}
+      bool VisitFunctionDecl(FunctionDecl *FD) {
+        if (FD->isLateTemplateParsed() &&
+            ParsedTemplates.count(FD->getNameAsString()))
+          LateParsedDecls.insert(FD);
+        return true;
+      }
+
+      std::set<FunctionDecl*> LateParsedDecls;
+    } v(ParsedTemplates);
+    v.TraverseDecl(context.getTranslationUnitDecl());
+    clang::Sema &sema = Instance.getSema();
+    for (const FunctionDecl *FD : v.LateParsedDecls) {
+      clang::LateParsedTemplate* LPT = sema.LateParsedTemplateMap.lookup(FD);
+      sema.LateTemplateParser(sema.OpaqueParser, *LPT);
+      llvm::errs() << "late-parsed-decl: \"" << FD->getNameAsString() << "\"\n";
+    }
+  }
+};
+
+class PrintFunctionNamesAction : public PluginASTAction {
+  std::set<std::string> ParsedTemplates;
+protected:
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 llvm::StringRef) override {
+    return llvm::make_unique<PrintFunctionsConsumer>(CI, ParsedTemplates);
+  }
+
+  bool ParseArgs(const CompilerInstance &CI,
+                 const std::vector<std::string> &args) override {
+    for (unsigned i = 0, e = args.size(); i != e; ++i) {
+      llvm::errs() << "PrintFunctionNames arg = " << args[i] << "\n";
+
+      // Example error handling.
+      DiagnosticsEngine &D = CI.getDiagnostics();
+      if (args[i] == "-an-error") {
+        unsigned DiagID = D.getCustomDiagID(DiagnosticsEngine::Error,
+                                            "invalid argument '%0'");
+        D.Report(DiagID) << args[i];
+        return false;
+      } else if (args[i] == "-parse-template") {
+        if (i + 1 >= e) {
+          D.Report(D.getCustomDiagID(DiagnosticsEngine::Error,
+                                     "missing -parse-template argument"));
+          return false;
+        }
+        ++i;
+        ParsedTemplates.insert(args[i]);
+      }
+    }
+    if (!args.empty() && args[0] == "help")
+      PrintHelp(llvm::errs());
+
+    return true;
+  }
+  void PrintHelp(llvm::raw_ostream& ros) {
+    ros << "Help for PrintFunctionNames plugin goes here\n";
+  }
+
+};
+
+}
+
+static FrontendPluginRegistry::Add<PrintFunctionNamesAction>
+X("print-fns", "print function names");