Skip to content

[LLD, MachO] Default objc_relative_method_lists on MacOS11+/iOS14+ #101360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 7, 2024
Merged

[LLD, MachO] Default objc_relative_method_lists on MacOS11+/iOS14+ #101360

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f50084
[LLD, MachO] Default objc_relative_method_lists on MacOS11+/iOS14+
DataCorrupted Jul 30, 2024
f40e80c
Update comments in the tests; Create a test to check if flag is defau…
DataCorrupted Jul 31, 2024
dee85bc
Merge two tests into one.
DataCorrupted Aug 1, 2024
d82b683
Default relative method lest: all platforms are included
DataCorrupted Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions lld/MachO/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1140,12 +1140,20 @@ static bool shouldEmitRelativeMethodLists(const InputArgList &args) {
if (arg && arg->getOption().getID() == OPT_no_objc_relative_method_lists)
return false;

// TODO: If no flag is specified, don't default to false, but instead:
// - default false on < ios14
// - default true on >= ios14
// For now, until this feature is confirmed stable, default to false if no
// flag is explicitly specified
return false;
// If no flag is specified:
// - default true on >= ios14/macos11
// - default false on everything else
switch (config->platformInfo.target.Platform) {
case PLATFORM_IOS:
case PLATFORM_IOSSIMULATOR:
return config->platformInfo.target.MinDeployment >= VersionTuple(14, 0);
case PLATFORM_MACOS:
return config->platformInfo.target.MinDeployment >= VersionTuple(11, 0);
default:
return false;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we can't enable it for the other platforms?

See for example what we do for chained fixups (which based on my understanding was basically introduced alongside this feature):

llvm-project/lld/MachO/Driver.cpp

Lines 1109 to 1132 in 0512ba0

static const std::array<std::pair<PlatformType, VersionTuple>, 9> minVersion =
{{
{PLATFORM_IOS, VersionTuple(13, 4)},
{PLATFORM_IOSSIMULATOR, VersionTuple(16, 0)},
{PLATFORM_MACOS, VersionTuple(13, 0)},
{PLATFORM_TVOS, VersionTuple(14, 0)},
{PLATFORM_TVOSSIMULATOR, VersionTuple(15, 0)},
{PLATFORM_WATCHOS, VersionTuple(7, 0)},
{PLATFORM_WATCHOSSIMULATOR, VersionTuple(8, 0)},
{PLATFORM_XROS, VersionTuple(1, 0)},
{PLATFORM_XROS_SIMULATOR, VersionTuple(1, 0)},
}};
PlatformType platform = config->platformInfo.target.Platform;
auto it = llvm::find_if(minVersion,
[&](const auto &p) { return p.first == platform; });
// We don't know the versions for other platforms, so default to disabled.
if (it == minVersion.end())
return false;
if (it->second > config->platformInfo.target.MinDeployment)
return false;
return true;

Looks like ld64 special-cases x86, but other than that, the version2020Fall condition covers watchOS, tvOS and friends as well.

https://github.com/apple-oss-distributions/ld64/blob/47f477cb721755419018f7530038b272e9d0cdea/src/ld/Options.cpp#L6085-L6101

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can definitely add those platforms as well.

But can you shed more light how to special case x86 like the code you mentioned?

I don't find equivalence of Options::kDynamicExecutable in LLVM yet, only MH_EXECUTE.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think that part is safe to ignore, no need to special case it.

Apple might need to back-deploy executables during macOS's development process, but that is not relevant for user code.

llvm_unreachable("RelativeMethodList should default to false, control flow "
"should not reach here");
}

void SymbolPatterns::clear() {
Expand Down
19 changes: 11 additions & 8 deletions lld/test/MachO/objc-category-conflicts.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# `-no_objc_relative_method_lists` needs to be explicitly added to this test to avoid crashing after `-objc_relative_method_lists` was made default.
# TODO: Make this test compatible with default `-objc_relative_method_lists` and remove the `-no_objc_relative_method_lists` flag. Issue #101419

# 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
Expand All @@ -10,31 +13,31 @@
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/cat2.s --defsym MAKE_LOAD_METHOD=1 -o %t/cat2-with-load.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos11.0 -I %t %t/klass.s --defsym MAKE_LOAD_METHOD=1 -o %t/klass-with-load.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: %lld -no_objc_relative_method_lists -dylib -lobjc %t/klass.o -o %t/libklass.dylib

# RUN: %no-fatal-warnings-lld --check-category-conflicts -dylib -lobjc %t/klass.o %t/cat1.o %t/cat2.o -o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -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 --check-category-conflicts -dylib -lobjc %t/libklass.dylib %t/cat1.o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -dylib -lobjc %t/libklass.dylib %t/cat1.o \
# RUN: %t/cat2.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=CATCAT

# RUN: %no-fatal-warnings-lld --check-category-conflicts -dylib -lobjc %t/klass_w_sym.o %t/cat1_w_sym.o %t/cat2_w_sym.o -o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -dylib -lobjc %t/klass_w_sym.o %t/cat1_w_sym.o %t/cat2_w_sym.o -o \
# RUN: /dev/null 2>&1 | FileCheck %s --check-prefixes=CATCLS_W_SYM,CATCAT_W_SYM
# RUN: %no-fatal-warnings-lld --check-category-conflicts -dylib -lobjc %t/libklass.dylib %t/cat1_w_sym.o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -dylib -lobjc %t/libklass.dylib %t/cat1_w_sym.o \
# RUN: %t/cat2_w_sym.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=CATCAT_W_SYM

## Check that we don't emit spurious warnings around the +load method while
## still emitting the other warnings. Note that we have made separate
## `*-with-load.s` files for ease of comparison with ld64; ld64 will not warn
## at all if multiple +load methods are present.
# RUN: %no-fatal-warnings-lld --check-category-conflicts -dylib -lobjc %t/klass-with-load.o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -dylib -lobjc %t/klass-with-load.o \
# RUN: %t/cat1-with-load.o %t/cat2-with-load.o -o /dev/null 2>&1 | \
# RUN: FileCheck %s --check-prefixes=CATCLS,CATCAT --implicit-check-not '+load'

## Regression test: Check that we don't crash.
# RUN: %no-fatal-warnings-lld --check-category-conflicts -dylib -lobjc %t/klass-with-no-rodata.o -o /dev/null
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists --check-category-conflicts -dylib -lobjc %t/klass-with-no-rodata.o -o /dev/null

## Check that we don't emit any warnings without --check-category-conflicts.
# RUN: %no-fatal-warnings-lld -dylib -lobjc %t/klass.o %t/cat1.o %t/cat2.o -o \
# RUN: %no-fatal-warnings-lld -no_objc_relative_method_lists -dylib -lobjc %t/klass.o %t/cat1.o %t/cat2.o -o \
# RUN: /dev/null 2>&1 | FileCheck %s --implicit-check-not 'warning' --allow-empty

# CATCLS: warning: method '+s1' has conflicting definitions:
Expand Down
15 changes: 9 additions & 6 deletions lld/test/MachO/objc-category-merging-complete-test.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
# `-no_objc_relative_method_lists` needs to be explicitly added to this test to avoid crashing after `-objc_relative_method_lists` was made default.
# TODO: Make this test compatible with default `-objc_relative_method_lists` and remove the `-no_objc_relative_method_lists` flag. Issue #101419

# REQUIRES: aarch64
# RUN: rm -rf %t; split-file %s %t && cd %t

############ Test merging multiple categories into a single category ############
## Create a dylib to link against(a64_file1.dylib) and merge categories in the main binary (file2_merge_a64.exe)
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o a64_file1.o a64_file1.s
# RUN: %lld -arch arm64 a64_file1.o -o a64_file1.dylib -dylib
# RUN: %lld -no_objc_relative_method_lists -arch arm64 a64_file1.o -o a64_file1.dylib -dylib

# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o a64_file2.o a64_file2.s
# RUN: %lld -arch arm64 -o a64_file2_no_merge.exe a64_file1.dylib a64_file2.o
# RUN: %lld -arch arm64 -o a64_file2_no_merge_v2.exe a64_file1.dylib a64_file2.o -no_objc_category_merging
# RUN: %lld -arch arm64 -o a64_file2_no_merge_v3.exe a64_file1.dylib a64_file2.o -objc_category_merging -no_objc_category_merging
# RUN: %lld -arch arm64 -o a64_file2_merge.exe -objc_category_merging a64_file1.dylib a64_file2.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -o a64_file2_no_merge.exe a64_file1.dylib a64_file2.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -o a64_file2_no_merge_v2.exe a64_file1.dylib a64_file2.o -no_objc_category_merging
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -o a64_file2_no_merge_v3.exe a64_file1.dylib a64_file2.o -objc_category_merging -no_objc_category_merging
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -o a64_file2_merge.exe -objc_category_merging a64_file1.dylib a64_file2.o

# RUN: llvm-objdump --objc-meta-data --macho a64_file2_no_merge.exe | FileCheck %s --check-prefixes=NO_MERGE_CATS
# RUN: llvm-objdump --objc-meta-data --macho a64_file2_no_merge_v2.exe | FileCheck %s --check-prefixes=NO_MERGE_CATS
# RUN: llvm-objdump --objc-meta-data --macho a64_file2_no_merge_v3.exe | FileCheck %s --check-prefixes=NO_MERGE_CATS
# RUN: llvm-objdump --objc-meta-data --macho a64_file2_merge.exe | FileCheck %s --check-prefixes=MERGE_CATS

############ Test merging multiple categories into the base class ############
# RUN: %lld -arch arm64 -o a64_file2_merge_into_class.exe -objc_category_merging a64_file1.o a64_file2.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -o a64_file2_merge_into_class.exe -objc_category_merging a64_file1.o a64_file2.o
# RUN: llvm-objdump --objc-meta-data --macho a64_file2_merge_into_class.exe | FileCheck %s --check-prefixes=MERGE_CATS_CLS


Expand Down
5 changes: 4 additions & 1 deletion lld/test/MachO/objc-category-merging-erase-objc-name-test.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
# `-no_objc_relative_method_lists` needs to be explicitly added to this test to avoid crashing after `-objc_relative_method_lists` was made default.
# TODO: Make this test compatible with default `-objc_relative_method_lists` and remove the `-no_objc_relative_method_lists` flag. Issue #101419

; REQUIRES: aarch64

; Here we test that if we defined a protocol MyTestProtocol and also a category MyTestProtocol
; then when merging the category into the base class (and deleting the category), we don't
; delete the 'MyTestProtocol' name

; RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o %T/erase-objc-name.o %s
; RUN: %lld -arch arm64 -dylib -o %T/erase-objc-name.dylib %T/erase-objc-name.o -objc_category_merging
; RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o %T/erase-objc-name.dylib %T/erase-objc-name.o -objc_category_merging
; RUN: llvm-objdump --objc-meta-data --macho %T/erase-objc-name.dylib | FileCheck %s --check-prefixes=MERGE_CATS

; === Check merge categories enabled ===
Expand Down
15 changes: 9 additions & 6 deletions lld/test/MachO/objc-category-merging-minimal.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,34 @@
# `-no_objc_relative_method_lists` needs to be explicitly added to this test to avoid crashing after `-objc_relative_method_lists` was made default.
# TODO: Make this test compatible with default `-objc_relative_method_lists` and remove the `-no_objc_relative_method_lists` flag. Issue #101419

# REQUIRES: aarch64
# RUN: rm -rf %t; split-file %s %t && cd %t

############ Test merging multiple categories into a single category ############
## Create a dylib with a fake base class to link against in when merging between categories
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o a64_fakedylib.o a64_fakedylib.s
# RUN: %lld -arch arm64 a64_fakedylib.o -o a64_fakedylib.dylib -dylib
# RUN: %lld -no_objc_relative_method_lists -arch arm64 a64_fakedylib.o -o a64_fakedylib.dylib -dylib

## Create our main testing dylib - linking against the fake dylib above
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o merge_cat_minimal.o merge_cat_minimal.s
# RUN: %lld -arch arm64 -dylib -o merge_cat_minimal_no_merge.dylib a64_fakedylib.dylib merge_cat_minimal.o
# RUN: %lld -arch arm64 -dylib -o merge_cat_minimal_merge.dylib -objc_category_merging a64_fakedylib.dylib merge_cat_minimal.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o merge_cat_minimal_no_merge.dylib a64_fakedylib.dylib merge_cat_minimal.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o merge_cat_minimal_merge.dylib -objc_category_merging a64_fakedylib.dylib merge_cat_minimal.o

## Now verify that the flag caused category merging to happen appropriatelly
# RUN: llvm-objdump --objc-meta-data --macho merge_cat_minimal_no_merge.dylib | FileCheck %s --check-prefixes=NO_MERGE_CATS
# RUN: llvm-objdump --objc-meta-data --macho merge_cat_minimal_merge.dylib | FileCheck %s --check-prefixes=MERGE_CATS

############ Test merging multiple categories into the base class ############
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o merge_base_class_minimal.o merge_base_class_minimal.s
# RUN: %lld -arch arm64 -dylib -o merge_base_class_minimal_yes_merge.dylib -objc_category_merging merge_base_class_minimal.o merge_cat_minimal.o
# RUN: %lld -arch arm64 -dylib -o merge_base_class_minimal_no_merge.dylib merge_base_class_minimal.o merge_cat_minimal.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o merge_base_class_minimal_yes_merge.dylib -objc_category_merging merge_base_class_minimal.o merge_cat_minimal.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o merge_base_class_minimal_no_merge.dylib merge_base_class_minimal.o merge_cat_minimal.o

# RUN: llvm-objdump --objc-meta-data --macho merge_base_class_minimal_no_merge.dylib | FileCheck %s --check-prefixes=NO_MERGE_INTO_BASE
# RUN: llvm-objdump --objc-meta-data --macho merge_base_class_minimal_yes_merge.dylib | FileCheck %s --check-prefixes=YES_MERGE_INTO_BASE

############ Test merging swift category into the base class ############
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o MyBaseClassSwiftExtension.o MyBaseClassSwiftExtension.s
# RUN: %lld -arch arm64 -dylib -o merge_base_class_swift_minimal_yes_merge.dylib -objc_category_merging MyBaseClassSwiftExtension.o merge_base_class_minimal.o
# RUN: %lld -no_objc_relative_method_lists -arch arm64 -dylib -o merge_base_class_swift_minimal_yes_merge.dylib -objc_category_merging MyBaseClassSwiftExtension.o merge_base_class_minimal.o
# RUN: llvm-objdump --objc-meta-data --macho merge_base_class_swift_minimal_yes_merge.dylib | FileCheck %s --check-prefixes=YES_MERGE_INTO_BASE_SWIFT

#### Check merge categories enabled ###
Expand Down
25 changes: 18 additions & 7 deletions lld/test/MachO/objc-relative-method-lists-simple.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,34 @@
# UNSUPPORTED: target=arm{{.*}}-unknown-linux-gnueabihf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add cases to test the new logic. Specifically:

MacOS 10 => defaults to not relative
MacOS 11 => defaults to relative

MacOS 10 + -objc_relative_method_lists => relative
MacOS 12 + -no_objc_relative_method_lists=> not relative

Copy link
Member Author

@DataCorrupted DataCorrupted Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. MacOS 10 => defaults to not relative
  2. MacOS 11 => defaults to relative
  3. MacOS 10 + -objc_relative_method_lists => relative
  4. MacOS 12 + -no_objc_relative_method_lists=> not relative

(2) Should be covered by removing -objc_relative_method_lists in the tests

(4) Should be covered by unchanged -no_objc_relative_method_lists on Line 17 (on MacOS 11 tho)

I need to modify lit config to test 1 and 3 (our lit config defaults to MacOS 11 now). I tried yesterday but with no luck. Let me try it again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our lit config defaults to MacOS 11 now

Why change lit configs - why not manually overwrite on the command line ? Like here.

# RUN: rm -rf %t; split-file %s %t && cd %t

## Compile a64_rel_dylib.o
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos -o a64_rel_dylib.o a64_simple_class.s
## Compile a64_rel_dylib.o with MacOS 11
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos11 -o a64_rel_dylib.o a64_simple_class.s

## Test arm64 + relative method lists
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -objc_relative_method_lists
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL

## Test arm64 + relative method lists + dead-strip
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -objc_relative_method_lists -dead_strip
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -dead_strip
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL

## Test arm64 + traditional method lists (no relative offsets)
# RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -no_objc_relative_method_lists
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_NO_REL

## Compile a64_rel_dylib.o with MacOS 10
# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos10 -o a64_rel_dylib.o a64_simple_class.s

CHK_REL: Contents of (__DATA_CONST,__objc_classlist) section
## Test arm64 + relative method lists by explicitly adding `-objc_relative_method_lists`.
# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.0 10.0 -objc_relative_method_lists
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_REL

## Test arm64 + no relative method lists by default.
# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.0 10.0
# RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib | FileCheck %s --check-prefix=CHK_NO_REL


CHK_REL: Contents of (__DATA{{(_CONST)?}},__objc_classlist) section
CHK_REL-NEXT: _OBJC_CLASS_$_MyClass
CHK_REL: baseMethods
CHK_REL-NEXT: entsize 12 (relative)
Expand Down Expand Up @@ -51,7 +62,7 @@ CHK_REL-NEXT: imp 0x{{[0-9a-f]*}} (0x{{[0-9a-f]*}}) +[MyClass class_method_0

CHK_NO_REL-NOT: (relative)

CHK_NO_REL: Contents of (__DATA_CONST,__objc_classlist) section
CHK_NO_REL: Contents of (__DATA{{(_CONST)?}},__objc_classlist) section
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CONST doesn't show up on MacOS 10

CHK_NO_REL-NEXT: _OBJC_CLASS_$_MyClass

CHK_NO_REL: baseMethods 0x{{[0-9a-f]*}} (struct method_list_t *)
Expand Down Expand Up @@ -125,7 +136,7 @@ CHK_NO_REL-NEXT: imp +[MyClass class_method_02]
.include "objc-macros.s"

.section __TEXT,__text,regular,pure_instructions
.build_version macos, 11, 0
.build_version macos, 10, 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build version still required or llvm-mc defaults to 11 somehow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It produces 11 output even when specifying -triple=arm64-apple-macos10 ?
Or where does it default - in this test we specificly specify either 10 or 11 triple in llvm-mc invocation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing the "old" target to 10.15 instead? -data_const is already the default for that version, so the change to the segment name above wouldn't be needed.

Even if we do say that our generated binaries should run on macOS 10.0 (from 2001!), they won't. Dyld opcodes -- which we use for pre-chained-fixups targets -- were introduced around the 10.9 times.

Copy link
Member Author

@DataCorrupted DataCorrupted Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10.15 still generates -data, any tips? (bumping to 10.16 would enable relative method list by default and invalidate the purpose of testing)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work:

--- a/lld/test/MachO/objc-relative-method-lists-simple.s
+++ b/lld/test/MachO/objc-relative-method-lists-simple.s
@@ -2,8 +2,8 @@
 # UNSUPPORTED: target=arm{{.*}}-unknown-linux-gnueabihf
 # RUN: rm -rf %t; split-file %s %t && cd %t
 
-## Compile a64_rel_dylib.o with MacOS 11
-# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos11 -o a64_rel_dylib.o a64_simple_class.s
+## Compile a64_rel_dylib.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos10.15 -o a64_rel_dylib.o a64_simple_class.s
 
 ## Test arm64 + relative method lists
 # RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64
@@ -17,19 +17,16 @@
 # RUN: %no-lsystem-lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -no_objc_relative_method_lists
 # RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib  | FileCheck %s --check-prefix=CHK_NO_REL
 
-## Compile a64_rel_dylib.o with MacOS 10
-# RUN: llvm-mc -filetype=obj -triple=arm64-apple-macos10 -o a64_rel_dylib.o a64_simple_class.s
-
 ## Test arm64 + relative method lists by explicitly adding `-objc_relative_method_lists`.
-# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.0 10.0 -objc_relative_method_lists
+# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.15 10.15 -objc_relative_method_lists
 # RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib  | FileCheck %s --check-prefix=CHK_REL
 
 ## Test arm64 + no relative method lists by default.
-# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.0 10.0
+# RUN: %lld a64_rel_dylib.o -o a64_rel_dylib.dylib -map a64_rel_dylib.map -dylib -arch arm64 -platform_version macOS 10.15 10.15
 # RUN: llvm-objdump --macho --objc-meta-data a64_rel_dylib.dylib  | FileCheck %s --check-prefix=CHK_NO_REL
 
 
-CHK_REL:       Contents of (__DATA{{(_CONST)?}},__objc_classlist) section
+CHK_REL:       Contents of (__DATA_CONST,__objc_classlist) section
 CHK_REL-NEXT:  _OBJC_CLASS_$_MyClass
 CHK_REL:       baseMethods
 CHK_REL-NEXT:  entsize 12 (relative)
@@ -62,7 +59,7 @@ CHK_REL-NEXT:    imp 0x{{[0-9a-f]*}} (0x{{[0-9a-f]*}})  +[MyClass class_method_0
 
 CHK_NO_REL-NOT: (relative)
 
-CHK_NO_REL:           Contents of (__DATA{{(_CONST)?}},__objc_classlist) section
+CHK_NO_REL:           Contents of (__DATA_CONST,__objc_classlist) section
 CHK_NO_REL-NEXT:      _OBJC_CLASS_$_MyClass
 
 CHK_NO_REL:            baseMethods 0x{{[0-9a-f]*}} (struct method_list_t *)
@@ -136,7 +133,7 @@ CHK_NO_REL-NEXT:		       imp +[MyClass class_method_02]
 .include "objc-macros.s"
 
 .section	__TEXT,__text,regular,pure_instructions
-.build_version macos, 10, 0
+.build_version macos, 10, 15
 
 .objc_selector_def "-[MyClass instance_method_00]"
 .objc_selector_def "-[MyClass instance_method_01]"
  • no need to run the assembler twice; we can link objects targeting 10.15 into macOS 11 binaries
  • had to change the version in the -platform_version flag

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh Cool! That works!


.objc_selector_def "-[MyClass instance_method_00]"
.objc_selector_def "-[MyClass instance_method_01]"
Expand Down
Loading