Use the dyld to show more meaningful message when a symbol is missing.

The dynamic library manager's dyld can search a symbol in the library path.
This patch uses that functionality to aid the 'unresolved while linking'
diagnostics.

Now we get:

cling -L lib/

****************** CLING ******************
* Type C++ code and press enter to run it *
*             Type .q to exit             *
*******************************************
[cling]$ extern int gErrorIgnoreLevel;
[cling]$ gErrorIgnoreLevel
IncrementalExecutor::executeFunction: symbol 'gErrorIgnoreLevel' unresolved while linking [cling interface function]!
Symbol found in '/.../lib/libCore.so'; did you mean to load it with .L /.../lib/libCore.so ?
[cling]$

include/cling/Interpreter/DynamicLibraryManager.h

@@ -20,7 +20,6 @@
 namespace cling {
   class Dyld;
   class InterpreterCallbacks;
-  class InvocationOptions;
 
   ///\brief A helper class managing dynamic shared objects.
   ///
@@ -59,10 +58,6 @@ namespace cling {
     DyLibs m_DyLibs;
     llvm::StringSet<> m_LoadedLibraries;
 
-    ///\brief Contains the list of the current include paths.
-    ///
-    const InvocationOptions& m_Opts;
-
     ///\brief System's include path, get initialized at construction time.
     ///
     SearchPathInfos m_SearchPaths;
@@ -95,7 +90,7 @@ namespace cling {
     ///
     static std::string getSymbolLocation(void* func);
   public:
-    DynamicLibraryManager(const InvocationOptions& Opts);
+    DynamicLibraryManager();
     ~DynamicLibraryManager();
     DynamicLibraryManager(const DynamicLibraryManager&) = delete;
     DynamicLibraryManager& operator=(const DynamicLibraryManager&) = delete;
@@ -112,8 +107,10 @@ namespace cling {
        return m_SearchPaths;
     }
 
-    void addSearchPath(llvm::StringRef dir) {
-      m_SearchPaths.emplace_back(SearchPathInfo{dir, /*IsUser*/ true});
+    void addSearchPath(llvm::StringRef dir, bool isUser = true,
+                       bool prepend = false) {
+      auto pos = prepend ? m_SearchPaths.begin() : m_SearchPaths.end();
+      m_SearchPaths.insert(pos, SearchPathInfo{dir, isUser});
     }
 
     ///\brief Looks up a library taking into account the current include paths
@@ -164,6 +161,8 @@ namespace cling {
     std::string searchLibrariesForSymbol(const std::string& mangledName,
                                          bool searchSystem = true) const;
 
+    void dump(llvm::raw_ostream* S = nullptr) const;
+
     /// On a success returns to full path to a shared object that holds the
     /// symbol pointed by func.
     ///

include/cling/Interpreter/Interpreter.h

@@ -201,10 +201,6 @@ namespace cling {
     ///
     std::unique_ptr<InterpreterCallbacks> m_Callbacks;
 
-    ///\brief Dynamic library manager object.
-    ///
-    std::unique_ptr<DynamicLibraryManager> m_DyLibManager;
-
     ///\brief Information about the last stored states through .storeState
     ///
     mutable std::vector<ClangInternalState*> m_StoredStates;
@@ -442,7 +438,13 @@ namespace cling {
     ///
     ///\param[in] S - stream to dump to or nullptr for default (cling::outs)
     ///
-    void DumpIncludePath(llvm::raw_ostream* S = nullptr);
+    void DumpIncludePath(llvm::raw_ostream* S = nullptr) const;
+
+    ///\brief Prints the current library paths and loaded libraries.
+    ///
+    ///\param[in] S - stream to dump to or nullptr for default (cling::outs)
+    ///
+    void DumpDynamicLibraryInfo(llvm::raw_ostream* S = nullptr) const;
 
     ///\brief Dump various internal data.
     ///
@@ -726,12 +728,8 @@ namespace cling {
     const InterpreterCallbacks* getCallbacks() const {return m_Callbacks.get();}
     InterpreterCallbacks* getCallbacks() { return m_Callbacks.get(); }
 
-    const DynamicLibraryManager* getDynamicLibraryManager() const {
-      return m_DyLibManager.get();
-    }
-    DynamicLibraryManager* getDynamicLibraryManager() {
-      return m_DyLibManager.get();
-    }
+    const DynamicLibraryManager* getDynamicLibraryManager() const;
+    DynamicLibraryManager* getDynamicLibraryManager();
 
     const Transaction* getFirstTransaction() const;
     const Transaction* getLastTransaction() const;

include/cling/MetaProcessor/MetaParser.h

@@ -33,7 +33,7 @@ namespace cling {
   //                            HelpCommand | FileExCommand | FilesCommand |
   //                            ClassCommand | GCommand | StoreStateCommand |
   //                            CompareStateCommand | StatsCommand | undoCommand
-  //                 LCommand := 'L' FilePath
+  //                 LCommand := 'L' [FilePath]
   //                 TCommand := 'T' FilePath FilePath
   //                 >Command := '>' FilePath
   //                 qCommand := 'q'

include/cling/MetaProcessor/MetaSema.h

@@ -59,7 +59,8 @@ namespace cling {
     const Interpreter& getInterpreter() const { return m_Interpreter; }
     bool isQuitRequested() const { return m_IsQuitRequested; }
 
-    ///\brief L command includes the given file or loads the given library.
+    ///\brief L command includes the given file or loads the given library. If
+    /// \c file is empty print the list of library paths.
     ///
     ///\param[in] file - The file/library to be loaded.
     ///\param[out] transaction - Transaction containing the loaded file.
@@ -139,7 +140,8 @@ namespace cling {
     ActionResult actOnUCommand(llvm::StringRef file);
 
     ///\brief Actions to be performed on add include path. It registers new
-    /// folder where header files can be searched.
+    /// folder where header files can be searched. If \c path is empty print the
+    /// list of include paths.
     ///
     ///\param[in] path - The path to add to header search.
     ///

lib/Interpreter/DynamicLibraryManager.cpp

@@ -9,7 +9,6 @@
 
 #include "cling/Interpreter/DynamicLibraryManager.h"
 #include "cling/Interpreter/InterpreterCallbacks.h"
-#include "cling/Interpreter/InvocationOptions.h"
 #include "cling/Utils/Paths.h"
 #include "cling/Utils/Platform.h"
 #include "cling/Utils/Output.h"
@@ -22,8 +21,7 @@
 #include <sys/stat.h>
 
 namespace cling {
-  DynamicLibraryManager::DynamicLibraryManager(const InvocationOptions& Opts)
-    : m_Opts(Opts) {
+  DynamicLibraryManager::DynamicLibraryManager()  {
     const llvm::SmallVector<const char*, 10> kSysLibraryEnv = {
       "LD_LIBRARY_PATH",
   #if __APPLE__
@@ -43,38 +41,28 @@ namespace cling {
     // Behaviour is to not add paths that don't exist...In an interpreted env
     // does this make sense? Path could pop into existance at any time.
     for (const char* Var : kSysLibraryEnv) {
-      if (Opts.Verbose())
-        cling::log() << "Adding library paths from '" << Var << "':\n";
       if (const char* Env = ::getenv(Var)) {
         llvm::SmallVector<llvm::StringRef, 10> CurPaths;
-        SplitPaths(Env, CurPaths, utils::kPruneNonExistant, platform::kEnvDelim,
-                   Opts.Verbose());
+        SplitPaths(Env, CurPaths, utils::kPruneNonExistant, platform::kEnvDelim);
         for (const auto& Path : CurPaths)
-          m_SearchPaths.push_back({Path.str(), /*IsUser*/true});
+          addSearchPath(Path.str());
       }
     }
 
+    // $CWD is the last user path searched.
+    addSearchPath(".");
+
     llvm::SmallVector<std::string, 64> SysPaths;
     platform::GetSystemLibraryPaths(SysPaths);
 
     for (const std::string& P : SysPaths)
-      m_SearchPaths.push_back({P, /*IsUser*/false});
-
-    // This will currently be the last path searched, should it be pushed to
-    // the front of the line, or even to the front of user paths?
-    m_SearchPaths.push_back({".", /*IsUser*/true});
+      addSearchPath(P, /*IsUser*/ false);
   }
 
   std::string
   DynamicLibraryManager::lookupLibInPaths(llvm::StringRef libStem) const {
-    llvm::SmallVector<SearchPathInfo, 128> Paths;
-    for (const std::string &P : m_Opts.LibSearchPath)
-      Paths.push_back({P, /*IsUser*/true});
-
-    Paths.append(m_SearchPaths.begin(), m_SearchPaths.end());
-
     llvm::SmallString<512> ThisPath;
-    for (const SearchPathInfo& Info : Paths) {
+    for (const SearchPathInfo& Info : m_SearchPaths) {
       ThisPath = Info.Path;
       llvm::sys::path::append(ThisPath, libStem);
       bool exists;
@@ -242,6 +230,17 @@ namespace cling {
     return false;
   }
 
+  void DynamicLibraryManager::dump(llvm::raw_ostream* S /*= nullptr*/) const {
+    llvm::raw_ostream &OS = S ? *S : cling::outs();
+
+    // FIXME: print in a stable order the contents of m_SearchPaths
+    for (const auto& Info : getSearchPaths()) {
+      if (!Info.IsUser)
+        OS << "[system] ";
+      OS << Info.Path.c_str() << "\n";
+    }
+  }
+
   void DynamicLibraryManager::ExposeHiddenSharedLibrarySymbols(void* handle) {
     llvm::sys::DynamicLibrary::addPermanentLibrary(const_cast<void*>(handle));
   }

lib/Interpreter/DynamicLibraryManagerSymbol.cpp

@@ -870,7 +870,9 @@ namespace cling {
 
   void DynamicLibraryManager::initializeDyld(
                  std::function<bool(llvm::StringRef)> shouldPermanentlyIgnore) {
-    assert(!m_Dyld && "Already initialized!");
+     //assert(!m_Dyld && "Already initialized!");
+    if (m_Dyld)
+      delete m_Dyld;
 
     std::string exeP = GetExecutablePath();
     auto ObjF =

lib/Interpreter/IncrementalExecutor.cpp

@@ -12,8 +12,9 @@
 #include "IncrementalJIT.h"
 #include "Threading.h"
 
-#include "cling/Interpreter/Value.h"
+#include "cling/Interpreter/DynamicLibraryManager.h"
 #include "cling/Interpreter/Transaction.h"
+#include "cling/Interpreter/Value.h"
 #include "cling/Utils/AST.h"
 #include "cling/Utils/Output.h"
 #include "cling/Utils/Platform.h"
@@ -112,6 +113,7 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& /*diags*/,
   : m_Diags(diags)
 #endif
 {
+  m_DyLibManager.initializeDyld([](llvm::StringRef){/*ignore*/ return false;});
 
   // MSVC doesn't support m_AtExitFuncsSpinLock=ATOMIC_FLAG_INIT; in the class definition
   std::atomic_flag_clear( &m_AtExitFuncsSpinLock );
@@ -379,6 +381,11 @@ IncrementalExecutor::executeWrapper(llvm::StringRef function,
   return kExeSuccess;
 }
 
+void IncrementalExecutor::setCallbacks(InterpreterCallbacks* callbacks) {
+  m_Callbacks = callbacks;
+  m_DyLibManager.setCallbacks(callbacks);
+}
+
 void
 IncrementalExecutor::installLazyFunctionCreator(LazyFunctionCreatorFunc_t fp)
 {
@@ -466,6 +473,22 @@ bool IncrementalExecutor::diagnoseUnresolvedSymbols(llvm::StringRef trigger,
           << "Maybe you need to load the corresponding shared library?\n";
     }
 
+#ifdef __APPLE__
+    // The JIT gives us a mangled name which has only one leading underscore on
+    // all platforms, for instance _ZN8TRandom34RndmEv. However, on OSX the
+    // linker stores this symbol as __ZN8TRandom34RndmEv (adding an extra _).
+    assert(!llvm::StringRef(sym).startswith("__") && "Already added!");
+    std::string libName = m_DyLibManager.searchLibrariesForSymbol('_' + sym,
+                                                        /*searchSystem=*/ true);
+#else
+    std::string libName = m_DyLibManager.searchLibrariesForSymbol(sym,
+                                                        /*searchSystem=*/ true);
+#endif //__APPLE__
+    if (!libName.empty())
+      cling::errs() << "Symbol found in '" << libName << "';"
+                    << " did you mean to load it with '.L "
+                    << libName << "'?\n";
+
     //llvm::Function *ff = m_engine->FindFunctionNamed(i->c_str());
     // i could also reference a global variable, in which case ff == 0.
     //if (ff)

lib/Interpreter/IncrementalExecutor.h

@@ -15,6 +15,7 @@
 #include "BackendPasses.h"
 #include "EnterUserCodeRAII.h"
 
+#include "cling/Interpreter/DynamicLibraryManager.h"
 #include "cling/Interpreter/InterpreterCallbacks.h"
 #include "cling/Interpreter/Transaction.h"
 #include "cling/Interpreter/Value.h"
@@ -44,6 +45,7 @@ namespace llvm {
 }
 
 namespace cling {
+  class DynamicLibraryManager;
   class IncrementalJIT;
   class Value;
 
@@ -142,6 +144,11 @@ namespace cling {
     ///\brief The list of llvm::Module-s to return the transaction
     /// after the JIT has emitted them.
     std::map<llvm::orc::VModuleKey, Transaction*> m_PendingModules;
+
+    /// Dynamic library manager object.
+    ///
+    DynamicLibraryManager m_DyLibManager;
+
   public:
     enum ExecutionResult {
       kExeSuccess,
@@ -158,9 +165,15 @@ namespace cling {
     void setExternalIncrementalExecutor(IncrementalExecutor *extIncrExec) {
       m_externalIncrementalExecutor = extIncrExec;
     }
-    void setCallbacks(InterpreterCallbacks* callbacks) {
-      m_Callbacks = callbacks;
+    void setCallbacks(InterpreterCallbacks* callbacks);
+
+    const DynamicLibraryManager& getDynamicLibraryManager() const {
+      return const_cast<IncrementalExecutor*>(this)->m_DyLibManager;
     }
+    DynamicLibraryManager& getDynamicLibraryManager() {
+      return m_DyLibManager;
+    }
+
     void installLazyFunctionCreator(LazyFunctionCreatorFunc_t fp);
 
     ///\brief Unload a set of JIT symbols.

lib/Interpreter/Interpreter.cpp

@@ -162,8 +162,7 @@ namespace cling {
   bool Interpreter::isValid() const {
     // Should we also check m_IncrParser->getFirstTransaction() ?
     // not much can be done without it (its the initializing transaction)
-    return m_IncrParser && m_IncrParser->isValid() &&
-           m_DyLibManager && m_LookupHelper &&
+    return m_IncrParser && m_IncrParser->isValid() && m_LookupHelper &&
            (isInSyntaxOnlyMode() || m_Executor);
   }
 
@@ -212,7 +211,6 @@ namespace cling {
       return;
 
     m_LLVMContext.reset(new llvm::LLVMContext);
-    m_DyLibManager.reset(new DynamicLibraryManager(getOptions()));
     m_IncrParser.reset(new IncrementalParser(this, llvmdir, moduleExtensions));
     if (!m_IncrParser->isValid(false))
       return;
@@ -259,8 +257,12 @@ namespace cling {
 
     if (!isInSyntaxOnlyMode()) {
       m_Executor.reset(new IncrementalExecutor(SemaRef.Diags, *getCI()));
+
       if (!m_Executor)
         return;
+
+      for (const std::string &P : m_Opts.LibSearchPath)
+        getDynamicLibraryManager()->addSearchPath(P);
     }
 
     bool usingCxxModules = getSema().getLangOpts().Modules;
@@ -278,10 +280,10 @@ namespace cling {
     }
 
     if(m_Opts.CompilerOpts.CUDAHost){
-       if(m_DyLibManager->loadLibrary("libcudart.so", true) ==
-         cling::DynamicLibraryManager::LoadLibResult::kLoadLibNotFound){
-           llvm::errs() << "Error: libcudart.so not found!\n" <<
-             "Please add the cuda lib path to LD_LIBRARY_PATH or set it via -L argument.\n";
+      if (getDynamicLibraryManager()->loadLibrary("libcudart.so", true) ==
+          cling::DynamicLibraryManager::LoadLibResult::kLoadLibNotFound){
+        llvm::errs() << "Error: libcudart.so not found!\n" <<
+          "Please add the cuda lib path to LD_LIBRARY_PATH or set it via -L argument.\n";
        }
     }
 
@@ -644,11 +646,17 @@ namespace cling {
     return AddIncludePaths(PathsStr, nullptr);
   }
 
-  void Interpreter::DumpIncludePath(llvm::raw_ostream* S) {
-    utils::DumpIncludePaths(getCI()->getHeaderSearchOpts(), S ? *S : cling::outs(),
+  void Interpreter::DumpIncludePath(llvm::raw_ostream* S) const {
+    utils::DumpIncludePaths(getCI()->getHeaderSearchOpts(),
+                            S ? *S : cling::outs(),
                             true /*withSystem*/, true /*withFlags*/);
   }
 
+  void Interpreter::DumpDynamicLibraryInfo(llvm::raw_ostream* S) const {
+    if (auto DLM = getDynamicLibraryManager())
+      DLM->dump(S);
+  }
+
   // FIXME: Add stream argument and move DumpIncludePath path here.
   void Interpreter::dump(llvm::StringRef what, llvm::StringRef filter) {
     llvm::raw_ostream &where = cling::log();
@@ -1624,9 +1632,6 @@ namespace cling {
     // We need it to enable LookupObject callback.
     if (!m_Callbacks) {
       m_Callbacks.reset(new MultiplexInterpreterCallbacks(this));
-      // FIXME: Move to the InterpreterCallbacks.cpp;
-      if (DynamicLibraryManager* DLM = getDynamicLibraryManager())
-        DLM->setCallbacks(m_Callbacks.get());
       if (m_Executor)
         m_Executor->setCallbacks(m_Callbacks.get());
     }
@@ -1635,6 +1640,15 @@ namespace cling {
       ->addCallback(std::move(C));
   }
 
+  const DynamicLibraryManager* Interpreter::getDynamicLibraryManager() const {
+    assert(m_Executor.get() && "We must have an executor");
+    return &m_Executor->getDynamicLibraryManager();
+  }
+  DynamicLibraryManager* Interpreter::getDynamicLibraryManager() {
+    return const_cast<DynamicLibraryManager*>(const_cast<const Interpreter*>(
+                                             this)->getDynamicLibraryManager());
+  }
+
   const Transaction* Interpreter::getFirstTransaction() const {
     return m_IncrParser->getFirstTransaction();
   }

lib/MetaProcessor/MetaParser.cpp

@@ -133,21 +133,22 @@ namespace cling {
   // FilePath := AnyString
   // AnyString := .*^('\t' Comment)
   bool MetaParser::isLCommand(MetaSema::ActionResult& actionResult) {
-    bool result = false;
     if (getCurTok().is(tok::ident) && getCurTok().getIdent().equals("L")) {
       consumeAnyStringToken(tok::comment);
+      llvm::StringRef filePath;
       if (getCurTok().is(tok::raw_ident)) {
-        result = true;
-        actionResult = m_Actions.actOnLCommand(getCurTok().getIdent());
+        filePath = getCurTok().getIdent();
         consumeToken();
         if (getCurTok().is(tok::comment)) {
           consumeAnyStringToken(tok::eof);
           m_Actions.actOnComment(getCurTok().getIdent());
         }
       }
+      actionResult = m_Actions.actOnLCommand(filePath);
+      return true;
     }
     // TODO: Some fine grained diagnostics
-    return result;
+    return false;
   }
 
   // T := 'T' FilePath Comment

lib/MetaProcessor/MetaSema.cpp

@@ -41,6 +41,11 @@ namespace cling {
 
   MetaSema::ActionResult MetaSema::actOnLCommand(llvm::StringRef file,
                                              Transaction** transaction /*= 0*/){
+    if (file.empty()) {
+      m_Interpreter.DumpDynamicLibraryInfo();
+      return AR_Success;
+    }
+
     ActionResult result = actOnUCommand(file);
     if (result != AR_Success)
       return result;

test/LibraryCall/library_path.C

@@ -0,0 +1,15 @@
+//------------------------------------------------------------------------------
+// CLING - the C++ LLVM-based InterpreterG :)
+//
+// This file is dual-licensed: you can choose to license it under the University
+// of Illinois Open Source License or the GNU Lesser General Public License. See
+// LICENSE.TXT for details.
+//------------------------------------------------------------------------------
+
+// REQUIRES: shell
+// RUN: env -i LD_LIBRARY_PATH="../:./../" %cling ".L" | FileCheck %s
+
+// CHECK: ../
+// CHECK-NEXT: ./../
+// CHECK-NEXT: .
+// CHECK-NEXT: [system]