[lld-macho][objc] Don't crash when rodata isn't statically linked

[int3]

Summary

We were previously assuming that to hold, but it appears that swiftc
may generate classes that link against __objc_empty_cache for their
rodata pointer.

This should allow us to re-land D142916: [lld-macho] Warn on method name collisions from category definitions (as a stack).

[int3]

@oontvoo wanna test this on the previous crashes you were seeing before I land it?

[oontvoo]

In D147229#4233402, @int3 wrote:

@oontvoo wanna test this on the previous crashes you were seeing before I land it?

yes - testing this now - give me ~2 hrs

[oontvoo]

P.S: Wait, this should be stacked on top of the previous patch, yes? (the one that supported emitting the warnings?)

[int3]

P.S: Wait, this should be stacked on top of the previous patch, yes? (the one that supported emitting the warnings?)

Yep that's right

[oontvoo]

In D147229#4233506, @oontvoo wrote:

In D147229#4233402, @int3 wrote:

@oontvoo wanna test this on the previous crashes you were seeing before I land it?

yes - testing this now - give me ~2 hrs

P.S: Seems to work! (no longer saw the previous crash) 👍
Thanks!

[int3]

Awesome!

Wanna stamp D147230 too? Otherwise you'll see spurious warnings when I land this :p

lld/MachO/ObjC.cpp

void ObjcCategoryChecker::parseCategory(const ConcatInputSection *catIsec) {
   }
 }
 
 void ObjcCategoryChecker::parseClass(const Defined *classSym) {
   // Given a Class struct, get its corresponding Methods struct
   auto getMethodsIsec =
       [&](const InputSection *classIsec) -> ConcatInputSection * {
     if (const auto *r = classIsec->getRelocAt(classLayout.roDataOffset)) {
-      const auto *roIsec =
-          cast<ConcatInputSection>(r->getReferentInputSection());
-      if (const auto *r = roIsec->getRelocAt(roClassLayout.baseMethodsOffset)) {
-        if (auto *methodsIsec =
-                cast_or_null<ConcatInputSection>(r->getReferentInputSection()))
+      if (const auto *roIsec =
+              cast_or_null<ConcatInputSection>(r->getReferentInputSection())) {
+        if (const auto *r =
+                roIsec->getRelocAt(roClassLayout.baseMethodsOffset)) {
+          if (auto *methodsIsec = cast_or_null<ConcatInputSection>(
+                  r->getReferentInputSection()))
             return methodsIsec;
         }
       }
+    }
     return nullptr;
   };
 
   const auto *classIsec = cast<ConcatInputSection>(classSym->isec);
 
   // Parse instance methods.
   if (const auto *instanceMethodsIsec = getMethodsIsec(classIsec))
     parseMethods(instanceMethodsIsec, classSym, classIsec, MCK_Class,

lld/test/MachO/objc-category-conflicts.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/cat1.s -o %t/cat1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/cat2.s -o %t/cat2.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/klass.s -o %t/klass.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/klass-with-no-rodata.s -o %t/klass-with-no-rodata.o
 # RUN: %lld -dylib -lobjc %t/klass.o -o %t/libklass.dylib
 
 # RUN: %no-fatal-warnings-lld -dylib -lobjc %t/klass.o %t/cat1.o %t/cat2.o -o \
 # RUN:   /dev/null 2>&1 | FileCheck %s --check-prefixes=CATCLS,CATCAT
 # RUN: %no-fatal-warnings-lld -dylib -lobjc %t/libklass.dylib %t/cat1.o \
 # RUN:   %t/cat2.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=CATCAT
 
+## Regression test: Check that we don't crash.
+# RUN: %no-fatal-warnings-lld -dylib -lobjc %t/klass-with-no-rodata.o -o /dev/null
+
 # CATCLS:      warning: method '+s1' has conflicting definitions:
 # CATCLS-NEXT: >>> defined in category Cat1 from {{.*}}cat1.o
 # CATCLS-NEXT: >>> defined in class Foo from {{.*}}klass.o
 
 # CATCLS:      warning: method '-m1' has conflicting definitions:
 # CATCLS-NEXT: >>> defined in category Cat1 from {{.*}}cat1.o
 # CATCLS-NEXT: >>> defined in class Foo from {{.*}}klass.o
 
.section __DATA,__objc_classlist,regular,no_dead_strip
   .quad _OBJC_CLASS_$_Foo
 
 .section __DATA,__objc_imageinfo,regular,no_dead_strip
   .long 0
   .long 64
 
 .subsections_via_symbols
 
+#--- klass-with-no-rodata.s
+
+.include "objc-macros.s"
+
+## swiftc generates some classes without a statically-linked rodata. Not
+## entirely sure what the corresponding Swift inputs are required for this to
+## happen; this test merely checks that we can gracefully handle this case
+## without crashing.
+## FIXME: It would be better if this test used the output of some real Swift
+## code.
+
+.globl _$s11FooAACfD
+
+.section __DATA,__objc_data
+_$s11FooAACfD:
+  .quad _$s11FooAACfD
+  .quad 0
+  .quad __objc_empty_cache
+  .quad 0
+  .quad __objc_empty_cache
+
+.section __DATA,__objc_catlist,regular,no_dead_strip
+  .quad __CATEGORY_METAFoo_$_Foo20
+
+.section __DATA,__objc_const
+__CATEGORY_METAFoo_$_Foo20:
+  .objc_classname "Foo20"
+  .quad _$s11FooAACfD
+  .quad 0
+  .quad 0
+  .quad 0
+  .quad 0
+  .quad 0
+  .long 64
+  .space 4
+
 #--- objc-macros.s
 
 # Macros for taking some of the boilerplate out of defining objc structs.
 
 # NOTE: \@ below is a variable that gets auto-incremented by the assembler on
 # each macro invocation. It serves as a mechanism for generating unique symbol
 # names for each macro call.