Skip to content

fix(compat): make the POSIX adapter build and run on macOS - #96

Open
AshrafAhmed9 wants to merge 2 commits into
embeddedos-org:masterfrom
AshrafAhmed9:fix-macos-posix-adapter
Open

fix(compat): make the POSIX adapter build and run on macOS#96
AshrafAhmed9 wants to merge 2 commits into
embeddedos-org:masterfrom
AshrafAhmed9:fix-macos-posix-adapter

Conversation

@AshrafAhmed9

@AshrafAhmed9 AshrafAhmed9 commented Aug 30, 2026

Copy link
Copy Markdown

os_adapter_posix.c opens by saying it "runs EoS services on Linux, macOS or
WSL". It does not run on macOS. Darwin does not implement three of the
primitives it reaches for:

primitive on Darwin effect
pthread_mutex_timedlock() absent file does not compile
sem_timedwait() absent file does not compile
sem_init() fails, ENOSYS no unnamed semaphores, so every create fails
sem_getvalue() fails, ENOSYS the count posix_sem_post() checks the ceiling against cannot be read

The first two are compile errors, so Build (macos-latest) cannot pass. The
other two would still fail at runtime once it built.

$ cc -std=c11 -Wall -Wextra -c services/compat/src/os_adapter_posix.c ...
os_adapter_posix.c:330:12: error: call to undeclared function 'pthread_mutex_timedlock'
os_adapter_posix.c:400:18: error: call to undeclared function 'sem_timedwait'

The file had no #ifdef __APPLE__ anywhere, which CONTRIBUTING.md asks for.

The approach

Named semaphores don't help: sem_open() works on Darwin but sem_getvalue()
still doesn't, and posix_sem_post() needs the count to enforce the ceiling.

So on Apple the semaphore is a mutex and a condition variable holding its own
count. That gives an exact sem_getvalue(), and makes the timed wait a real
pthread_cond_timedwait() rather than a poll. The timed mutex lock has no such
equivalent, so it polls pthread_mutex_trylock() on a 1 ms interval until the
deadline — that interval bounds how far a timed lock can overshoot.

Everything else keeps the system primitives. Both sides sit behind the same
compat_* wrappers, so the call sites are unchanged and the non-Apple wrappers
compile to a direct call. The EINTR retry loops moved into the wrappers, where
they were already duplicated at each call site.

Testing

Verified on macOS 15, Apple Clang. Before, the translation unit fails with the
two errors above. After, it compiles clean, no warnings:

$ cc -std=c11 -Wall -Wextra -c services/compat/src/os_adapter_posix.c \
    -I services/compat/include -I include -I kernel/include -I hal/include -I core/include
$

The new primitives were exercised directly rather than assumed:

init(2) -> count=2 ok
trywait -> 0 ok
count=1 ok
trywait on empty -> rc=-1 ok (blocks)
timedwait(100ms) on empty -> rc=-1 after 105ms ok (times out)
timedwait woken by post -> rc=0 after 152ms ok (wakes)
mutex_timedlock(80ms) held -> rc=-1 after 80ms ok (times out)
mutex_timedlock free -> rc=0 ok

That covers the count, the empty case, a timeout that fires, a wait woken by
another thread posting at 150 ms, a timed lock that expires against a held
mutex, and one that succeeds once released.

I couldn't run ctest end to end — master doesn't compile for reasons unrelated
to this file (duplicate test targets in tests/CMakeLists.txt, duplicate
eos_task_set_current_internal in task.c, the unresolved priority-inheritance
merge in sync.c), which #93 and #94 address. Once #94 is in, this file is
reached by the normal build and the suite should run on macOS for the first time.

The file documents itself as running on Linux, macOS or WSL, but Darwin
does not implement three of the primitives it uses:

  - pthread_mutex_timedlock() is absent, so the file does not compile.
  - sem_timedwait() is absent, same.
  - sem_init() fails with ENOSYS. Darwin has no unnamed semaphores, so
    every semaphore creation would fail at runtime even once it built.
  - sem_getvalue() fails with ENOSYS, so the count that posix_sem_post()
    enforces the ceiling against cannot be read back.

Named semaphores are not a way out: sem_open() works but sem_getvalue()
still does not, and the ceiling check needs the count.

On Apple platforms the semaphore is therefore a mutex and a condition
variable holding its own count, which also makes the timed wait exact
rather than polled, and the timed mutex lock polls trylock until the
deadline. Every other POSIX host keeps the system primitives, reached
through the same small compat_* wrappers so the call sites read the same
on both.

CONTRIBUTING.md asks for #ifdef __APPLE__ around Apple-specific code;
this file previously had none.

Verified on macOS 15 with Apple Clang. Before the change the translation
unit fails with two "call to undeclared function" errors; after it
compiles clean under -std=c11 -Wall -Wextra with no warnings. The new
primitives were exercised directly: a 100 ms timed wait on an empty
semaphore returns -1 after 105 ms, the same wait returns 0 after 152 ms
when another thread posts at 150 ms, an 80 ms timed lock on a held mutex
returns -1 after 80 ms, and the same lock succeeds once released.
srpatcha
srpatcha previously approved these changes Aug 30, 2026

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. The Darwin reasoning is right and the Linux path is genuinely
unchanged — I checked that rather than assuming it, because the diff touches
shared call sites, not only #ifdef __APPLE__ blocks.

What I verified

Thirty changed lines sit outside the Apple guards — pthread_mutex_timedlock
compat_mutex_timedlock, sem_tcompat_sem_t, and the five sem_* calls.
So every POSIX host now goes through the shims, and the question is whether the
#else branch reproduces the originals exactly. It does:

typedef sem_t compat_sem_t;
static int compat_sem_init(compat_sem_t *s, unsigned value) { return sem_init(s, 0, value); }
static int compat_sem_destroy(compat_sem_t *s)              { return sem_destroy(s); }
...

sem_init(s, 0, value) keeps the not-shared 0 from the original call. The
typedef is the same type, so no struct layout moves.

The one thing that looked like a behaviour change is not. compat_sem_wait
carries an EINTR retry loop, but master:393 already had it at the call site:

while ((rc = sem_wait(s)) == -1 && errno == EINTR) { }

It moved into the shim; it was not added.

Built and ran on Linux, applied onto a base where master's duplicate test
targets are repaired:

0 build errors
100% tests passed, 0 tests failed out of 28

What I did not verify

The Darwin behaviour itself. I have no macOS host, so sem_init returning
ENOSYS, sem_getvalue being unavailable even for named semaphores, and the
absence of pthread_mutex_timedlock are all things I am taking from your
description and from the documented Darwin behaviour, not from a run. Saying so
explicitly rather than implying the whole PR is tested.

Two things I would want a macOS run to confirm before relying on it:

  1. COMPAT_LOCK_POLL_NS at 1 ms bounds the overshoot of a timed lock, as your
    comment says. That is the correct trade-off to name, and the value only
    matters if something in the tree takes short timed locks — worth a note in
    the docs if any caller does.
  2. The condition-variable semaphore enforces the ceiling against a count it
    owns. Since that count is now the authority rather than sem_getvalue, a
    post that would exceed max has to be rejected on that path too, not only
    on the POSIX one.

Replacing a polled timed wait with an exact one on Darwin is a real improvement
over what a sem_trywait loop would have given, and the comment block explaining
why named semaphores do not rescue sem_getvalue is exactly the kind of thing
that stops someone "simplifying" this back into a bug later.

Needs #82 or #93 to land first — on master today, CMake cannot generate any
test target, so CI here cannot run its own tests.

@AshrafAhmed9

Copy link
Copy Markdown
Author

Thanks for checking the shared call sites rather than trusting the #ifdef. That was the right thing to be suspicious of, since thirty of the changed lines do sit outside the guards.

On the ceiling. It is rejected on the Apple path. posix_sem_post is shared rather than per-platform, and the check runs through compat_sem_getvalue, which on Darwin reads the count the condvar semaphore owns:

int value = 0;
if (compat_sem_getvalue(&g_sems[handle].sem, &value) == 0 &&
    (uint32_t)value >= g_sems[handle].max) {
    return -1;
}
return compat_sem_post(&g_sems[handle].sem) == 0 ? 0 : -1;

So the count that became the authority is the one the ceiling is tested against.

You are onto something real one level down, though. That check is not atomic. compat_sem_getvalue takes the lock, reads, and releases it; compat_sem_post then takes it again to increment. Two threads posting at count == max - 1 can both see value < max, both succeed, and leave the count at max + 1.

Master has the same shape on POSIX today (sem_getvalue then sem_post, with nothing held across the two), so this is pre-existing rather than something the Apple path introduces. It is now equally broken on both backends instead of just one, which is not much of a defence.

The fix is a per-semaphore mutex held across the check and the increment, on both backends, so the invariant survives contention instead of holding only when posts happen to serialise. That is a real behaviour change to the POSIX path, so I would rather not smuggle it into a PR whose stated job is "make it compile on macOS". Happy either way: say the word and I will add it here, otherwise I will open it separately once this lands.

On the poll interval. Nothing in the tree reaches it. posix_mutex_lock_fn sends timeout_ms == 0 to pthread_mutex_trylock and EOS_OSA_WAIT_FOREVER to plain pthread_mutex_lock, so compat_mutex_timedlock only runs for a finite non-zero timeout, and every eos_mutex_lock call in the repo passes 0 or EOS_NO_WAIT. The 1 ms granularity is unreachable from current callers. It starts to matter the first time somebody adds a finite timed lock, which is exactly the argument for documenting it now rather than after. I will put it in the header comment.

Agreed on #93. CMake cannot generate a test target on master, so CI here has nothing to run until that lands.

COMPAT_LOCK_POLL_NS bounds how far a timed mutex lock can overshoot on
Apple, where pthread_mutex_timedlock() does not exist. No caller reaches
it today: posix_mutex_lock_fn() routes a zero timeout to trylock and
EOS_OSA_WAIT_FOREVER to a plain lock, and every eos_mutex_lock() in the
tree passes 0 or EOS_NO_WAIT. Record the granularity now so the first
caller to want a finite timed lock finds it, rather than after.

Comment only, no behaviour change.

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-approving — my earlier review was auto-dismissed by the follow-up commit, not
withdrawn.

docs(compat): note the timed-lock poll granularity on Darwin addresses the
first of the two things I flagged. Documenting that COMPAT_LOCK_POLL_NS bounds
how far a timed lock can overshoot is the right response: the value is a
trade-off, not a constant someone should tune without knowing what it buys.

Everything in my previous review stands:

  • The Linux path is unchanged. Every compat_* in the #else branch delegates
    to the original call, sem_init(s, 0, value) keeps the not-shared 0, and
    typedef sem_t compat_sem_t moves no struct layout. The EINTR retry loop in
    compat_sem_wait was already at the call site on master:393; it moved rather
    than appearing.
  • Built and ran on Linux against a repaired base: 0 build errors, 28/28
    passed
    .
  • I still have not verified the Darwin behaviour itself — no macOS host here — so
    sem_init returning ENOSYS, sem_getvalue being unavailable even for named
    semaphores, and the absence of pthread_mutex_timedlock are taken from your
    description and the documented Darwin behaviour, not from a run.

The second point from last time is still open, and is the one I would want a
macOS run to settle: the condition-variable semaphore now owns its own count, so
the max ceiling has to be enforced on that path too, not only on the POSIX one.
Worth a test if you have a Darwin machine.

Needs #82 or #93 to land first — on master today CMake cannot generate a single
test target, so CI here cannot run its own tests.

@AshrafAhmed9

Copy link
Copy Markdown
Author

Ran the ceiling on Darwin. It holds.

The coverage you wanted already exists: test_semaphore_ceiling in tests/test_os_adapter.c drives it through the adapter vtable, so on Apple it exercises the condvar semaphore rather than a POSIX one. It just had never been run on a Mac.

macOS 15, Apple Clang, arm64:

=== EoS OS Adapter Tests ===
-- posix semaphore --
  [PASS] semaphore honours its ceiling
...
=== 83/83 checks passed ===

So post beyond max is refused, the count is unchanged after the refusal, and sem_create with initial > max is still rejected, all against the count the condvar semaphore owns.

One thing that test does not prove, and I would rather say it than let the green run imply otherwise: the ceiling check is single-threaded there. posix_sem_post still reads the count and increments it under two separate lock acquisitions, so the race I mentioned earlier is untouched by this result. It is the same on both backends and predates this PR.

Getting it to run took an unrelated fix

eos does not link on macOS at all right now, independently of this PR. #90 wired eos_hal_init() to pick a backend:

#ifdef __linux__
    eos_hal_linux_register();
#else
    eos_hal_rtos_register();
#endif

but CMake picks the source file on a different question:

if(EOS_PLATFORM STREQUAL "linux" OR NOT CMAKE_CROSSCOMPILING)
    target_sources(eos_hal PRIVATE hal/src/hal_linux.c)
else()
    target_sources(eos_hal PRIVATE hal/src/hal_rtos.c)
endif()

CMake asks "am I cross-compiling", the C asks "am I on Linux". On a Mac those disagree. CMake compiles hal_linux.c, whose body is entirely inside #ifdef __linux__ and so emits no symbols, while the preprocessor takes the #else and calls eos_hal_rtos_register(), which lives in hal_rtos.c. That file is not compiled, and cannot be on a host: it contains __asm volatile ("cpsid i").

Undefined symbols for architecture arm64:
  "_eos_hal_rtos_register", referenced from:
      _eos_hal_init in libeos_hal.a[2](hal_common.c.o)

The comment above the declarations in hal.h states the invariant the build does not implement: "Exactly one of these is compiled: hal_linux.c is guarded on __linux__ and hal_rtos.c on its absence."

I widened all three guards to defined(__linux__) || defined(__APPLE__) locally to get the suite running. The hal_linux.c body then compiles clean on Darwin under -Wall -Wextra, so the host backend does appear to be portable rather than Linux-specific. None of that is in this PR. Happy to open it separately if you want it fixed that way, or leave it to you if the intended answer is a third host backend.

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