wallace updated this revision to Diff 254268.
wallace added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77186/new/
https://reviews.llvm.org/D77186
Files:
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
Index: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
===================================================================
--- lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
@@ -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 @@
@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 @@
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")
-
- # 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...")
+ invalid_path = src_dir + "invalid_path"
+ invalid_path2 = src_dir + "invalid_path2"
+
+ # 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)
Index: lldb/source/Interpreter/OptionValuePathMappings.cpp
===================================================================
--- lldb/source/Interpreter/OptionValuePathMappings.cpp
+++ lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -61,7 +61,7 @@
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 @@
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 @@
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 @@
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 @@
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;
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
