Make CoreCLR/NativeAOT assembly compile with .subsections_via_symbols on Apple platforms by filipnavara · Pull Request #92520 · dotnet/runtime

filipnavara · 2023-09-23T08:03:17Z

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

filipnavara · 2023-09-23T08:03:17Z

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

… on Apple platforms

ghost · 2023-09-23T08:03:30Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #92297 #92491

The new Xcode 15 linker mishandles object files without the MH_SUBSECTIONS_VIA_SYMBOLS flag (.subsections_via_symbols assembly code). It ends up treating the whole object file as one function and produces unwind info only for the first function in the file.

Author:	filipnavara
Assignees:	-
Labels:	`community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-09-23T08:11:55Z

+bhsLOCAL_LABEL(NotInHeap)

-NotInHeap:
+ b C_FUNC(RhpAssignRefArm64)


Conditional jump to non-local labels are not allowed in subsections-via-symbols mode. I updated 3-4 places to use non-conditional branches instead, it's mostly in the "rare" code paths.
Most of the diff is just decorating local labels with LOCAL_LABEL.

jkotas

LGTM. Thank you!

jkotas · 2023-09-23T23:33:46Z

/backport to release/8.0

github-actions · 2023-09-23T23:33:56Z

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6286618371

jkotas · 2023-09-24T00:17:49Z

We depend on assembly code to stay together here:

runtime/src/coreclr/vm/amd64/JitHelpers_Fast.asm

Lines 54 to 56 in 5dfd805

LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
ret
LEAF_END JIT_PatchedCodeStart, _TEXT

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

filipnavara · 2023-09-24T04:58:57Z

Can .subsections_via_symbols break this guarantee by reordering methods inside the file?

In theory it probably can. I'll check what the linker happens to do in practice. I don't think it does reordering, and we don't use "dead strip" or "identical code folding" options that could make this a real problem.

(good catch!)

filipnavara · 2023-09-24T06:43:30Z

~~I am blocked by unrelated build breakage (related to PR #92301 and discussed there).~~ I don't feel overly confident in the results of the analysis (with and without -dead_strip) that I gathered so far, so I will have to resolve the build breakage first to proceed further.

Meanwhile, I checked that some other platforms (eg. arm32) split of this code into patchedcode.S file. I suppose we don't depend on unwind information for these particular functions, especially since they are patched at runtime. Would it make sense to do the split for arm64 and amd64 too and turn off .subsections_via_symbols for them? That way we would get the original ordering/preservation guarantees and only lose the unwind information for this specific code [with the Xcode 15 linker].

filipnavara · 2023-09-24T07:43:16Z

Checked output on arm64 so far, both in the default configuration and with added -dead_strip, with the Xcode 15 default linker and the legacy one (-ld_classic switch). In all cases the code was preserved and didn't get reordered. So, from strictly practical point of view, the generated output is correct.

From the theoretical point of view, I think the linker is allowed to do some reordering. The comments in the old linker specifically mention "Atoms in the same section from the same .o file should be contiguous and sequenced in the same order they were in the .o file," but it's not obvious how hard the guarantee is and whether it applies only to the initial order or whether it can be affected by some subsequent optimization. The only two reordering optimizations that I am aware of are a) reordering initialiser/terminator functions to the start/end of code section, and b) reordering DATA section to group together data modified by the dyld dynamic linker. Neither of those optimizations applies here.

That leaves us with several options:

Do nothing. The downside is that a future linker update may start doing some optimization that breaks the code.
Split the patchable code into separate patchedcode.S file and disable .subsections_via_symbols for it individually. The new linker would lose the unwind info for this specific code but we may not need it anyway. I'd need someone more knowledgable to confirm or deny that assumption. The upside of that approach is that we would still communicate the correct semantics to the linker so it's less likely to be accidentally broken. The downside is that we would still hit the Xcode 15 linker bug but only for a very limited part of code where it may not have consequences.
Use the -order_file linker switch and explicitly enforce the order of the symbols inside the patchable section. This is potentially an unchartered territory where we may hit another linker bug minefield. It also introduces a maintenance burden where the order file needs to be kept up-to-date with the assembly code.
Detect Xcode version and force the use of classic linker with -ld_classic switch if Xcode 15 is used. This complicates the build process and it would only buy us some limited amount of time. Apple publicly stated that they intend to remove the old linker in future Xcode update.

Interestingly, -dead_strip does strip about 120Kb of code on debug libcoreclr.dylib build, so it might be worth checking out separately as a size optimization.

filipnavara · 2023-09-24T08:19:49Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

jkotas · 2023-09-26T02:34:57Z

One more thing, JIT_UpdateWriteBarrierState is inside the patched section for .asm (Windows) and outside for .S (non-Windows). Is that intentional?

I do not see a good reason for it. I think it should be outside the patched section in both cases.

janvorli

LGTM, thank you!

Make CoreCLR/NativeAOT assembly compile with .subsections_via_symbols…
aba0b02
… on Apple platforms

ghost added area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member labels Sep 23, 2023

filipnavara commented Sep 23, 2023
View reviewed changes

Fix build with LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT
254a1bc

filipnavara marked this pull request as ready for review September 23, 2023 11:24

filipnavara requested a review from MichalStrehovsky as a code owner September 23, 2023 11:24

jkotas approved these changes Sep 23, 2023
View reviewed changes

jkotas merged commit 5dfd805 into dotnet:mainSep 23, 2023

github-actionsBot mentioned this pull request Sep 23, 2023
[release/8.0] Make CoreCLR/NativeAOT assembly compile with .subsections_via_symbols on Apple platforms #92544
Merged

vcsjones mentioned this pull request Sep 23, 2023
Exception unwinding broken with Xcode 15 linker #92297
Closed

This was referenced Sep 24, 2023
Should JIT_UpdateWriteBarrierState be in the patchable section or not? #92554
Closed
Split off patched code into separate .S file and disable subsections-via-symbols for it #92555
Merged

janvorli reviewed Sep 26, 2023
View reviewed changes

ghost locked as resolved and limited conversation to collaborators Oct 26, 2023

filipnavara deleted the subsections_via_symbols branch June 5, 2025 07:44