[PATCH] D112195: [lld-macho] Simplify lc-linker-option.ll and re-enable it on Windows

[Jez Ng via Phabricator via llvm-commits]

int3 created this revision.
int3 added a reviewer: lld-macho.
Herald added a reviewer: gkm.
Herald added a project: lld-macho.
int3 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

While attempting to simplify it, I discovered a concerning discrepancy
between our handling of LC_LINKER_OPTION vs ld64's. In particular, ld64
does not appear to check for `-all_load` nor `-ObjC` when processing
those options. Thus, if/when we fix this behavior, no duplicate symbol
error will be expected regardless of the use-after-free. As such, I've
removed the test logic that tries to induce the duplicate symbol error.
We can just rely on ASAN to do the verification.

In order to make the test run on Windows, I've removed the symlink
logic. Both ld64 and LLD handle this un-symlinked framework just fine.

I also capitalized the framework name, since that's the typical
convention.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112195

Files:
  lld/test/MachO/lc-linker-option.ll


Index: lld/test/MachO/lc-linker-option.ll
===================================================================
--- lld/test/MachO/lc-linker-option.ll
+++ lld/test/MachO/lc-linker-option.ll
@@ -48,22 +48,19 @@
 ; RUN: not %lld %t/invalid.o -o /dev/null 2>&1 | FileCheck --check-prefix=INVALID %s
 ; INVALID: error: -why_load is not allowed in LC_LINKER_OPTION
 
-;; We are testing this because we want to check a dangling string reference problem (see https://reviews.llvm.org/D111706).
-;; To trigger this problem, we need to create a framework that is an archive,
-;; and it needs to contain a symbol starting with OBJC_CLASS_$.
-;; The bug is triggered as the linker loads this framework twice via LC_LINKER_OPTION.
-;; When the linker adds this framework, it will fail to map the path of framework to this archive due to dangling reference.
-;; Therefore, it will load the framework twice, and if there is any symbol with OBJC_CLASS_$ prefix with forceLoadObjC enabled,
-;; the linker will fetch this symbol twice, which leads to a duplicate symbol error.
+;; This is a regression test for a dangling string reference issue that occurred
+;; when loading an archive-based framework via LC_LINKER_OPTION (see
+;; D111706). Prior to the fix, this would trigger a heap-use-after-free when run
+;; under ASAN.
 ; RUN: llc %t/foo.ll -o %t/foo.o -filetype=obj
-; RUN: mkdir -p %t/foo.framework/Versions/A
-; RUN: llvm-ar rcs %t/foo.framework/Versions/A/foo %t/foo.o
-; RUN: ln -sf A %t/foo.framework/Versions/Current
-; RUN: ln -sf Versions/Current/foo %t/foo.framework/foo
+; RUN: mkdir -p %t/Foo.framework
+;; In a proper framework, this is technically supposed to be a symlink to the
+;; actual archive at Foo.framework/Versions/Current, but we skip that here so
+;; that this test can run on Windows.
+; RUN: llvm-ar rcs %t/Foo.framework/Foo %t/foo.o
 ; RUN: llc %t/objfile1.ll -o %t/objfile1.o -filetype=obj
-; RUN: llc %t/objfile2.ll -o %t/objfile2.o -filetype=obj
 ; RUN: llc %t/main.ll -o %t/main.o -filetype=obj
-; RUN: %lld -demangle -ObjC %t/objfile1.o %t/objfile2.o %t/main.o -o %t/main.out -arch x86_64 -platform_version macos 11.0.0 0.0.0 -F%t
+; RUN: %lld %t/objfile1.o %t/main.o -o /dev/null -F%t
 
 ;--- framework.ll
 target triple = "x86_64-apple-macosx10.15.0"
@@ -112,14 +109,7 @@
 target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
-!0 = !{!"-framework", !"foo"}
-!llvm.linker.options = !{!0}
-
-;--- objfile2.ll
-target triple = "x86_64-apple-macosx10.15.0"
-target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-
-!0 = !{!"-framework", !"foo"}
+!0 = !{!"-framework", !"Foo"}
 !llvm.linker.options = !{!0}
 
 ;--- main.ll
@@ -130,11 +120,13 @@
   ret void
 }
 
+!0 = !{!"-framework", !"Foo"}
+!llvm.linker.options = !{!0}
+
 ;--- foo.ll
 target triple = "x86_64-apple-macosx10.15.0"
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
-%0 = type opaque
-%struct._class_t = type {}
-
-@"OBJC_CLASS_$_TestClass" = global %struct._class_t {}, section "__DATA, __objc_data", align 8
+define void @foo() {
+  ret void
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112195.381137.patch
Type: text/x-patch
Size: 3208 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211021/1ca8405a/attachment.bin>