[dsymutil][DWARFLinker][NFC] Refactor usages of UniquingStringPool.

[avl]

Summary

That refactoring is helpful since it reduces data inter-dependencies.
Which is good for current implementation and even more good for
fully multi-thread implementation. The idea of the refactoring
is to delete UniquingStringPool from the global DWARFLinker level.
It is used to unique type names while ODR deduplication is done.
Thus we move UniquingStringPool into the DeclContextTree which
matched to UniquingStringPool usage scope.

golden-dsymutil/dsymutil 23787992
clang MD5: 7d9873ff94f0246b6ab1ec3e8d0f3f06

build-Release/bin/dsymutil 23921272
clang MD5: 7d9873ff94f0246b6ab1ec3e8d0f3f06

[dblaikie]

Driveby spelling correction - but I don't know much of anything about the code in question, so deeper review will have to wait for someone else.

llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp

91

Spelling correction.

[JDevlieghere]

LGTM with the comments and clang-tidy warning addressed.

llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h

158

Can this be implemented in the cpp file?

llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp

91–98

[avl]

Thank you! Will address comments.

llvm/include/llvm/DWARFLinker/DWARFLinker.h

private:
   /// clang module, register it in ClangModules and return true.
   ///
   /// A skeleton CU is a CU without children, a DW_AT_gnu_dwo_name
   /// pointing to the module, and a DW_AT_gnu_dwo_id with the module
   /// hash.
   bool registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit,
                                const DWARFFile &File,
                                OffsetsStringPool &OffsetsStringPool,
-                               UniquingStringPool &UniquingStringPoolStringPool,
                                DeclContextTree &ODRContexts,
                                uint64_t ModulesEndOffset, unsigned &UnitID,
                                bool IsLittleEndian, unsigned Indent = 0,
                                bool Quiet = false);
 
   /// Recursively add the debug info in this clang module .pcm
   /// file (and all the modules imported by it in a bottom-up fashion)
   /// to Units.
   Error loadClangModule(DWARFDie CUDie, StringRef FilePath,
                         StringRef ModuleName, uint64_t DwoId,
                         const DWARFFile &File,
                         OffsetsStringPool &OffsetsStringPool,
-                        UniquingStringPool &UniquingStringPool,
                         DeclContextTree &ODRContexts, uint64_t ModulesEndOffset,
                         unsigned &UnitID, bool IsLittleEndian,
                         unsigned Indent = 0, bool Quiet = false);
 
   /// Mark the passed DIE as well as all the ones it depends on as kept.
   void keepDIEAndDependencies(AddressesMap &RelocMgr, RangesTy &Ranges,
                               const UnitListTy &Units, const DWARFDie &DIE,
                               CompileUnit::DIEInfo &MyInfo,

llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h

 //===- DWARFLinkerCompileUnit.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 LLVM_DWARFLINKER_DWARFLINKERCOMPILEUNIT_H
 #define LLVM_DWARFLINKER_DWARFLINKERCOMPILEUNIT_H
 
 #include "llvm/ADT/IntervalMap.h"
 #include "llvm/CodeGen/DIE.h"
+#include "llvm/CodeGen/NonRelocatableStringpool.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"
 #include "llvm/Support/DataExtractor.h"
 
 namespace llvm {
 
 class DeclContext;
 
 template <typename KeyT, typename ValT>
AccelInfo(DwarfStringPoolEntryRef Name, const DIE *Die,
           ObjcClassImplementation(ObjCClassIsImplementation) {}
   };
 
   const std::vector<AccelInfo> &getPubnames() const { return Pubnames; }
   const std::vector<AccelInfo> &getPubtypes() const { return Pubtypes; }
   const std::vector<AccelInfo> &getNamespaces() const { return Namespaces; }
   const std::vector<AccelInfo> &getObjC() const { return ObjC; }
 
-  /// Get the full path for file \a FileNum in the line table
-  StringRef getResolvedPath(unsigned FileNum) {
-    if (FileNum >= ResolvedPaths.size())
-      return StringRef();
-    return ResolvedPaths[FileNum];
-  }
-
-  /// Set the fully resolved path for the line-table's file \a FileNum
-  /// to \a Path.
-  void setResolvedPath(unsigned FileNum, StringRef Path) {
-    if (ResolvedPaths.size() <= FileNum)
-      ResolvedPaths.resize(FileNum + 1);
-    ResolvedPaths[FileNum] = Path;
-  }
-
   MCSymbol *getLabelBegin() { return LabelBegin; }
   void setLabelBegin(MCSymbol *S) { LabelBegin = S; }
 
 private:
   DWARFUnit &OrigUnit;
   unsigned ID;
   std::vector<DIEInfo> Info; ///< DIE info indexed by DIE index.
   Optional<BasicDIEUnit> NewUnit;
private:
   /// sections and the apple* ones.
   /// @{
   std::vector<AccelInfo> Pubnames;
   std::vector<AccelInfo> Pubtypes;
   std::vector<AccelInfo> Namespaces;
   std::vector<AccelInfo> ObjC;
   /// @}
 
-  /// Cached resolved paths from the line table.
-  /// Note, the StringRefs here point in to the intern (uniquing) string pool.
-  /// This means that a StringRef returned here doesn't need to then be uniqued
-  /// for the purposes of getting a unique address for each string.
-  std::vector<StringRef> ResolvedPaths;
-
   /// Is this unit subject to the ODR rule?
   bool HasODR;
 
   /// Did a DIE actually contain a valid reloc?
   bool HasInterestingContent;
 
   /// The DW_AT_language of this unit.
   uint16_t Language = 0;

llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h

 #define LLVM_DWARFLINKER_DWARFLINKERDECLCONTEXT_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/NonRelocatableStringpool.h"
 #include "llvm/DWARFLinker/DWARFLinkerCompileUnit.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugLine.h"
 #include "llvm/DebugInfo/DWARF/DWARFDie.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 namespace llvm {
 
 struct DeclMapInfo;
 
 /// Small helper that resolves and caches file paths. This helps reduce the
 /// number of calls to realpath which is expensive. We assume the input are
 /// files, and cache the realpath of their parent. This way we can quickly
 /// resolve different files under the same path.
 class CachedPathResolver {
 public:
   /// Resolve a path by calling realpath and cache its result. The returned
   /// StringRef is interned in the given \p StringPool.
-  StringRef resolve(std::string Path, NonRelocatableStringpool &StringPool) {
+  StringRef resolve(const std::string &Path,
+                    NonRelocatableStringpool &StringPool) {
     StringRef FileName = sys::path::filename(Path);
-    SmallString<256> ParentPath = sys::path::parent_path(Path);
+    StringRef ParentPath = sys::path::parent_path(Path);
 
     // If the ParentPath has not yet been resolved, resolve and cache it for
     // future look-ups.
     if (!ResolvedPaths.count(ParentPath)) {
       SmallString<256> RealPath;
       sys::fs::real_path(ParentPath, RealPath);
-      ResolvedPaths.insert({ParentPath, StringRef(RealPath).str()});
+      ResolvedPaths.insert(
+          {ParentPath, std::string(RealPath.c_str(), RealPath.size())});
     }
 
     // Join the file name again with the resolved path.
     SmallString<256> ResolvedPath(ResolvedPaths[ParentPath]);
     sys::path::append(ResolvedPath, FileName);
     return StringPool.internString(ResolvedPath);
   }
 
public:
 
   uint32_t getCanonicalDIEOffset() const { return CanonicalDIEOffset; }
   void setCanonicalDIEOffset(uint32_t Offset) { CanonicalDIEOffset = Offset; }
 
   bool isDefinedInClangModule() const { return DefinedInClangModule; }
   void setDefinedInClangModule(bool Val) { DefinedInClangModule = Val; }
 
   uint16_t getTag() const { return Tag; }
-  StringRef getName() const { return Name; }
 
 private:
   friend DeclMapInfo;
 
   unsigned QualifiedNameHash = 0;
   uint32_t Line = 0;
   uint32_t ByteSize = 0;
   uint16_t Tag = dwarf::DW_TAG_compile_unit;
public:
   /// this context is invalid.
   ///
   /// An invalid context means it shouldn't be considered for uniquing, but its
   /// not returning null, because some children of that context might be
   /// uniquing candidates.
   ///
   /// FIXME: The invalid bit along the return value is to emulate some
   /// dsymutil-classic functionality.
-  PointerIntPair<DeclContext *, 1>
-  getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
-                      CompileUnit &Unit, UniquingStringPool &StringPool,
+  PointerIntPair<DeclContext *, 1> getChildDeclContext(DeclContext &Context,
+                                                       const DWARFDie &DIE,
+                                                       CompileUnit &Unit,
                                                        bool InClangModule);
 
   DeclContext &getRoot() { return Root; }
 
 private:
   BumpPtrAllocator Allocator;
   DeclContext Root;
   DeclContext::Map Contexts;
 
-  /// Cache resolved paths from the line table.
+  /// Cached resolved paths from the line table.
+  /// The key is <UniqueUnitID, FileIdx>.
+  using ResolvedPathsMap = DenseMap<std::pair<unsigned, unsigned>, StringRef>;
+  ResolvedPathsMap ResolvedPaths;
+
+  /// Helper that resolves and caches fragments of file paths.
   CachedPathResolver PathResolver;
+
+  /// String pool keeping real path bodies.
+  NonRelocatableStringpool StringPool;
+
+  StringRef getResolvedPath(CompileUnit &CU, unsigned FileNum,
+                            const DWARFDebugLine::LineTable &LineTable) {
+    std::pair<unsigned, unsigned> Key = {CU.getUniqueID(), FileNum};
+
+    ResolvedPathsMap::const_iterator it = ResolvedPaths.find(Key);
+
+    if (it == ResolvedPaths.end()) {
+      std::string FileName;
+      bool FoundFileName = LineTable.getFileNameByIndex(
+          FileNum, CU.getOrigUnit().getCompilationDir(),
+          DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, FileName);
+      (void)FoundFileName;
+      assert(FoundFileName && "Must get file name from line table");
+
+      // Second level of caching, this time based on the file's parent
+      // path.
+      StringRef ResolvedPath = PathResolver.resolve(FileName, StringPool);
+
+      it = ResolvedPaths.insert(std::make_pair(Key, ResolvedPath)).first;
+    }
+
+    return it->second;
+  }
 };
 
 /// Info type for the DenseMap storing the DeclContext pointers.
 struct DeclMapInfo : private DenseMapInfo<DeclContext *> {
   using DenseMapInfo<DeclContext *>::getEmptyKey;
   using DenseMapInfo<DeclContext *>::getTombstoneKey;
 
   static unsigned getHashValue(const DeclContext *Ctxt) {

llvm/lib/DWARFLinker/DWARFLinker.cpp

 ///
 /// This function uses the same work list approach as lookForDIEsToKeep.
 ///
 /// \return true when this DIE and all of its children are only
 /// forward declarations to types defined in external clang modules
 /// (i.e., forward declarations that are children of a DW_TAG_module).
 static bool analyzeContextInfo(
     const DWARFDie &DIE, unsigned ParentIdx, CompileUnit &CU,
-    DeclContext *CurrentDeclContext, UniquingStringPool &StringPool,
-    DeclContextTree &Contexts, uint64_t ModulesEndOffset,
-    swiftInterfacesMap *ParseableSwiftInterfaces,
+    DeclContext *CurrentDeclContext, DeclContextTree &Contexts,
+    uint64_t ModulesEndOffset, swiftInterfacesMap *ParseableSwiftInterfaces,
     std::function<void(const Twine &, const DWARFDie &)> ReportWarning,
     bool InImportedModule = false) {
   // LIFO work list.
   std::vector<ContextWorklistItem> Worklist;
   Worklist.emplace_back(DIE, CurrentDeclContext, ParentIdx, InImportedModule);
 
   while (!Worklist.empty()) {
     ContextWorklistItem Current = Worklist.back();
if (Current.Die.getTag() == dwarf::DW_TAG_module &&
                             ReportWarning);
     }
 
     Info.ParentIdx = Current.ParentIdx;
     bool InClangModule = CU.isClangModule() || Current.InImportedModule;
     if (CU.hasODR() || InClangModule) {
       if (Current.Context) {
         auto PtrInvalidPair = Contexts.getChildDeclContext(
-            *Current.Context, Current.Die, CU, StringPool, InClangModule);
+            *Current.Context, Current.Die, CU, InClangModule);
         Current.Context = PtrInvalidPair.getPointer();
         Info.Ctxt =
             PtrInvalidPair.getInt() ? nullptr : PtrInvalidPair.getPointer();
         if (Info.Ctxt)
           Info.Ctxt->setDefinedInClangModule(InClangModule);
       } else
         Info.Ctxt = Current.Context = nullptr;
     }
static std::string remapPath(StringRef Path,
 
   SmallString<256> p = Path;
   for (const auto &Entry : ObjectPrefixMap)
     if (llvm::sys::path::replace_path_prefix(p, Entry.first, Entry.second))
       break;
   return p.str().str();
 }
 
-bool DWARFLinker::registerModuleReference(
-    DWARFDie CUDie, const DWARFUnit &Unit, const DWARFFile &File,
-    OffsetsStringPool &StringPool, UniquingStringPool &UniquingStringPool,
-    DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID,
-    bool IsLittleEndian, unsigned Indent, bool Quiet) {
+bool DWARFLinker::registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit,
+                                          const DWARFFile &File,
+                                          OffsetsStringPool &StringPool,
+                                          DeclContextTree &ODRContexts,
+                                          uint64_t ModulesEndOffset,
+                                          unsigned &UnitID, bool IsLittleEndian,
+                                          unsigned Indent, bool Quiet) {
   std::string PCMfile = dwarf::toString(
       CUDie.find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}), "");
   if (PCMfile.empty())
     return false;
   if (Options.ObjectPrefixMap)
     PCMfile = remapPath(PCMfile, *Options.ObjectPrefixMap);
 
   // Clang module DWARF skeleton CUs abuse this for the path to the module.
bool DWARFLinker::registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit,
   }
   if (!Quiet && Options.Verbose)
     outs() << " ...\n";
 
   // Cyclic dependencies are disallowed by Clang, but we still
   // shouldn't run into an infinite loop, so mark it as processed now.
   ClangModules.insert({PCMfile, DwoId});
 
-  if (Error E =
-          loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool,
-                          UniquingStringPool, ODRContexts, ModulesEndOffset,
-                          UnitID, IsLittleEndian, Indent + 2, Quiet)) {
+  if (Error E = loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool,
+                                ODRContexts, ModulesEndOffset, UnitID,
+                                IsLittleEndian, Indent + 2, Quiet)) {
     consumeError(std::move(E));
     return false;
   }
   return true;
 }
 
 Error DWARFLinker::loadClangModule(
     DWARFDie CUDie, StringRef Filename, StringRef ModuleName, uint64_t DwoId,
     const DWARFFile &File, OffsetsStringPool &StringPool,
-    UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts,
-    uint64_t ModulesEndOffset, unsigned &UnitID, bool IsLittleEndian,
-    unsigned Indent, bool Quiet) {
+    DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID,
+    bool IsLittleEndian, unsigned Indent, bool Quiet) {
   /// Using a SmallString<0> because loadClangModule() is recursive.
   SmallString<0> Path(Options.PrependPath);
   if (sys::path::is_relative(Filename))
     resolveRelativeObjectPath(Path, CUDie);
   sys::path::append(Path, Filename);
   // Don't use the cached binary holder because we have no thread-safety
   // guarantee and the lifetime is limited.
 
   if (Options.ObjFileLoader == nullptr)
     return Error::success();
 
   auto ErrOrObj = Options.ObjFileLoader(File.FileName, Path);
   if (!ErrOrObj)
     return Error::success();
 
   std::unique_ptr<CompileUnit> Unit;
 
   for (const auto &CU : ErrOrObj->Dwarf->compile_units()) {
     updateDwarfVersion(CU->getVersion());
     // Recursively get all modules imported by this one.
     auto CUDie = CU->getUnitDIE(false);
     if (!CUDie)
       continue;
-    if (!registerModuleReference(
-            CUDie, *CU, File, StringPool, UniquingStringPool, ODRContexts,
-            ModulesEndOffset, UnitID, IsLittleEndian, Indent, Quiet)) {
+    if (!registerModuleReference(CUDie, *CU, File, StringPool, ODRContexts,
+                                 ModulesEndOffset, UnitID, IsLittleEndian,
+                                 Indent, Quiet)) {
       if (Unit) {
         std::string Err =
             (Filename +
              ": Clang modules are expected to have exactly 1 compile unit.\n")
                 .str();
         reportError(Err, File);
         return make_error<StringError>(Err, inconvertibleErrorCode());
       }
if (!registerModuleReference(CUDie, *CU, File, StringPool, ODRContexts,
         // Update the cache entry with the DwoId of the module loaded from disk.
         ClangModules[Filename] = PCMDwoId;
       }
 
       // Add this module.
       Unit = std::make_unique<CompileUnit>(*CU, UnitID++, !Options.NoODR,
                                            ModuleName);
       Unit->setHasInterestingContent();
-      analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(),
-                         UniquingStringPool, ODRContexts, ModulesEndOffset,
-                         Options.ParseableSwiftInterfaces,
+      analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(), ODRContexts,
+                         ModulesEndOffset, Options.ParseableSwiftInterfaces,
                          [&](const Twine &Warning, const DWARFDie &DIE) {
                            reportWarning(Warning, File, &DIE);
                          });
       // Keep everything.
       Unit->markEverythingAsKept();
     }
   }
   if (!Unit->getOrigUnit().getUnitDIE().hasChildren())
bool DWARFLinker::link() {
 
   // A unique ID that identifies each compile unit.
   unsigned UnitID = 0;
 
   // First populate the data structure we need for each iteration of the
   // parallel loop.
   unsigned NumObjects = ObjectContexts.size();
 
-  // This Dwarf string pool which is only used for uniquing. This one should
-  // never be used for offsets as its not thread-safe or predictable.
-  UniquingStringPool UniquingStringPool(nullptr, true);
-
   // This Dwarf string pool which is used for emission. It must be used
   // serially as the order of calling getStringOffset matters for
   // reproducibility.
   OffsetsStringPool OffsetsStringPool(StringsTranslator, true);
 
   // ODR Contexts for the optimize.
   DeclContextTree ODRContexts;
 
for (const auto &CU : OptContext.File.Dwarf->compile_units()) {
         outs() << "Input compilation unit:";
         DIDumpOptions DumpOpts;
         DumpOpts.ChildRecurseDepth = 0;
         DumpOpts.Verbose = Options.Verbose;
         CUDie.dump(outs(), 0, DumpOpts);
       }
       if (CUDie && !LLVM_UNLIKELY(Options.Update))
         registerModuleReference(CUDie, *CU, OptContext.File, OffsetsStringPool,
-                                UniquingStringPool, ODRContexts, 0, UnitID,
+                                ODRContexts, 0, UnitID,
                                 OptContext.File.Dwarf->isLittleEndian());
     }
   }
 
   // If we haven't seen any CUs, pick an arbitrary valid Dwarf version anyway.
   if (MaxDwarfVersion == 0)
     MaxDwarfVersion = 3;
 
for (const auto &CU : Context.File.Dwarf->compile_units()) {
       // over fully resolved skeleton units. This second pass of
       // registerModuleReferences doesn't do any new work, but it
       // will collect top-level errors, which are suppressed. Module
       // warnings were already displayed in the first iteration.
       bool Quiet = true;
       auto CUDie = CU->getUnitDIE(false);
       if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
           !registerModuleReference(CUDie, *CU, Context.File, OffsetsStringPool,
-                                   UniquingStringPool, ODRContexts,
-                                   ModulesEndOffset, UnitID, Quiet)) {
+                                   ODRContexts, ModulesEndOffset, UnitID,
+                                   Quiet)) {
         Context.CompileUnits.push_back(std::make_unique<CompileUnit>(
             *CU, UnitID++, !Options.NoODR && !Options.Update, ""));
       }
     }
 
     // Now build the DIE parent links that we will use during the next phase.
     for (auto &CurrentUnit : Context.CompileUnits) {
       auto CUDie = CurrentUnit->getOrigUnit().getUnitDIE();
       if (!CUDie)
         continue;
       analyzeContextInfo(CurrentUnit->getOrigUnit().getUnitDIE(), 0,
-                         *CurrentUnit, &ODRContexts.getRoot(),
-                         UniquingStringPool, ODRContexts, ModulesEndOffset,
-                         Options.ParseableSwiftInterfaces,
+                         *CurrentUnit, &ODRContexts.getRoot(), ODRContexts,
+                         ModulesEndOffset, Options.ParseableSwiftInterfaces,
                          [&](const Twine &Warning, const DWARFDie &DIE) {
                            reportWarning(Warning, Context.File, &DIE);
                          });
     }
   };
 
   // For each object file map how many bytes were emitted.
   StringMap<DebugInfoSize> SizeByObject;

llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp

if (LastSeenCompileUnitID == U.getUniqueID()) {
     return false;
   }
 
   LastSeenCompileUnitID = U.getUniqueID();
   LastSeenDIE = Die;
   return true;
 }
 
-PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
-    DeclContext &Context, const DWARFDie &DIE, CompileUnit &U,
-    UniquingStringPool &StringPool, bool InClangModule) {
+PointerIntPair<DeclContext *, 1>
+DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
+                                     CompileUnit &U, bool InClangModule) {
   unsigned Tag = DIE.getTag();
 
   // FIXME: dsymutil-classic compat: We should bail out here if we
   // have a specification or an abstract_origin. We will get the
   // parent context wrong here.
 
   switch (Tag) {
   default:
case dwarf::DW_TAG_typedef:
     // demand. For example implicitly defined constructors are ambiguous
     // because of the way we identify contexts, and they won't be generated
     // every time everywhere.
     if (dwarf::toUnsigned(DIE.find(dwarf::DW_AT_artificial), 0))
       return PointerIntPair<DeclContext *, 1>(nullptr);
     break;
   }
 
-  const char *Name = DIE.getLinkageName();
-  const char *ShortName = DIE.getShortName();
-
-  if (!Name)
-    Name = ShortName;
-
   StringRef NameRef;
-  StringRef ShortNameRef;
   StringRef FileRef;
 
-  if (Name)
-    NameRef = StringPool.internString(Name);
-  else if (Tag == dwarf::DW_TAG_namespace)
+  if (const char *LinkageName = DIE.getLinkageName())
+    NameRef = StringPool.internString(LinkageName);
+  else if (const char *ShortName = DIE.getShortName())
+    NameRef = StringPool.internString(ShortName);
+
+  bool IsAnonimousNamespace = false;
+
+  if (NameRef.empty() && Tag == dwarf::DW_TAG_namespace) {
     // FIXME: For dsymutil-classic compatibility. I think uniquing within
     // anonymous namespaces is wrong. There is no ODR guarantee there.
-    NameRef = StringPool.internString("(anonymous namespace)");
-
-  if (ShortName && ShortName != Name)
-    ShortNameRef = StringPool.internString(ShortName);
-  else
-    ShortNameRef = NameRef;
+    NameRef = "(anonymous namespace)";
+    IsAnonimousNamespace = true;
+  }
 
   if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type &&
       Tag != dwarf::DW_TAG_union_type &&
       Tag != dwarf::DW_TAG_enumeration_type && NameRef.empty())
     return PointerIntPair<DeclContext *, 1>(nullptr);
 
   unsigned Line = 0;
   unsigned ByteSize = std::numeric_limits<uint32_t>::max();
 
   if (!InClangModule) {
     // Gather some discriminating data about the DeclContext we will be
     // creating: File, line number and byte size. This shouldn't be necessary,
     // because the ODR is just about names, but given that we do some
     // approximations with overloaded functions and anonymous namespaces, use
     // these additional data points to make the process safer.
     //
     // This is disabled for clang modules, because forward declarations of
     // module-defined types do not have a file and line.
     ByteSize = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_byte_size),
                                  std::numeric_limits<uint64_t>::max());
-    if (Tag != dwarf::DW_TAG_namespace || !Name) {
+    if (Tag != dwarf::DW_TAG_namespace || IsAnonimousNamespace) {
       if (unsigned FileNum =
               dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_file), 0)) {
         if (const auto *LT = U.getOrigUnit().getContext().getLineTableForUnit(
                 &U.getOrigUnit())) {
           // FIXME: dsymutil-classic compatibility. I'd rather not
           // unique anything in anonymous namespaces, but if we do, then
           // verify that the file and line correspond.
-          if (!Name && Tag == dwarf::DW_TAG_namespace)
+          if (IsAnonimousNamespace)
             FileNum = 1;
 
           if (LT->hasFileAtIndex(FileNum)) {
             Line = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_line), 0);
             // Cache the resolved paths based on the index in the line table,
-            // because calling realpath is expansive.
-            StringRef ResolvedPath = U.getResolvedPath(FileNum);
-            if (!ResolvedPath.empty()) {
-              FileRef = ResolvedPath;
-            } else {
-              std::string File;
-              bool FoundFileName = LT->getFileNameByIndex(
-                  FileNum, U.getOrigUnit().getCompilationDir(),
-                  DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
-                  File);
-              (void)FoundFileName;
-              assert(FoundFileName && "Must get file name from line table");
-              // Second level of caching, this time based on the file's parent
-              // path.
-              FileRef = PathResolver.resolve(File, StringPool);
-              U.setResolvedPath(FileNum, FileRef);
-            }
+            // because calling realpath is expensive.
+            FileRef = getResolvedPath(U, FileNum, *LT);
           }
         }
       }
     }
   }
 
   if (!Line && NameRef.empty())
     return PointerIntPair<DeclContext *, 1>(nullptr);
 
   // We hash NameRef, which is the mangled name, in order to get most
   // overloaded functions resolve correctly.
   //
   // Strictly speaking, hashing the Tag is only necessary for a
   // DW_TAG_module, to prevent uniquing of a module and a namespace
   // with the same name.
   //
   // FIXME: dsymutil-classic won't unique the same type presented
   // once as a struct and once as a class. Using the Tag in the fully
   // qualified name hash to get the same effect.
   unsigned Hash = hash_combine(Context.getQualifiedNameHash(), Tag, NameRef);
 
   // FIXME: dsymutil-classic compatibility: when we don't have a name,
   // use the filename.
-  if (Tag == dwarf::DW_TAG_namespace && NameRef == "(anonymous namespace)")
+  if (IsAnonimousNamespace)
     Hash = hash_combine(Hash, FileRef);
 
   // Now look if this context already exists.
   DeclContext Key(Hash, Line, ByteSize, Tag, NameRef, FileRef, Context);
   auto ContextIter = Contexts.find(&Key);
 
   if (ContextIter == Contexts.end()) {
     // The context wasn't found.