[lld] 366df11 - [lld-macho] Rework mergeFlag to behave closer to what ld64 does.

[Vy Nguyen via llvm-commits]

Author: Vy Nguyen
Date: 2021-06-17T14:22:58-04:00
New Revision: 366df11a35392c946678f1af94038945c23f06c8

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

LOG: [lld-macho] Rework mergeFlag to behave closer to what ld64 does.

Details:
I've been getting a few weird errors similar to the following from our internal tests:

```
ld64.lld.darwinnew: error: Cannot merge section __eh_frame (type=0x0) into __eh_frame (type=0xB): inconsistent types
ld64.lld.darwinnew: error: Cannot merge section __eh_frame (flags=0x0) into __eh_frame (flags=0x6800000B): strict flags differ
ld64.lld.darwinnew: error: Cannot merge section __eh_frame (type=0x0) into __eh_frame (type=0xB): inconsistent types
ld64.lld.darwinnew: error: Cannot merge section __eh_frame (flags=0x0) into __eh_frame (flags=0x6800000B): strict flags differ
```

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

Added: 
    

Modified: 
    lld/MachO/ConcatOutputSection.cpp
    lld/MachO/ConcatOutputSection.h
    lld/test/MachO/builtin-rename.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp
index 232ab5ce39fd..ca4ab2d2381a 100644
--- a/lld/MachO/ConcatOutputSection.cpp
+++ b/lld/MachO/ConcatOutputSection.cpp
@@ -30,7 +30,7 @@ void ConcatOutputSection::addInput(ConcatInputSection *input) {
     flags = input->flags;
   } else {
     align = std::max(align, input->align);
-    mergeFlags(input);
+    finalizeFlags(input);
   }
   inputs.push_back(input);
   input->parent = this;
@@ -331,30 +331,29 @@ void ConcatOutputSection::writeTo(uint8_t *buf) const {
   }
 }
 
-// TODO: this is most likely wrong; reconsider how section flags
-// are actually merged. The logic presented here was written without
-// any form of informed research.
-void ConcatOutputSection::mergeFlags(InputSection *input) {
-  uint8_t baseType = sectionType(flags);
-  uint8_t inputType = sectionType(input->flags);
-  if (baseType != inputType)
-    error("Cannot merge section " + input->name + " (type=0x" +
-          to_hexString(inputType) + ") into " + name + " (type=0x" +
-          to_hexString(baseType) + "): inconsistent types");
-
-  constexpr uint32_t strictFlags = S_ATTR_DEBUG | S_ATTR_STRIP_STATIC_SYMS |
-                                   S_ATTR_NO_DEAD_STRIP | S_ATTR_LIVE_SUPPORT;
-  if ((input->flags ^ flags) & strictFlags)
-    error("Cannot merge section " + input->name + " (flags=0x" +
-          to_hexString(input->flags) + ") into " + name + " (flags=0x" +
-          to_hexString(flags) + "): strict flags 
diff er");
-
-  // Negate pure instruction presence if any section isn't pure.
-  uint32_t pureMask = ~S_ATTR_PURE_INSTRUCTIONS | (input->flags & flags);
-
-  // Merge the rest
-  flags |= input->flags;
-  flags &= pureMask;
+void ConcatOutputSection::finalizeFlags(InputSection *input) {
+  uint8_t inputType = input->flags & SECTION_TYPE;
+  switch (inputType) {
+  default /*type-unspec'ed*/:
+    // FIXME: Add additional logics here when supporting emitting obj files.
+    break;
+  case S_4BYTE_LITERALS:
+  case S_8BYTE_LITERALS:
+  case S_16BYTE_LITERALS:
+  case S_CSTRING_LITERALS:
+  case S_ZEROFILL:
+  case S_LAZY_SYMBOL_POINTERS:
+  case S_MOD_TERM_FUNC_POINTERS:
+  case S_THREAD_LOCAL_REGULAR:
+  case S_THREAD_LOCAL_ZEROFILL:
+  case S_THREAD_LOCAL_VARIABLES:
+  case S_THREAD_LOCAL_INIT_FUNCTION_POINTERS:
+  case S_THREAD_LOCAL_VARIABLE_POINTERS:
+  case S_NON_LAZY_SYMBOL_POINTERS:
+  case S_SYMBOL_STUBS:
+    flags |= input->flags;
+    break;
+  }
 }
 
 void ConcatOutputSection::eraseOmittedInputSections() {

diff  --git a/lld/MachO/ConcatOutputSection.h b/lld/MachO/ConcatOutputSection.h
index dad87e3388b2..f8332f6410d4 100644
--- a/lld/MachO/ConcatOutputSection.h
+++ b/lld/MachO/ConcatOutputSection.h
@@ -52,7 +52,7 @@ class ConcatOutputSection final : public OutputSection {
   }
 
 private:
-  void mergeFlags(InputSection *input);
+  void finalizeFlags(InputSection *input);
 
   size_t size = 0;
   uint64_t fileSize = 0;

diff  --git a/lld/test/MachO/builtin-rename.s b/lld/test/MachO/builtin-rename.s
index 5cae3e01297b..faa003090e83 100644
--- a/lld/test/MachO/builtin-rename.s
+++ b/lld/test/MachO/builtin-rename.s
@@ -5,14 +5,6 @@
 # RUN:     %t/main.s -o %t/main.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin \
 # RUN:     %t/renames.s -o %t/renames.o
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin \
-# RUN:     %t/error.s -o %t/error.o
-
-# RUN: not %lld            -o %t/error %t/main.o %t/error.o -lSystem 2>&1 | \
-# RUN:     FileCheck %s --check-prefix=ERROR
-
-## Check the error diagnostic for merging mismatched section types
-# ERROR: Cannot merge section __pointers (type=0x0) into __nl_symbol_ptr (type=0x6): inconsistent types
 
 ## Check that section and segment renames happen as expected
 # RUN: %lld                -o %t/ydata %t/main.o %t/renames.o -lSystem
@@ -151,18 +143,6 @@ __IMPORT__pointers:
 __TEXT__StaticInit:
   .space 8
 
-#--- error.s
-
-.section __DATA,__nl_symbol_ptr
-.global __DATA__nl_symbol_ptr
-__DATA__nl_symbol_ptr:
-  .space 8
-
-.section __IMPORT,__pointers
-.global __IMPORT__pointers
-__IMPORT__pointers:
-  .space 8
-
 #--- main.s
 .text
 .global _main