[LLD][COFF] More detailed information for /failifmismatch

[aganea]

Summary

When mismatched #pragma detect_mismatch declarations occur, now print the conflicting OBJs.

MSVC prints:

test2.obj : error LNK2038: mismatch detected for 'TEST': value '2' doesn't match value '1' in test.obj
test.exe : fatal error LNK1319: 1 mismatches detected

LLD now prints:

lld-link: error: /failifmismatch: mismatch detected for 'TEST':
>>> test.obj: has value 1
>>> test2.obj: has value 2

Fixes PR38579

[rnk]

lgtm

test/COFF/Inputs/failmismatch1.bc

1

Use the .ll file extension instead of .bc for textual IR files

[ruiu]

lld/trunk/COFF/DriverUtils.cpp

701 ↗

(On Diff #189557)

In general, an error string that may be composed should be of type const Twine & instead of StringRef to avoid cost of string concatenation.

[aganea]

lld/trunk/COFF/DriverUtils.cpp

701 ↗

(On Diff #189557)

The Twine would need to be flattened anyway below, because it has to be stored as a value in Config->MustMatch.
As an alternative, I can pass a InputFile *Source instead as a parameter. Would you prefer that? Please see D59411.

COFF/Config.h

struct Configuration {
   StringRef ManifestLevel = "'asInvoker'";
   StringRef ManifestUIAccess = "'false'";
   StringRef ManifestFile;
 
   // Used for /aligncomm.
   std::map<std::string, int> AlignComm;
 
   // Used for /failifmismatch.
-  std::map<StringRef, StringRef> MustMatch;
+  std::map<StringRef, std::pair<StringRef, std::string>> MustMatch;
 
   // Used for /alternatename.
   std::map<StringRef, StringRef> AlternateNames;
 
   // Used for /order.
   llvm::StringMap<int> Order;
 
   // Used for /lldmap.

COFF/Driver.h

private:
   COFFOptTable Table;
 };
 
 class LinkerDriver {
 public:
   void link(llvm::ArrayRef<const char *> Args);
 
   // Used by the resolver to parse .drectve section contents.
-  void parseDirectives(StringRef S);
+  void parseDirectives(InputFile *File);
 
   // Used by ArchiveFile to enqueue members.
   void enqueueArchiveMember(const Archive::Child &C, StringRef SymName,
                             StringRef ParentName);
 
   MemoryBufferRef takeBuffer(std::unique_ptr<MemoryBuffer> MB);
 
 private:

 Export parseExport(StringRef Arg);
 void fixupExports();
 void assignExportOrdinals();
 
 // Parses a string in the form of "key=value" and check
 // if value matches previous values for the key.
 // This feature used in the directive section to reject
 // incompatible objects.
-void checkFailIfMismatch(StringRef Arg);
+void checkFailIfMismatch(StringRef Arg, StringRef Source);
 
 // Convert Windows resource files (.res files) to a .obj file.
 MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> MBs);
 
 void runMSVCLinker(std::string Rsp, ArrayRef<StringRef> Objects);
 
 // Create enum with OPT_xxx values for each option in Options.td
 enum {

COFF/Driver.cpp

 
 static bool isDecorated(StringRef Sym) {
   return Sym.startswith("@") || Sym.contains("@@") || Sym.startswith("?") ||
          (!Config->MinGW && Sym.contains('@'));
 }
 
 // Parses .drectve section contents and returns a list of files
 // specified by /defaultlib.
-void LinkerDriver::parseDirectives(StringRef S) {
+void LinkerDriver::parseDirectives(InputFile *File) {
+  StringRef S = File->getDirectives();
+  if (S.empty())
+    return;
+
+  log("Directives: " + toString(File) + ": " + S);
+
   ArgParser Parser;
   // .drectve is always tokenized using Windows shell rules.
   // /EXPORT: option can appear too many times, processing in fastpath.
   opt::InputArgList Args;
   std::vector<StringRef> Exports;
   std::tie(Args, Exports) = Parser.parseDirectives(S);
 
   for (StringRef E : Exports) {
for (auto *Arg : Args) {
     case OPT_defaultlib:
       if (Optional<StringRef> Path = findLib(Arg->getValue()))
         enqueuePath(*Path, false);
       break;
     case OPT_entry:
       Config->Entry = addUndefined(mangle(Arg->getValue()));
       break;
     case OPT_failifmismatch:
-      checkFailIfMismatch(Arg->getValue());
+      checkFailIfMismatch(Arg->getValue(), toString(File));
       break;
     case OPT_incl:
       addUndefined(Arg->getValue());
       break;
     case OPT_merge:
       parseMerge(Arg->getValue());
       break;
     case OPT_nodefaultlib:
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   // Handle /lldsavecachepolicy
   if (auto *Arg = Args.getLastArg(OPT_lldltocachepolicy))
     Config->LTOCachePolicy = CHECK(
         parseCachePruningPolicy(Arg->getValue()),
         Twine("/lldltocachepolicy: invalid cache policy: ") + Arg->getValue());
 
   // Handle /failifmismatch
   for (auto *Arg : Args.filtered(OPT_failifmismatch))
-    checkFailIfMismatch(Arg->getValue());
+    checkFailIfMismatch(Arg->getValue(), "cmd-line");
 
   // Handle /merge
   for (auto *Arg : Args.filtered(OPT_merge))
     parseMerge(Arg->getValue());
 
   // Add default section merging rules after user rules. User rules take
   // precedence, but we will emit a warning if there is a conflict.
   parseMerge(".idata=.rdata");

COFF/DriverUtils.cpp

for (Export &E : Config->Exports)
     Max = std::max(Max, E.Ordinal);
   for (Export &E : Config->Exports)
     if (E.Ordinal == 0)
       E.Ordinal = ++Max;
 }
 
 // Parses a string in the form of "key=value" and check
 // if value matches previous values for the same key.
-void checkFailIfMismatch(StringRef Arg) {
+void checkFailIfMismatch(StringRef Arg, StringRef Source) {
   StringRef K, V;
   std::tie(K, V) = Arg.split('=');
   if (K.empty() || V.empty())
     fatal("/failifmismatch: invalid argument: " + Arg);
-  StringRef Existing = Config->MustMatch[K];
-  if (!Existing.empty() && V != Existing)
-    fatal("/failifmismatch: mismatch detected: " + Existing + " and " + V +
-          " for key " + K);
-  Config->MustMatch[K] = V;
+  std::pair<StringRef, StringRef> Existing = Config->MustMatch[K];
+  if (!Existing.first.empty() && V != Existing.first) {
+    fatal("/failifmismatch: mismatch detected for '" + K + "':\n>>> " +
+          Existing.second + ": has value " + Existing.first + "\n>>> " +
+          Source + ": has value " + V);
+  }
+  Config->MustMatch[K] = {V, Source};
 }
 
 // Convert Windows resource files (.res files) to a .obj file.
 MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> MBs) {
   object::WindowsResourceParser Parser;
 
   for (MemoryBufferRef MB : MBs) {
     std::unique_ptr<object::Binary> Bin = check(object::createBinary(MB));

COFF/SymbolTable.cpp

void SymbolTable::addFile(InputFile *File) {
   if (auto *F = dyn_cast<ObjFile>(File)) {
     ObjFile::Instances.push_back(F);
   } else if (auto *F = dyn_cast<BitcodeFile>(File)) {
     BitcodeFile::Instances.push_back(F);
   } else if (auto *F = dyn_cast<ImportFile>(File)) {
     ImportFile::Instances.push_back(F);
   }
 
-  StringRef S = File->getDirectives();
-  if (S.empty())
-    return;
-
-  log("Directives: " + toString(File) + ": " + S);
-  Driver->parseDirectives(S);
+  Driver->parseDirectives(File);
 }
 
 static void errorOrWarn(const Twine &S) {
   if (Config->ForceUnresolved)
     warn(S);
   else
     error(S);
 }

test/COFF/Inputs/failmismatch1.bc

+; ModuleID = 'test.cpp'
+source_filename = "test.cpp"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.16.27027"
+
+; Function Attrs: noinline nounwind optnone sspstrong uwtable
+define dso_local i32 @"?f@@YAHXZ"() #0 {
+  ret i32 0
+}
+
+attributes #0 = { noinline nounwind optnone sspstrong uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.linker.options = !{!0, !1, !2}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = !{!"/DEFAULTLIB:libcmt.lib"}
+!1 = !{!"/DEFAULTLIB:oldnames.lib"}
+!2 = !{!"/FAILIFMISMATCH:\22TEST=1\22"}
+!3 = !{i32 1, !"wchar_size", i32 2}
+!4 = !{i32 7, !"PIC Level", i32 2}
+!5 = !{!"clang version 7.0.1 (tags/RELEASE_701/final)"}

test/COFF/Inputs/failmismatch2.bc

+; ModuleID = 'test2.cpp'
+source_filename = "test2.cpp"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.16.27027"
+
+; Function Attrs: noinline norecurse nounwind optnone sspstrong uwtable
+define dso_local i32 @main() #0 {
+  %1 = alloca i32, align 4
+  store i32 0, i32* %1, align 4
+  %2 = call i32 @"?f@@YAHXZ"()
+  ret i32 %2
+}
+
+declare dso_local i32 @"?f@@YAHXZ"() #1
+
+attributes #0 = { noinline norecurse nounwind optnone sspstrong uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.linker.options = !{!0, !1, !2}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+
+!0 = !{!"/DEFAULTLIB:libcmt.lib"}
+!1 = !{!"/DEFAULTLIB:oldnames.lib"}
+!2 = !{!"/FAILIFMISMATCH:\22TEST=2\22"}
+!3 = !{i32 1, !"wchar_size", i32 2}
+!4 = !{i32 7, !"PIC Level", i32 2}
+!5 = !{!"clang version 7.0.1 (tags/RELEASE_701/final)"}

test/COFF/failifmismatch.test

-# RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
-# RUN:   %p/Inputs/ret42.obj
+RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
+RUN:   %p/Inputs/ret42.obj
 
-# RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
-# RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k2=v1
+RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
+RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k2=v1
 
-# RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
-# RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k1=v1
+RUN: lld-link /entry:main /subsystem:console /out:%t.exe \
+RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k1=v1
 
-# RUN: not lld-link /entry:main /subsystem:console /out:%t.exe \
-# RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k1=v2
+RUN: not lld-link /entry:main /subsystem:console /out:%t.exe \
+RUN:   %p/Inputs/ret42.obj /failifmismatch:k1=v1 /failifmismatch:k1=v2 2>&1 | FileCheck %s 
+
+RUN: llc < %p/Inputs/failmismatch1.bc -filetype obj -o %t1.obj
+RUN: llc < %p/Inputs/failmismatch2.bc -filetype obj -o %t2.obj
+RUN: not lld-link %t1.obj %t2.obj 2>&1 | FileCheck %s -check-prefix OBJ
+
+RUN: llvm-lib %t1.obj /out:%t.lib
+RUN: not lld-link %t.lib %t2.obj 2>&1 | FileCheck %s -check-prefix LIB
+
+CHECK: lld-link: error: /failifmismatch: mismatch detected for 'k1':
+CHECK-NEXT: >>> cmd-line: has value v1
+CHECK-NEXT: >>> cmd-line: has value v2
+
+OBJ: lld-link: error: /failifmismatch: mismatch detected for 'TEST':
+OBJ-NEXT: >>> {{.*}}failifmismatch.test.tmp1.obj: has value 1
+OBJ-NEXT: >>> {{.*}}failifmismatch.test.tmp2.obj: has value 2
+
+LIB: lld-link: error: /failifmismatch: mismatch detected for 'TEST':
+LIB-NEXT: >>> {{.*}}failifmismatch.test.tmp2.obj: has value 2
+LIB-NEXT: >>> failifmismatch.test.tmp.lib(failifmismatch.test.tmp1.obj): has value 1