[gold] -thinlto-object-suffix-replace: don't append new suffix if path does not end with old suffix

[MaskRay]

Summary

This is to be consistent with lld behavior since rLLD340364.

[tejohnson]

Let's hold on this review until we resolve the discussion on the lld patch first. I'd like to understand more about why we are hitting this case.

[tejohnson]

Based on decision in D51055 we should go forward with this change to be consistent with lld. But please add a test and summary.

[MaskRay]

Add test

[MaskRay]

.

[MaskRay]

Update summary with arc diff --edit --verbatim

[tejohnson]

LGTM. Thanks for the cleanup!

test/tools/gold/X86/thinlto_object_suffix_replace.ll

 ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:    -m elf_x86_64 \
 ; RUN:    --plugin-opt=thinlto \
 ; RUN:    --plugin-opt=thinlto-index-only \
 ; RUN:    --plugin-opt=thinlto-object-suffix-replace=".thinlink.bc;.o" \
 ; RUN:    -shared %t1.thinlink.bc -o %t3
 ; RUN: diff %t1.o.thinlto.bc.orig %t1.o.thinlto.bc
 
+; If filename does not end with old suffix, no suffix change should occur,
+; so ".thinlto.bc" will simply be appended to the input file name.
+; RUN: rm -f %t1.thinlink.bc.thinlto.bc
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    -m elf_x86_64 \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=thinlto-index-only \
+; RUN:    --plugin-opt=thinlto-object-suffix-replace=".abc.bc;.o" \
+; RUN:    -shared %t1.thinlink.bc -o /dev/null
+; RUN: ls %t1.thinlink.bc.thinlto.bc
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 define void @f() {
 entry:
   ret void
 }
 
 !llvm.dbg.cu = !{}
 
 !1 = !{i32 2, !"Debug Info Version", i32 3}
 !llvm.module.flags = !{!1}

tools/gold/gold-plugin.cpp

static void getThinLTOOldAndNewSuffix(std::string &OldSuffix,
   StringRef SuffixReplace = options::thinlto_object_suffix_replace;
   std::tie(OldSuffix, NewSuffix) = SuffixReplace.split(';');
 }
 
 /// Given the original \p Path to an output file, replace any filename
 /// suffix matching \p OldSuffix with \p NewSuffix.
 static std::string getThinLTOObjectFileName(StringRef Path, StringRef OldSuffix,
                                             StringRef NewSuffix) {
-  if (OldSuffix.empty() && NewSuffix.empty())
-    return Path;
-  StringRef NewPath = Path;
-  NewPath.consume_back(OldSuffix);
-  std::string NewNewPath = NewPath;
-  NewNewPath += NewSuffix;
-  return NewNewPath;
+  if (Path.consume_back(OldSuffix))
+    return (Path + NewSuffix).str();
 }
 
 // Returns true if S is valid as a C language identifier.
 static bool isValidCIdentifier(StringRef S) {
   return !S.empty() && (isAlpha(S[0]) || S[0] == '_') &&
          std::all_of(S.begin() + 1, S.end(),
                      [](char C) { return C == '_' || isAlnum(C); });
 }