diff --git a/lldb/source/Interpreter/OptionValuePathMappings.cpp b/lldb/source/Interpreter/OptionValuePathMappings.cpp --- a/lldb/source/Interpreter/OptionValuePathMappings.cpp +++ b/lldb/source/Interpreter/OptionValuePathMappings.cpp @@ -61,7 +61,7 @@ count); } else { bool changed = false; - for (size_t i = 1; i < argc; i += 2) { + for (size_t i = 1; i < argc; idx++, i += 2) { const char *orginal_path = args.GetArgumentAtIndex(i); const char *replace_path = args.GetArgumentAtIndex(i + 1); if (VerifyPathExists(replace_path)) { @@ -70,14 +70,12 @@ 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( "%sthe replacement path doesn't exist: \"%s\"", previousError.c_str(), replace_path); - break; } } if (changed) @@ -156,7 +154,6 @@ error.SetErrorStringWithFormat( "%sthe replacement path doesn't exist: \"%s\"", previousError.c_str(), replace_path); - break; } } if (changed) @@ -171,32 +168,22 @@ case eVarSetOperationRemove: if (argc > 0) { std::vector remove_indexes; - bool all_indexes_valid = true; - size_t i; - for (i = 0; all_indexes_valid && i < argc; ++i) { - const int idx = - StringConvert::ToSInt32(args.GetArgumentAtIndex(i), INT32_MAX); - if (idx == INT32_MAX) - all_indexes_valid = false; - else + for (size_t i = 0; i < argc; ++i) { + int idx = StringConvert::ToSInt32(args.GetArgumentAtIndex(i), INT32_MAX); + if (idx < 0 || idx >= (int)m_path_mappings.GetSize()) { + error.SetErrorStringWithFormat( + "invalid array index '%s', aborting remove operation", + args.GetArgumentAtIndex(i)); + break; + } else remove_indexes.push_back(idx); } - if (all_indexes_valid) { - size_t num_remove_indexes = remove_indexes.size(); - if (num_remove_indexes) { - // Sort and then erase in reverse so indexes are always valid - llvm::sort(remove_indexes.begin(), remove_indexes.end()); - for (size_t j = num_remove_indexes - 1; j < num_remove_indexes; ++j) { - m_path_mappings.Remove(j, m_notify_changes); - } - } - NotifyValueChanged(); - } else { - error.SetErrorStringWithFormat( - "invalid array index '%s', aborting remove operation", - args.GetArgumentAtIndex(i)); - } + // Sort and then erase in reverse so indexes are always valid + llvm::sort(remove_indexes.rbegin(), remove_indexes.rend()); + for (auto index : remove_indexes) + m_path_mappings.Remove(index, m_notify_changes); + NotifyValueChanged(); } else { error.SetErrorString("remove operation takes one or more array index"); } diff --git a/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py b/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py --- a/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py +++ b/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py @@ -42,67 +42,93 @@ self.assertEquals(bp.GetNumLocations(), 0, "make sure no breakpoints were resolved without map") + valid_path = os.path.dirname(src_dir) + valid_path2 = os.path.dirname(valid_path) 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), + 'settings set target.source-map . "%s" . "%s" . "%s" . "%s' \ + % (invalid_path, src_dir, invalid_path2, valid_path), substrs=[ - 'the replacement path doesn\'t exist: "%s"' % (invalid_path), + 'error: 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)], + substrs=[ + '[0] "." -> "%s"' % (src_dir), + '[1] "." -> "%s"' % (valid_path), + ], ) assertBreakpointWithSourceMap(src_path) - # Index 0 is the valid mapping, and modifying it to an invalid one should have no effect + # Attempts to replace an index to an invalid mapping should have no effect. + # Modifications to valid mappins should work. self.expect( - 'settings replace target.source-map 0 . "%s"' % (invalid_path), - substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)], + 'settings replace target.source-map 0 . "%s" . "%s"' % (invalid_path, valid_path2), + 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)] + substrs=[ + '[0] "." -> "%s"' % (src_dir), + '[1] "." -> "%s"' % (valid_path2), + ] ) assertBreakpointWithSourceMap(src_path) - # Let's clear and add the mapping in with insert-after + # Let's clear and add the mapping back 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", + substrs=['[0] "." -> "%s"' % (valid_path2)], ) - - # 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)], + 'settings insert-after target.source-map 0 . "%s" . "%s" . "%s"' \ + % (invalid_path, invalid_path2, src_dir), + substrs=[ + 'error: 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"' % (another_valid_path)] + substrs=[ + '[0] "." -> "%s"' % (valid_path2), + '[1] "." -> "%s"' % (src_dir), + ] ) - # Let's clear and add the mapping in with append - self.expect('settings remove target.source-map 0') + # Let's clear using remove and add the mapping in with append + self.runCmd('settings remove target.source-map 1') self.expect( 'settings show target.source-map', - endstr="target.source-map (path-map) =\n", + substrs=[ + '[0] "." -> "%s"' % (valid_path2), + ] ) - + self.runCmd('settings clear target.source-map') self.expect( - 'settings append target.source-map . "%s" . "%s"' % (invalid_path, src_dir), - substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)], + 'settings append target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2), + substrs=[ + 'error: 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)