[source maps] fix source mapping when there are multiple matching rules

[wallace]

Summary

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.

[wallace]

Fixed another bug and added tests

[clayborg]

lldb/include/lldb/Target/PathMappingList.h

75

"only_if_exists" be a better name for "check_filesystem"

lldb/source/Target/PathMappingList.cpp

168

Would "only_if_exists" be a better name for "check_filesystem"?

217

"only_if_exists" be a better name for "check_filesystem

[wallace]

As a note, the test is an lldb-vscode test, because I already have a set up for source maps in this kind of scenario, from breakpoints to frames, so it was easy.

[wallace]

improve the variable name

[wallace]

last nit

lldb/include/lldb/Target/PathMappingList.h

 //===-- PathMappingList.h ---------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_TARGET_PATHMAPPINGLIST_H
 #define LLDB_TARGET_PATHMAPPINGLIST_H
 
 #include <map>
 #include <vector>
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 namespace lldb_private {
 
 class PathMappingList {
 public:
   typedef void (*ChangedCallback)(const PathMappingList &path_list,
                                   void *baton);
public:
   ///
   /// Remaps \a path if any source remappings match. This function
   /// does NOT stat the file system so it can be used in tight loops
   /// where debug info is being parsed.
   ///
   /// \param[in] path
   ///     The original source file path to try and remap.
   ///
+  /// \param[in] check_filesystem
+  ///     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 check_filesystem = false) const;
   bool RemapPath(const char *, std::string &) const = delete;
 
   bool ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const;
 
   /// Finds a source file given a file spec using the path remappings.
   ///
   /// Tries to resolve \a orig_spec by checking the path remappings.
   /// It makes sure the file exists by checking with the file system,

lldb/source/Target/PathMappingList.cpp

 //===-- PathMappingList.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <climits>
 #include <cstring>
 
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-private-enumerations.h"
 
 using namespace lldb;
static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
     while (component != e &&
         llvm::sys::path::is_separator(*component->data(), style))
       ++component;
     for (; component != e; ++component)
       path.AppendPathComponent(*component);
 }
 
 llvm::Optional<FileSpec>
-PathMappingList::RemapPath(llvm::StringRef path) const {
+PathMappingList::RemapPath(llvm::StringRef path, bool check_filesystem) const {
   if (m_pairs.empty() || path.empty())
     return {};
   LazyBool path_is_relative = eLazyBoolCalculate;
   for (const auto &it : m_pairs) {
     auto prefix = it.first.GetStringRef();
     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 "."
       // carefully.
       if (prefix != ".")
         continue;
       // We need to figure out if the "path" argument is relative. If it is,
       // then we should remap, else skip this entry.
       if (path_is_relative == eLazyBoolCalculate) {
         path_is_relative =
             FileSpec(path).IsRelative() ? eLazyBoolYes : eLazyBoolNo;
       }
       if (!path_is_relative)
         continue;
     }
     FileSpec remapped(it.second.GetStringRef());
     auto orig_style = FileSpec::GuessPathStyle(prefix).getValueOr(
         llvm::sys::path::Style::native);
     AppendPathComponents(remapped, path, orig_style);
+    if (!check_filesystem || FileSystem::Instance().Exists(remapped))
       return remapped;
   }
   return {};
 }
 
 bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
   for (const auto &it : m_pairs) {
     if (!path_ref.consume_front(it.second.GetStringRef()))
       continue;
     auto orig_file = it.first.GetStringRef();
     auto orig_style = FileSpec::GuessPathStyle(orig_file).getValueOr(
         llvm::sys::path::Style::native);
     fixed.SetFile(orig_file, orig_style);
     AppendPathComponents(fixed, path_ref, orig_style);
     return true;
   }
   return false;
 }
 
 
 llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (auto remapped = RemapPath(orig_spec.GetPath()))
-    if (FileSystem::Instance().Exists(*remapped))
+  if (auto remapped = RemapPath(orig_spec.GetPath(), /*check_filesystem=*/true))
       return remapped;
 
   return {};
 }
 
 bool PathMappingList::Replace(ConstString path,
                               ConstString new_path, bool notify) {
   uint32_t idx = FindIndexForPath(path);
   if (idx < m_pairs.size()) {