Author: Walter Erquinigo Date: 2021-07-23T17:53:12-07:00 New Revision: ef8c6849a235e97b8b981e0f998d430fdbd7bc2a
URL: https://github.com/llvm/llvm-project/commit/ef8c6849a235e97b8b981e0f998d430fdbd7bc2a DIFF: https://github.com/llvm/llvm-project/commit/ef8c6849a235e97b8b981e0f998d430fdbd7bc2a.diff LOG: [source maps] fix source mapping when there are multiple matching rules D104406 introduced an error in which, if there are multiple matchings rules for a given path, lldb was only checking for the validity in the filesystem of the first match instead of looking exhaustively one by one until a valid file is found. Besides that, a call to consume_front was being done incorrectly, as it was modifying the input, which renders subsequent matches incorrect. I added a test that checks for both cases. Differential Revision: https://reviews.llvm.org/D106723 Added: Modified: lldb/include/lldb/Target/PathMappingList.h lldb/source/Target/PathMappingList.cpp lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h index 46d7a427d3071..d788d120c47e9 100644 --- a/lldb/include/lldb/Target/PathMappingList.h +++ b/lldb/include/lldb/Target/PathMappingList.h @@ -72,9 +72,19 @@ class PathMappingList { /// \param[in] path /// The original source file path to try and remap. /// + /// \param[in] only_if_exists + /// If \b true, besides matching \p path with the remapping rules, this + /// tries to check with the filesystem that the remapped file exists. If + /// no valid file is found, \b None is returned. This might be expensive, + /// specially on a network. + /// + /// If \b false, then the existence of the returned remapping is not + /// checked. + /// /// \return /// The remapped filespec that may or may not exist on disk. - llvm::Optional<FileSpec> RemapPath(llvm::StringRef path) const; + llvm::Optional<FileSpec> RemapPath(llvm::StringRef path, + bool only_if_exists = false) const; bool RemapPath(const char *, std::string &) const = delete; bool ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const; diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp index 86b0ad5f933ff..b660c310ef31a 100644 --- a/lldb/source/Target/PathMappingList.cpp +++ b/lldb/source/Target/PathMappingList.cpp @@ -165,12 +165,17 @@ static void AppendPathComponents(FileSpec &path, llvm::StringRef components, } llvm::Optional<FileSpec> -PathMappingList::RemapPath(llvm::StringRef path) const { - if (m_pairs.empty() || path.empty()) +PathMappingList::RemapPath(llvm::StringRef mapping_path, + bool only_if_exists) const { + if (m_pairs.empty() || mapping_path.empty()) return {}; LazyBool path_is_relative = eLazyBoolCalculate; + for (const auto &it : m_pairs) { - auto prefix = it.first.GetStringRef(); + llvm::StringRef prefix = it.first.GetStringRef(); + // We create a copy of mapping_path because StringRef::consume_from + // effectively modifies the instance itself. + llvm::StringRef path = mapping_path; if (!path.consume_front(prefix)) { // Relative paths won't have a leading "./" in them unless "." is the // only thing in the relative path so we need to work around "." @@ -190,7 +195,8 @@ PathMappingList::RemapPath(llvm::StringRef path) const { auto orig_style = FileSpec::GuessPathStyle(prefix).getValueOr( llvm::sys::path::Style::native); AppendPathComponents(remapped, path, orig_style); - return remapped; + if (!only_if_exists || FileSystem::Instance().Exists(remapped)) + return remapped; } return {}; } @@ -211,11 +217,9 @@ bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) co return false; } - llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) const { - if (auto remapped = RemapPath(orig_spec.GetPath())) - if (FileSystem::Instance().Exists(*remapped)) - return remapped; + if (auto remapped = RemapPath(orig_spec.GetPath(), /*only_if_exists=*/true)) + return remapped; return {}; } diff --git a/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py b/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py index 5c129cb91825a..626a6536d6b96 100644 --- a/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py @@ -26,6 +26,12 @@ def setUp(self): @skipIfWindows @skipIfRemote def test_source_map(self): + """ + This test simulates building two files in a folder, and then moving + each source to a diff erent folder. Then, the debug session is started + with the corresponding source maps to have breakpoints and frames + working. + """ self.build_and_create_debug_adaptor() other_basename = 'other-copy.c' @@ -87,6 +93,16 @@ def test_source_map(self): self.assertEqual(other_basename, breakpoint['source']['name']) self.assertEqual(new_other_path, breakpoint['source']['path']) + # now we check the stack trace making sure that we got mapped source paths + frames = self.vscode.request_stackTrace()['body']['stackFrames'] + + self.assertEqual(frames[0]['source']['name'], other_basename) + self.assertEqual(frames[0]['source']['path'], new_other_path) + + self.assertEqual(frames[1]['source']['name'], self.main_basename) + self.assertEqual(frames[1]['source']['path'], new_main_path) + + @skipIfWindows @skipIfRemote def test_set_and_clear(self): _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
