[source maps] Ensure all valid source maps are added instead of failing with the first invalid one

Summary:
Several lldb-vscode users have noticed that when a source map rule is invalid (because a folder doesn't exist anymore), the rest of the source maps from their configurations are not applied.
This happens because lldb-vscode executes a single "settings set target.source-map" command with all the source maps and LLDB processes them one by one until one fails.

Instead of doing this, we can process in LLDB all the source map rules and apply the valid ones instead of failing fast.

Reviewers: clayborg, labath, kusmour, aadsm

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D77186
This commit is contained in:
Walter Erquinigo 2020-03-31 17:08:40 -07:00
parent e65e677ee4
commit 30350c2541
2 changed files with 94 additions and 19 deletions

View File

@ -61,7 +61,7 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
count);
} else {
bool changed = false;
for (size_t i = 1; i < argc; i += 2, ++idx) {
for (size_t i = 1; i < argc; i += 2) {
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
@ -70,9 +70,13 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
if (!m_path_mappings.Replace(a, b, idx, m_notify_changes))
m_path_mappings.Append(a, b, m_notify_changes);
changed = true;
idx++;
} else {
std::string previousError =
error.Fail() ? std::string(error.AsCString()) + "\n" : "";
error.SetErrorStringWithFormat(
"the replacement path doesn't exist: \"%s\"", replace_path);
"%sthe replacement path doesn't exist: \"%s\"",
previousError.c_str(), replace_path);
break;
}
}
@ -109,9 +113,11 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
m_value_was_set = true;
changed = true;
} else {
std::string previousError =
error.Fail() ? std::string(error.AsCString()) + "\n" : "";
error.SetErrorStringWithFormat(
"the replacement path doesn't exist: \"%s\"", replace_path);
break;
"%sthe replacement path doesn't exist: \"%s\"",
previousError.c_str(), replace_path);
}
}
if (changed)
@ -135,7 +141,7 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
bool changed = false;
if (op == eVarSetOperationInsertAfter)
++idx;
for (size_t i = 1; i < argc; i += 2, ++idx) {
for (size_t i = 1; i < argc; i += 2) {
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
@ -143,9 +149,13 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
ConstString b(replace_path);
m_path_mappings.Insert(a, b, idx, m_notify_changes);
changed = true;
idx++;
} else {
std::string previousError =
error.Fail() ? std::string(error.AsCString()) + "\n" : "";
error.SetErrorStringWithFormat(
"the replacement path doesn't exist: \"%s\"", replace_path);
"%sthe replacement path doesn't exist: \"%s\"",
previousError.c_str(), replace_path);
break;
}
}

View File

@ -1,6 +1,7 @@
import lldb
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
import os
class TestTargetSourceMap(TestBase):
@ -10,6 +11,21 @@ class TestTargetSourceMap(TestBase):
@no_debug_info_test
def test_source_map(self):
"""Test target.source-map' functionality."""
def assertBreakpointWithSourceMap(src_path):
# Set a breakpoint after we remap source and verify that it succeeds
bp = target.BreakpointCreateByLocation(src_path, 2)
self.assertEquals(bp.GetNumLocations(), 1,
"make sure breakpoint was resolved with map")
# Now make sure that we can actually FIND the source file using this
# remapping:
retval = lldb.SBCommandReturnObject()
self.dbg.GetCommandInterpreter().HandleCommand("source list -f main.c -l 2", retval)
self.assertTrue(retval.Succeeded(), "source list didn't succeed.")
self.assertNotEqual(retval.GetOutput(), None, "We got no ouput from source list")
self.assertTrue("return" in retval.GetOutput(), "We didn't find the source file...")
# Set the target soure map to map "./" to the current test directory
src_dir = self.getSourceDir()
src_path = os.path.join(src_dir, "main.c")
@ -25,19 +41,68 @@ class TestTargetSourceMap(TestBase):
bp = target.BreakpointCreateByLocation(src_path, 2)
self.assertEquals(bp.GetNumLocations(), 0,
"make sure no breakpoints were resolved without map")
src_map_cmd = 'settings set target.source-map . "%s"' % (src_dir)
self.dbg.HandleCommand(src_map_cmd)
# Set a breakpoint after we remap source and verify that it succeeds
bp = target.BreakpointCreateByLocation(src_path, 2)
self.assertEquals(bp.GetNumLocations(), 1,
"make sure breakpoint was resolved with map")
invalid_path = src_dir + "invalid_path"
invalid_path2 = src_dir + "invalid_path2"
# Now make sure that we can actually FIND the source file using this
# remapping:
retval = lldb.SBCommandReturnObject()
self.dbg.GetCommandInterpreter().HandleCommand("source list -f main.c -l 2", retval)
self.assertTrue(retval.Succeeded(), "source list didn't succeed.")
self.assertNotEqual(retval.GetOutput(), None, "We got no ouput from source list")
self.assertTrue("return" in retval.GetOutput(), "We didn't find the source file...")
# We make sure the error message contains all the invalid paths
self.expect(
'settings set target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2),
substrs=[
'the replacement path doesn\'t exist: "%s"' % (invalid_path),
'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
],
error=True,
)
self.expect(
'settings show target.source-map',
substrs=['[0] "." -> "%s"' % (src_dir)],
)
assertBreakpointWithSourceMap(src_path)
# Index 0 is the valid mapping, and modifying it to an invalid one should have no effect
self.expect(
'settings replace target.source-map 0 . "%s"' % (invalid_path),
substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
error=True,
)
self.expect(
'settings show target.source-map',
substrs=['[0] "." -> "%s"' % (src_dir)]
)
assertBreakpointWithSourceMap(src_path)
# Let's clear and add the mapping in with insert-after
self.runCmd('settings remove target.source-map 0')
self.expect(
'settings show target.source-map',
endstr="target.source-map (path-map) =\n",
)
# We add a valid but useless mapping so that we can use insert-after
another_valid_path = os.path.dirname(src_dir)
self.runCmd('settings set target.source-map . "%s"' % (another_valid_path))
self.expect(
'settings replace target.source-map 0 . "%s"' % (invalid_path),
substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
error=True,
)
self.expect(
'settings show target.source-map',
substrs=['[0] "." -> "%s"' % (another_valid_path)]
)
# Let's clear and add the mapping in with append
self.expect('settings remove target.source-map 0')
self.expect(
'settings show target.source-map',
endstr="target.source-map (path-map) =\n",
)
self.expect(
'settings append target.source-map . "%s" . "%s"' % (invalid_path, src_dir),
substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
error=True,
)
assertBreakpointWithSourceMap(src_path)