Skip to content

ARM64 CFI support#11099

Merged
xavierleroy merged 3 commits intoocaml:trunkfrom
ctk21:arm64_cfi_support
Mar 16, 2022
Merged

ARM64 CFI support#11099
xavierleroy merged 3 commits intoocaml:trunkfrom
ctk21:arm64_cfi_support

Conversation

@ctk21
Copy link
Copy Markdown
Contributor

@ctk21 ctk21 commented Mar 9, 2022

This PR fixes up the CFI commands to allow unwind on ARM64 that was left over from #10972. tests/unwind now passes on both MacOS x86_64 and ARM64.

To get things working involved some trial-and-error to force the toolchain to give me DWARF information on MacOS ARM64. In my experience I had to remove the -no_compact_unwind link flag on ARM64 for tests/unwind to work. However on x86_64, I found that the -no_compact_unwind link flag was necessary to keep tests/unwind working. After attempts to try and find sanity by exploring past issues (e.g. #5921, #7120) and some time with a search engine trying to figure out what other languages do, I was not able to shed strong clarity on this.

However I believe the CFI is correct in the PR, even if we ultimately decide to strip some of the CFI from MacOS ARM64 builds by retaining -no_compact_unwind. My feeling is we should drop -no_compact_unwind on MacOS ARM64 and keep tests/unwind running. 4.14 has disabled tests/unwind for ARM64 because there did not appear to be a way to emit unwind information; see 482b7fe.

@xavierleroy
Copy link
Copy Markdown
Contributor

Thanks a lot for this work.

I forgot that I turned off the unwind test on macos-ARM64 in 4.12. As far as I can remember, at that time I failed to make libunwind work even with a plain C program, regardless of the options passed at link time, so I concluded something was wrong with libunwind. I haven't tried (with a plain C program) since.

Re: no-compact-unwind, this was necessary to avoid link-time (or assembly-time?) warnings of the "unwind information is too big to fit the compact format" kind. Maybe it's an x86-specific issue, or maybe the unwind information format changed enough that it's no longer relevant.

@xavierleroy
Copy link
Copy Markdown
Contributor

I confirm unwinding works for a pure C program on macos 10.15 arm64, but it does not work if no_compact_unwind is passed.

Copy link
Copy Markdown
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

Comment on lines +1172 to +1173
(* avoid collision for unwind test with code_begin symbol *)
if macosx then ` nop\n .align 3\n`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There may be other reasons to add a nop here, see #4690. But why .align 3?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the reference to #4690.
The align is needed because the linker would otherwise giving warnings about shared_startup__code_{begin,end} for tests/lib-dynlink-pr4839. These warnings cause the test to fail.
I've updated the comment for these two points in 996e206

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, that's good to know, thanks!

@dra27 dra27 mentioned this pull request Mar 16, 2022
16 tasks
@xavierleroy xavierleroy merged commit 2101ca0 into ocaml:trunk Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants