[llvm] llvm-ifs: Support for handling empty IFS and merging weak+strong symbols.

[plotfi]

Summary

The following changes enable llvm-ifs to handle the following merge conflicts:

* Weak + Strong symbol merging for the same symbol
* empty vs non-empty triple field
* empty vs non-empty object file format

[compnerd]

llvm/tools/llvm-ifs/llvm-ifs.cpp

426

I would probably just write this as a ternary.

441

Why the inverse check style here?

450

Similar

501

Indentation seems off? Does this ensure that the rest of the symbol is identical? What happens if one of the size is different?

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

426

So Stub.ObjectFileFormat = !Stub.ObjectFileFormat.empty() ? Stub.ObjectFileFormat : TargetStub->ObjectFileFormat; ? That doesn't seem that much cleaner to read to me than the if

441

A little less error prone? Should I change it to the other way around?

501

Yeah I realized this and changed it locally.

[plotfi]

Updated based on @compnerd's suggestions.

[plotfi]

fixing indentation

[plotfi]

Update based on @compnerd's feedback

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

501

The Type and Size checks are above before the Weak/Strong check. So yes they will be identical. I should add a test case though that exercises this.

[compnerd]

LGTM with the additional test for the size/type mismatch validation.

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

501

I won't land until I add a lit test exercising strong/weak mismatch plus size mismatch.

[plotfi]

adding tests for weak/strong mismatch that is also a size/type mismatch for a given symbol.

llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs

+# NOTE: Used by weak-mismatch.ifs
+--- !experimental-ifs-v1
+IfsVersion: 1.0
+Triple: x86_64-unknown-linux-gnu
+ObjectFileFormat: ELF
+Symbols:
+  foobar: { Type: Object, Size: 2 }
+...

llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs

+# NOTE: Used by weak-mismatch.ifs
+--- !experimental-ifs-v1
+IfsVersion: 1.0
+Triple: x86_64-unknown-linux-gnu
+ObjectFileFormat: ELF
+Symbols:
+  foobar: { Type: Func }
+...

llvm/test/tools/llvm-ifs/conflict-weak.ifs

-# RUN: not llvm-ifs -action write-ifs -o - %s %S/func.ifs 2>&1 | \
+# RUN: llvm-ifs -action write-ifs -o - %s %S/func.ifs 2>&1 | \
 # RUN: FileCheck %s --check-prefixes=CHECK-IFS
 
-# Here we are testing to see if two symbols with identical names will fail to
-# merge in conflict due to one being weak and one not. Eventually this will work
-# when llvm-ifs has support for resolving these kinds of conflicts.
-# CHECK-IFS: error: Interface Stub: Weak Mismatch for a.
-# CHECK-IFS-NEXT: Filename:
-# CHECK-IFS-NEXT: Weak Values: 1 0
+# CHECK-IFS: Symbols:
+# CHECK-IFS-NEXT: a: { Type: Func, Weak: true }
 
 --- !experimental-ifs-v1
 IfsVersion: 1.0
 Triple: x86_64-unknown-linux-gnu
 ObjectFileFormat: ELF
 Symbols:
   a: { Type: Func, Weak: true }
 ...

llvm/test/tools/llvm-ifs/default-empty.ifs

+# RUN: llvm-ifs -action write-ifs -o - %s | FileCheck --check-prefixes=CHECK-DEFAULT %s
+# RUN: llvm-ifs -action write-ifs -o - %s %S/weak.ifs | FileCheck --check-prefixes=CHECK-MERGE %s
+
+# CHECK-DEFAULT: --- !experimental-ifs-v1
+# CHECK-DEFAULT-NEXT: IfsVersion:      1.2
+# CHECK-DEFAULT-NEXT: Triple:          ''
+# CHECK-DEFAULT-NEXT: ObjectFileFormat: ELF
+# CHECK-DEFAULT-NEXT: Symbols:         {}
+# CHECK-DEFAULT-NEXT: ...
+
+# CHECK-MERGE: --- !experimental-ifs-v1
+# CHECK-MERGE-NEXT: IfsVersion: 1.0
+# CHECK-MERGE-NEXT: Triple: x86_64-unknown-linux-gnu
+# CHECK-MERGE-NEXT: ObjectFileFormat: ELF
+# CHECK-MERGE-NEXT: Symbols:
+# CHECK-MERGE-DAG:   _Z8weakFuncv: { Type: Func, Weak: true }
+# CHECK-MERGE-DAG:   _Z10strongFuncv: { Type: Func }
+# CHECK-MERGE: ...
+
+--- !experimental-ifs-v1
+IfsVersion:      1.2
+Triple:          ''
+ObjectFileFormat: ELF
+Symbols:         {}
+...

llvm/test/tools/llvm-ifs/strong.ifs

+# RUN: llvm-ifs -action write-ifs -o - %s %S/strong.ifs | FileCheck %s --check-prefixes=CHECK-IFS
+
+# CHECK-IFS: --- !experimental-ifs-v1
+# CHECK-IFS-NEXT: IfsVersion: 1.0
+# CHECK-IFS-NEXT: Triple: x86_64-unknown-linux-gnu
+# CHECK-IFS-NEXT: ObjectFileFormat: ELF
+# CHECK-IFS-NEXT: Symbols:
+# CHECK-IFS-DAG:   _Z8weakFuncv: { Type: Func }
+# CHECK-IFS: ...
+
+--- !experimental-ifs-v1
+IfsVersion: 1.0
+Triple: x86_64-unknown-linux-gnu
+ObjectFileFormat: ELF
+Symbols:
+  _Z8weakFuncv: { Type: Func }
+...

llvm/test/tools/llvm-ifs/weak-mismatch.ifs

+# RUN: not llvm-ifs -action write-ifs -o - %s %S/Inputs/strong-mismatch-size.ifs 2>&1 | FileCheck %s --check-prefixes=CHECK-SIZE
+# RUN: not llvm-ifs -action write-ifs -o - %s %S/Inputs/strong-mismatch-type.ifs 2>&1 | FileCheck %s --check-prefixes=CHECK-TYPE
+
+# CHECK-SIZE: error: Interface Stub: Size Mismatch for foobar.
+# CHECK-SIZE-NEXT: Filename:
+# CHECK-SIZE-NEXT: Size Values: 1 2
+
+
+# CHECK-TYPE: error: Interface Stub: Type Mismatch for foobar.
+# CHECK-TYPE-NEXT: Filename:
+# CHECK-TYPE-NEXT: Type Values: Object Func
+
+--- !experimental-ifs-v1
+IfsVersion: 1.0
+Triple: x86_64-unknown-linux-gnu
+ObjectFileFormat: ELF
+Symbols:
+  foobar: { Type: Object, Size: 1, Weak: true }
+...

llvm/tools/llvm-ifs/llvm-ifs.cpp

for (const std::string &InputFilePath : InputFilenames) {
     if (Stub.Triple.empty()) {
       PreviousInputFilePath = InputFilePath;
       Stub.IfsVersion = TargetStub->IfsVersion;
       Stub.Triple = TargetStub->Triple;
       Stub.ObjectFileFormat = TargetStub->ObjectFileFormat;
       Stub.SOName = TargetStub->SOName;
       Stub.NeededLibs = TargetStub->NeededLibs;
     } else {
+      Stub.ObjectFileFormat = !Stub.ObjectFileFormat.empty()
+                                  ? Stub.ObjectFileFormat
+                                  : TargetStub->ObjectFileFormat;
+
       if (Stub.IfsVersion != TargetStub->IfsVersion) {
         if (Stub.IfsVersion.getMajor() != IFSVersionCurrent.getMajor()) {
           WithColor::error()
               << "Interface Stub: IfsVersion Mismatch."
               << "\nFilenames: " << PreviousInputFilePath << " "
               << InputFilePath << "\nIfsVersion Values: " << Stub.IfsVersion
               << " " << TargetStub->IfsVersion << "\n";
           return -1;
         }
         if (TargetStub->IfsVersion > Stub.IfsVersion)
           Stub.IfsVersion = TargetStub->IfsVersion;
       }
-      if (Stub.ObjectFileFormat != TargetStub->ObjectFileFormat) {
+      if (Stub.ObjectFileFormat != TargetStub->ObjectFileFormat &&
+          !TargetStub->ObjectFileFormat.empty()) {
         WithColor::error() << "Interface Stub: ObjectFileFormat Mismatch."
                            << "\nFilenames: " << PreviousInputFilePath << " "
                            << InputFilePath << "\nObjectFileFormat Values: "
                            << Stub.ObjectFileFormat << " "
                            << TargetStub->ObjectFileFormat << "\n";
         return -1;
       }
-      if (Stub.Triple != TargetStub->Triple) {
+      if (Stub.Triple != TargetStub->Triple && !TargetStub->Triple.empty()) {
         WithColor::error() << "Interface Stub: Triple Mismatch."
                            << "\nFilenames: " << PreviousInputFilePath << " "
                            << InputFilePath
                            << "\nTriple Values: " << Stub.Triple << " "
                            << TargetStub->Triple << "\n";
         return -1;
       }
       if (Stub.SOName != TargetStub->SOName) {
for (auto Symbol : TargetStub->Symbols) {
       if (Symbol.Size != SI->second.Size) {
         WithColor::error() << "Interface Stub: Size Mismatch for "
                            << Symbol.Name << ".\nFilename: " << InputFilePath
                            << "\nSize Values: " << SI->second.Size << " "
                            << Symbol.Size << "\n";
 
         return -1;
       }
       if (Symbol.Weak != SI->second.Weak) {
-        // TODO: Add conflict resolution for Weak vs non-Weak.
-        WithColor::error() << "Interface Stub: Weak Mismatch for "
-                           << Symbol.Name << ".\nFilename: " << InputFilePath
-                           << "\nWeak Values: " << SI->second.Weak << " "
-                           << Symbol.Weak << "\n";
-
-        return -1;
+        Symbol.Weak = false;
+        continue;
       }
       // TODO: Not checking Warning. Will be dropped.
     }
 
     PreviousInputFilePath = InputFilePath;
   }
 
   if (Stub.IfsVersion != IFSVersionCurrent)