Skip to content

Conversation

@graeme-winter
Copy link
Contributor

Fix build on macOS, resolves ld: Found illegal text-relocations

@graeme-winter graeme-winter changed the title Use darwin syntax, with #define Use darwin / clang syntax, with #define for platform Sep 15, 2025
Copy link
Collaborator

@Araneidae Araneidae left a comment

Choose a reason for hiding this comment

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

Thank you, this is a major step forward. I hope that your patch can be simplified to a single unconditional adr instruction; if you can test this I think we'll merge this and try and update our CI runners if necessary.

Comment on lines +108 to +113
#if defined(__aarch64__) && defined(__APPLE__) // darwin syntax
" adrp lr, action_entry@PAGE\n"
" add lr, lr, action_entry@PAGEOFF\n"
#else
" ldr lr, =action_entry\n"
#endif
Copy link
Collaborator

@Araneidae Araneidae Sep 15, 2025

Choose a reason for hiding this comment

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

I don't think there's anything specific to Apple about this two part syntax. If possible I think I'd like to see this unconditionally using the new syntax.

It does occur to me that we shouldn't need to be using a linker relocation here at all: action_entry is nearby at a constant offset, so I'd expect something like

 adr lr, action_entry

to work on both targets (it looks like ADR is the appropriate instruction here).

@AlexanderWells-diamond, do we have CI runners for Linux and OSX ARM64 for cothread?

Copy link
Contributor

Choose a reason for hiding this comment

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

We currently do not have Linux ARM64, nor any MacOS CI as part of our default set.

In theory, as per this blog post, we can simply add ubuntu-24.04-arm and macos-latest to the test matrix to get full coverage of Arm64 platforms. However, a quick test shows failures on both platforms.

I'll do a little digging into why these are failing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed, the corresponding instruction in switch-arm.c also ought to be ADR, but I guess that's out of scope for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's currently no drop-in way to make either Linux or MacOS Arm64 work.

Both have the issue that they will need changes to the epics-base container to support ARM.

MacOS additionally needs a complete overhaul of how the CI works; we currently use the epics-base container to provide us an EPICS environment for various tests. We cannot use this on MacOS, the Github runners do not let us run containers inside them. So we need to either stop doing all the EPICS-specific tests there, or set up the environment ourselves in the CI scripts.

Araneidae added a commit that referenced this pull request Sep 17, 2025
This is all arising from issue #68 and PR #76.  It seems that MacOS Arm64
does not like the use of relocation in the original switch-arm64.c code.
This arises from a single instruction
    ldr lr, =action_entry

As action_entry is extremely close to this code this can simply be replaced
with
    adr lr, action_entry
for both ARM32 and ARM64.
@Araneidae
Copy link
Collaborator

This is now obsoleted by PR #77 which uses the adr instruction and covers both ARM architectures.

@Araneidae Araneidae closed this Sep 17, 2025
Araneidae added a commit that referenced this pull request Oct 6, 2025
This is all arising from issue #68 and PR #76.  It seems that MacOS Arm64
does not like the use of relocation in the original switch-arm64.c code.
This arises from a single instruction
    ldr lr, =action_entry

As action_entry is extremely close to this code this can simply be replaced
with
    adr lr, action_entry
for both ARM32 and ARM64.
@Araneidae
Copy link
Collaborator

Oh well. It looks like PR #77 can't be made to work on OSX.

@Araneidae Araneidae reopened this Oct 8, 2025
@Araneidae Araneidae merged commit aa43929 into DiamondLightSource:master Oct 8, 2025
8 checks passed
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.

3 participants