[clang] daad48d - -fdebug-prefix-map=: make the last win when multiple prefixes match

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 25 15:12:21 PDT 2023


Author: Fangrui Song
Date: 2023-04-25T15:12:17-07:00
New Revision: daad48d6b236d74c6b29daebba46289b98104241

URL: https://github.com/llvm/llvm-project/commit/daad48d6b236d74c6b29daebba46289b98104241
DIFF: https://github.com/llvm/llvm-project/commit/daad48d6b236d74c6b29daebba46289b98104241.diff

LOG: -fdebug-prefix-map=: make the last win when multiple prefixes match

For
`clang -c -g -fdebug-prefix-map=a/b=y -fdebug-prefix-map=a=x a/b/c.c`,
we apply the longest prefix substitution, but
GCC has always been picking the last applicable option (`a=x`, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109591).

I feel that GCC's behavior is reasonable given the convention that the last
value wins for the same option.

Before D49466, Clang appeared to apply the shortest prefix substitution,
which likely made the least sense.

Reviewed By: #debug-info, scott.linder

Differential Revision: https://reviews.llvm.org/D148975

Added: 
    

Modified: 
    clang/include/clang/Basic/CodeGenOptions.h
    clang/include/clang/Driver/Options.td
    clang/lib/CodeGen/CGDebugInfo.cpp
    clang/lib/CodeGen/CGDebugInfo.h
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/test/CodeGen/debug-prefix-map.c
    clang/tools/driver/cc1as_main.cpp
    llvm/include/llvm/MC/MCContext.h
    llvm/lib/MC/MCContext.cpp
    llvm/test/MC/ELF/debug-prefix-map.s

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index 6192d3fafd411..662271fc025d4 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -206,7 +206,7 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// if non-empty.
   std::string RecordCommandLine;
 
-  std::map<std::string, std::string> DebugPrefixMap;
+  llvm::SmallVector<std::pair<std::string, std::string>, 0> DebugPrefixMap;
   std::map<std::string, std::string> CoveragePrefixMap;
 
   /// The ABI to use for passing floating point arguments.

diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index eb48c142e84dd..c8f505a92a7c8 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3218,7 +3218,8 @@ def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, Group<f_Gr
 def fdebug_prefix_map_EQ
   : Joined<["-"], "fdebug-prefix-map=">, Group<f_Group>,
     Flags<[CC1Option,CC1AsOption]>,
-    HelpText<"remap file source paths in debug info">;
+    MetaVarName<"<old>=<new>">,
+    HelpText<"For paths in debug info, remap directory <old> to <new>. If multiple options match a path, the last option wins">;
 def fcoverage_prefix_map_EQ
   : Joined<["-"], "fcoverage-prefix-map=">, Group<f_Group>,
     Flags<[CC1Option]>,

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 5100d6c65baa4..2cf7b025744ac 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -73,8 +73,6 @@ CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
     : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
       DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
       DBuilder(CGM.getModule()) {
-  for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
-    DebugPrefixMap[KV.first] = KV.second;
   CreateCompileUnit();
 }
 
@@ -470,12 +468,9 @@ llvm::DIFile *CGDebugInfo::createFile(
 }
 
 std::string CGDebugInfo::remapDIPath(StringRef Path) const {
-  if (DebugPrefixMap.empty())
-    return Path.str();
-
   SmallString<256> P = Path;
-  for (const auto &Entry : DebugPrefixMap)
-    if (llvm::sys::path::replace_path_prefix(P, Entry.first, Entry.second))
+  for (auto &[From, To] : llvm::reverse(CGM.getCodeGenOpts().DebugPrefixMap))
+    if (llvm::sys::path::replace_path_prefix(P, From, To))
       break;
   return P.str().str();
 }

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 5a3839d7f3946..5c42d3620bdbd 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -86,9 +86,6 @@ class CGDebugInfo {
   /// Cache of previously constructed Types.
   llvm::DenseMap<const void *, llvm::TrackingMDRef> TypeCache;
 
-  std::map<llvm::StringRef, llvm::StringRef, std::greater<llvm::StringRef>>
-      DebugPrefixMap;
-
   /// Cache that maps VLA types to size expressions for that type,
   /// represented by instantiated Metadata nodes.
   llvm::SmallDenseMap<QualType, llvm::Metadata *> SizeExprCache;

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 71204debce13d..a1481365daf98 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1696,8 +1696,7 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
     auto Split = StringRef(Arg).split('=');
-    Opts.DebugPrefixMap.insert(
-        {std::string(Split.first), std::string(Split.second)});
+    Opts.DebugPrefixMap.emplace_back(Split.first, Split.second);
   }
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fcoverage_prefix_map_EQ)) {

diff  --git a/clang/test/CodeGen/debug-prefix-map.c b/clang/test/CodeGen/debug-prefix-map.c
index f604f1643ba02..e2421806b055b 100644
--- a/clang/test/CodeGen/debug-prefix-map.c
+++ b/clang/test/CodeGen/debug-prefix-map.c
@@ -9,6 +9,10 @@
 // RUN: %clang -g -fdebug-prefix-map=%p=./UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-REL
 // RUN: %clang -g -ffile-prefix-map=%p=./UNLIKELY_PATH/empty -S -c %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-REL
 
+// RUN: rm -rf %t && mkdir -p %t/a/b && cp %s %t/a/b/c.c
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -I%S -fdebug-prefix-map=%t/a/b=y -fdebug-prefix-map=%t/a=x %t/a/b/c.c -o - | FileCheck %s --check-prefix=CHECK-X
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -I%S -fdebug-prefix-map=%t/a=x -fdebug-prefix-map=%t/a/b=y %t/a/b/c.c -o - | FileCheck %s --check-prefix=CHECK-Y
+
 #include "Inputs/stdio.h"
 
 int main(int argc, char **argv) {
@@ -47,3 +51,6 @@ void test_rewrite_includes(void) {
 // CHECK-REL: !DIFile(filename: "./UNLIKELY_PATH/empty{{/|\\\\}}{{.*}}",
 // CHECK-REL: !DIFile(filename: "./UNLIKELY_PATH/empty{{/|\\\\}}{{.*}}Inputs/stdio.h",
 // CHECK-REL-SAME:    directory: ""
+
+// CHECK-X: !DIFile(filename: "x{{/|\\\\}}b{{/|\\\\}}c.c", directory: "")
+// CHECK-Y: !DIFile(filename: "y{{/|\\\\}}c.c", directory: "")

diff  --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index ee54a0f7b0eab..ecfb1be8c55af 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -97,7 +97,7 @@ struct AssemblerInvocation {
   std::string DwarfDebugFlags;
   std::string DwarfDebugProducer;
   std::string DebugCompilationDir;
-  std::map<const std::string, const std::string> DebugPrefixMap;
+  llvm::SmallVector<std::pair<std::string, std::string>, 0> DebugPrefixMap;
   llvm::DebugCompressionType CompressDebugSections =
       llvm::DebugCompressionType::None;
   std::string MainFileName;
@@ -275,8 +275,7 @@ bool AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
     auto Split = StringRef(Arg).split('=');
-    Opts.DebugPrefixMap.insert(
-        {std::string(Split.first), std::string(Split.second)});
+    Opts.DebugPrefixMap.emplace_back(Split.first, Split.second);
   }
 
   // Frontend Options

diff  --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h
index 74c3fe5c61f29..26ee1803cb6ef 100644
--- a/llvm/include/llvm/MC/MCContext.h
+++ b/llvm/include/llvm/MC/MCContext.h
@@ -190,7 +190,7 @@ class MCContext {
   SmallString<128> CompilationDir;
 
   /// Prefix replacement map for source file information.
-  std::map<std::string, const std::string, std::greater<>> DebugPrefixMap;
+  SmallVector<std::pair<std::string, std::string>, 0> DebugPrefixMap;
 
   /// The main file name if passed in explicitly.
   std::string MainFileName;

diff  --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 80a036a950bc9..6ec1a9ae8444d 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -881,11 +881,11 @@ MCSubtargetInfo &MCContext::getSubtargetCopy(const MCSubtargetInfo &STI) {
 
 void MCContext::addDebugPrefixMapEntry(const std::string &From,
                                        const std::string &To) {
-  DebugPrefixMap.insert(std::make_pair(From, To));
+  DebugPrefixMap.emplace_back(From, To);
 }
 
 void MCContext::remapDebugPath(SmallVectorImpl<char> &Path) {
-  for (const auto &[From, To] : DebugPrefixMap)
+  for (const auto &[From, To] : llvm::reverse(DebugPrefixMap))
     if (llvm::sys::path::replace_path_prefix(Path, From, To))
       break;
 }

diff  --git a/llvm/test/MC/ELF/debug-prefix-map.s b/llvm/test/MC/ELF/debug-prefix-map.s
index 28a0bb03a45c7..6e054d6b231e4 100644
--- a/llvm/test/MC/ELF/debug-prefix-map.s
+++ b/llvm/test/MC/ELF/debug-prefix-map.s
@@ -14,9 +14,13 @@
 # RUN: cd %t.foo/bar
 # RUN: llvm-mc -triple=x86_64 -g -dwarf-version=4 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.4.o -fdebug-prefix-map=%t.foo=/broken_root -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root
 # RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.4.o | FileCheck --check-prefix=MAPABS_V4 %s
-# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root -fdebug-prefix-map=%t.foo=/broken_root
+# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo=/broken_root -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root
 # RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.5.o | FileCheck --check-prefix=MAPABS_V5 %s
 
+## The last -fdebug-prefix-map= wins even if the prefix is shorter.
+# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo%{fs-sep}bar=/broken_root -fdebug-prefix-map=%t.foo=/src_root
+# RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.5.o | FileCheck --check-prefix=MAPABS_V5_A %s
+
 f:
   nop
 
@@ -46,3 +50,6 @@ f:
 # MAPABS_V5:      DW_AT_comp_dir [DW_FORM_string] ("{{(/|\\)+}}src_root")
 # MAPABS_V5:      DW_AT_decl_file [DW_FORM_data4] ("/src_root{{(/|\\)+}}src.s")
 # MAPABS_V5:      include_directories[ 0] = .debug_line_str[0x00000000] = "/src_root"
+
+# MAPABS_V5_A:    DW_AT_comp_dir [DW_FORM_string] ("{{(/|\\)+}}src_root/bar")
+# MAPABS_V5_A:    DW_AT_decl_file [DW_FORM_data4] ("/src_root{{(/|\\)+}}bar{{(/|\\)+}}src.s")


        


More information about the cfe-commits mailing list