Make DWARF at_comp_dir symbolic links configurable via plugin.symbol-file.dwarf.comp-dir-symlink-paths setting.

[ovyalov]

Summary

Make DWARF at_comp_dir symbolic links configurable via newly added plugin.symbol-file.dwarf.comp-dir-symlink-paths setting.
By default, plugin.symbol-file.dwarf.comp-dir-symlink-paths is empty and can be customized by debug session.
Tests don't support Windows since symbolic links on Windows require special permissions' treatment.

[clayborg]

We need a new "symbol-file" plug-in category. Copy how we did it for "process" and add one for "symbol-file". The setting should be "plugin.symbol-file.dwarf.symlink-paths". See inlined comments.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

118

Is this limited only the DW_AT_comp_dir? Then we should make the setting title clearer. Should this be "comp-dir-symlink-paths"? Then the description should be a bit more verbose: "If the DW_AT_comp_dir matches any of these paths the symbolic links will be resolved at DWARF parse time."

341–344

We need a new SymbolFile based version of PluginManager::CreateSettingForProcessPlugin():

PluginManager::CreateSettingForSymbolFilePlugin(...)

Please create this function and have the setting name be "symbol-file".

[ovyalov]

Addressed review comments.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

341–344

My bad - copied from ProcessGDBRemote. Fixed.

[clayborg]

Do you want a default value for this, or are people going to manually set this setting? Looks good though.

[ovyalov]

In D11586#214427, @clayborg wrote:

Do you want a default value for this, or are people going to manually set this setting? Looks good though.

I propose to leave it empty by default to let users specify it manually when needed.

Thank you for the review.

[ovyalov]

Files:

/lldb/trunk/include/lldb/Core/PluginManager.h
/lldb/trunk/source/Core/PluginManager.cpp
/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
/lldb/trunk/test/functionalities/breakpoint/comp_dir_symlink/
/lldb/trunk/test/functionalities/breakpoint/comp_dir_symlink/Makefile
/lldb/trunk/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
/lldb/trunk/test/functionalities/breakpoint/comp_dir_symlink/main.cpp
/lldb/trunk/test/lldbtest.py

Users:

ovyalov (Author)

http://reviews.llvm.org/rL243580

include/lldb/Core/PluginManager.h

public:
     GetProcessPluginDescriptionAtIndex (uint32_t idx);
 
     //------------------------------------------------------------------
     // SymbolFile
     //------------------------------------------------------------------
     static bool
     RegisterPlugin (const ConstString &name,
                     const char *description,
-                    SymbolFileCreateInstance create_callback);
+                    SymbolFileCreateInstance create_callback,
+                    DebuggerInitializeCallback debugger_init_callback = nullptr);
 
     static bool
     UnregisterPlugin (SymbolFileCreateInstance create_callback);
 
     static SymbolFileCreateInstance
     GetSymbolFileCreateCallbackAtIndex (uint32_t idx);
 
     static SymbolFileCreateInstance

source/Core/PluginManager.cpp

 
 #pragma mark SymbolFile
 
 struct SymbolFileInstance
 {
     SymbolFileInstance() :
         name(),
         description(),
-        create_callback(NULL)
+        create_callback(nullptr),
+        debugger_init_callback(nullptr)
     {
     }
 
     ConstString name;
     std::string description;
     SymbolFileCreateInstance create_callback;
+    DebuggerInitializeCallback debugger_init_callback;
 };
 
 typedef std::vector<SymbolFileInstance> SymbolFileInstances;
 
 static Mutex &
 GetSymbolFileMutex ()
 {
     static Mutex g_instances_mutex (Mutex::eMutexTypeRecursive);
     return g_instances_mutex;
 }
 
 static SymbolFileInstances &
 GetSymbolFileInstances ()
 {
     static SymbolFileInstances g_instances;
     return g_instances;
 }
 
 
 bool
 PluginManager::RegisterPlugin
 (
     const ConstString &name,
     const char *description,
-    SymbolFileCreateInstance create_callback
+    SymbolFileCreateInstance create_callback,
+    DebuggerInitializeCallback debugger_init_callback
 )
 {
     if (create_callback)
     {
         SymbolFileInstance instance;
         assert ((bool)name);
         instance.name = name;
         if (description && description[0])
             instance.description = description;
         instance.create_callback = create_callback;
+        instance.debugger_init_callback = debugger_init_callback;
         Mutex::Locker locker (GetSymbolFileMutex ());
         GetSymbolFileInstances ().push_back (instance);
     }
     return false;
 }
 
 bool
 PluginManager::UnregisterPlugin (SymbolFileCreateInstance create_callback)
// Initialize the Platform plugins
     
         PlatformInstances::iterator pos, end = instances.end();
         for (pos = instances.begin(); pos != end; ++ pos)
         {
             if (pos->debugger_init_callback)
                 pos->debugger_init_callback (debugger);
         }
     }
-    
     // Initialize the Process plugins
     {
         Mutex::Locker locker (GetProcessMutex());
         ProcessInstances &instances = GetProcessInstances();
         
         ProcessInstances::iterator pos, end = instances.end();
         for (pos = instances.begin(); pos != end; ++ pos)
         {
             if (pos->debugger_init_callback)
                 pos->debugger_init_callback (debugger);
         }
     }
 
+    // Initialize the SymbolFile plugins
+    {
+        Mutex::Locker locker (GetSymbolFileMutex());
+        for (auto& sym_file: GetSymbolFileInstances())
+        {
+            if (sym_file.debugger_init_callback)
+                sym_file.debugger_init_callback (debugger);
+        }
+    }
 }
 
 // This is the preferred new way to register plugin specific settings.  e.g.
 // This will put a plugin's settings under e.g. "plugin.<plugin_type_name>.<plugin_type_desc>.SETTINGNAME".
 static lldb::OptionValuePropertiesSP
 GetDebuggerPropertyForPlugins (Debugger &debugger,
                                        const ConstString &plugin_type_name,
                                        const ConstString &plugin_type_desc,

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

public:
     // Static Functions
     //------------------------------------------------------------------
     static void
     Initialize();
 
     static void
     Terminate();
 
+    static void
+    DebuggerInitialize(lldb_private::Debugger &debugger);
+
     static lldb_private::ConstString
     GetPluginNameStatic();
 
     static const char *
     GetPluginDescriptionStatic();
 
     static lldb_private::SymbolFile*
     CreateInstance (lldb_private::ObjectFile* obj_file);

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 #include "lldb/Core/Timer.h"
 #include "lldb/Core/Value.h"
 
 #include "lldb/Expression/ClangModulesDeclVendor.h"
 
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 
+#include "lldb/Interpreter/OptionValueFileSpecList.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
+
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/ClangExternalASTSourceCallbacks.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/VariableList.h"
 

 //    case DW_TAG_class_type:
 //    case DW_TAG_structure_type:
 //    case DW_TAG_union_type:
 //        return true;
 //    }
 //    return false;
 //}
 //
+
+namespace {
+
+    PropertyDefinition
+    g_properties[] =
+    {
+        { "symlink-paths" , OptionValue::eTypeFileSpecList, true,  0 ,   nullptr, nullptr, "DWARF AT_comp_dir symbolic links." },
+        {  nullptr        , OptionValue::eTypeInvalid     , false, 0,    nullptr, nullptr, nullptr }
+    };
+
+    enum
+    {
+        ePropertySymLinkPaths
+    };
+
+
+    class PluginProperties : public Properties
+    {
+    public:
+        static ConstString
+        GetSettingName()
+        {
+            return SymbolFileDWARF::GetPluginNameStatic();
+        }
+
+        PluginProperties()
+        {
+            m_collection_sp.reset (new OptionValueProperties(GetSettingName()));
+            m_collection_sp->Initialize(g_properties);
+        }
+
+        FileSpecList&
+        GetSymLinkPaths()
+        {
+            OptionValueFileSpecList *option_value = m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr, true, ePropertySymLinkPaths);
+            assert(option_value);
+            return option_value->GetCurrentValue();
+        }
+
+    };
+
+    typedef std::shared_ptr<PluginProperties> SymbolFileDWARFPropertiesSP;
+
+    static const SymbolFileDWARFPropertiesSP&
+    GetGlobalPluginProperties()
+    {
+        static const auto g_settings_sp(std::make_shared<PluginProperties>());
+        return g_settings_sp;
+    }
+
+}  // anonymous namespace end
+
+
 static AccessType
 DW_ACCESS_to_AccessType (uint32_t dwarf_accessibility)
 {
     switch (dwarf_accessibility)
     {
         case DW_ACCESS_public:      return eAccessPublic;
         case DW_ACCESS_private:     return eAccessPrivate;
         case DW_ACCESS_protected:   return eAccessProtected;
if (
         '\\' == path_from_dwarf[2])
     {
         return path_from_dwarf;
     }
     
     return colon_pos + 1;
 }
 
-// DW_AT_comp_dir can be overridden by setting compiler's PWD environment
-// variable - for example, set to "/proc/self/cwd" in order to cope with
-// distributed builds.
-static const char* comp_dir_symlinks[] = {"/proc/self/cwd"};
-
 static const char*
 resolveCompDir(const char* path_from_dwarf)
 {
     if (!path_from_dwarf)
         return nullptr;
 
     // DWARF2/3 suggests the form hostname:pathname for compilation directory.
     // Remove the host part if present.
     const char* local_path = removeHostnameFromPathname(path_from_dwarf);
     if (!local_path)
         return nullptr;
 
     bool is_symlink = false;
-    for (unsigned long i = 0; i < sizeof(comp_dir_symlinks)/sizeof(comp_dir_symlinks[0]) && !is_symlink; ++i)
-        is_symlink = !strcmp(comp_dir_symlinks[i], local_path);
+    FileSpec local_path_spec(local_path, false);
+    const auto& file_specs = GetGlobalPluginProperties()->GetSymLinkPaths();
+    for (size_t i = 0; i < file_specs.GetSize() && !is_symlink; ++i)
+        is_symlink = FileSpec::Equal(file_specs.GetFileSpecAtIndex(i), local_path_spec, true);
 
     if (!is_symlink)
         return local_path;
 
-    const FileSpec local_path_spec(local_path, true);
     if (!local_path_spec.IsSymbolicLink())
         return local_path;
 
     FileSpec resolved_local_path_spec;
     const auto error = FileSystem::Readlink(local_path_spec, resolved_local_path_spec);
     if (error.Success())
         return resolved_local_path_spec.GetCString();
 

 #endif
 
 void
 SymbolFileDWARF::Initialize()
 {
     LogChannelDWARF::Initialize();
     PluginManager::RegisterPlugin (GetPluginNameStatic(),
                                    GetPluginDescriptionStatic(),
-                                   CreateInstance);
+                                   CreateInstance,
+                                   DebuggerInitialize);
+}
+
+void
+SymbolFileDWARF::DebuggerInitialize(Debugger &debugger)
+{
+    if (!PluginManager::GetSettingForProcessPlugin(debugger, PluginProperties::GetSettingName()))
+    {
+        const bool is_global_setting = true;
+        PluginManager::CreateSettingForProcessPlugin(debugger,
+                                                     GetGlobalPluginProperties()->GetValueProperties(),
+                                                     ConstString ("Properties for the dwarf symbol file plug-in."),
+                                                     is_global_setting);
+    }
 }
 
 void
 SymbolFileDWARF::Terminate()
 {
     PluginManager::UnregisterPlugin (CreateInstance);
     LogChannelDWARF::Initialize();
 }

test/functionalities/breakpoint/comp_dir_symlink/Makefile

+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+
+EXE := out/CompDirSymLink
+
+include $(LEVEL)/Makefile.rules

test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py

+"""
+Test breakpoint command with AT_comp_dir set to symbolic link.
+"""
+import os
+import unittest2
+import lldb
+from lldbtest import *
+import lldbutil
+import shutil
+
+
+_EXE_NAME = 'out/CompDirSymLink'  # Must match Makefile
+_SRC_FILE = 'main.cpp'
+
+class CompDirSymLinkTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line number to break inside main().
+        self.line = line_number(_SRC_FILE, '// Set break point at this line.')
+        self.src_path = os.path.join(os.getcwd(), _SRC_FILE)
+
+        self.out_dir = os.path.join(os.getcwd(), 'out')
+        os.mkdir(self.out_dir)
+        self.addTearDownHook(lambda: shutil.rmtree(self.out_dir))
+
+    @dwarf_test
+    @skipIfHostWindows
+    def test_symlink_paths_set(self):
+        pwd_symlink = self.create_src_symlink()
+        self.build(pwd_symlink)
+        self.runCmd('settings set plugin.process.dwarf.symlink-paths ' + pwd_symlink)
+        lldbutil.run_break_set_by_file_and_line(self, self.src_path, self.line)
+
+    @dwarf_test
+    @skipUnlessHostLinux
+    def test_symlink_paths_set_procselfcwd(self):
+        pwd_symlink = '/proc/self/cwd'
+        self.build(pwd_symlink)
+        self.runCmd('settings set plugin.process.dwarf.symlink-paths ' + pwd_symlink)
+        lldbutil.run_break_set_by_file_and_line(self, self.src_path, self.line)
+
+    @dwarf_test
+    @skipIfHostWindows
+    def test_symlink_paths_unset(self):
+        pwd_symlink = self.create_src_symlink()
+        self.build(pwd_symlink)
+        self.runCmd('settings clear plugin.process.dwarf.symlink-paths')
+        self.assertRaises(AssertionError, lldbutil.run_break_set_by_file_and_line, self, self.src_path, self.line)
+
+    def create_src_symlink(self):
+        pwd_symlink = os.path.join(os.getcwd(), 'pwd_symlink')
+        os.symlink(os.getcwd(), pwd_symlink)
+        self.addTearDownHook(lambda: os.remove(pwd_symlink))
+        return pwd_symlink
+
+    def build(self, pwd_symlink):
+        self.buildDwarf(None, None, {'PWD': pwd_symlink}, True)
+
+        exe = os.path.join(os.getcwd(), _EXE_NAME)
+        self.runCmd('file ' + exe, CURRENT_EXECUTABLE_SET)
+
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

test/functionalities/breakpoint/comp_dir_symlink/main.cpp

+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+int main (int argc, char const *argv[])
+{
+    return 0; // Set break point at this line.
+}

test/lldbtest.py

 def skipIfDarwin(func):
     """Decorate the item to skip tests that should be skipped on Darwin."""
     return skipIfPlatform(getDarwinOSTriples())(func)
 
 def skipIfLinux(func):
     """Decorate the item to skip tests that should be skipped on Linux."""
     return skipIfPlatform(["linux"])(func)
 
+def skipUnlessHostLinux(func):
+    """Decorate the item to skip tests that should be skipped on any non Linux host."""
+    return skipUnlessHostPlatform(["linux"])(func)
+
 def skipIfWindows(func):
     """Decorate the item to skip tests that should be skipped on Windows."""
     return skipIfPlatform(["windows"])(func)
 
 def skipIfHostWindows(func):
     """Decorate the item to skip tests that should be skipped on Windows."""
     return skipIfHostPlatform(["windows"])(func)