Skip to content

fix: gate x86 idle hlt for hosted builds - #76

Merged
srpatcha merged 1 commit into
embeddedos-org:masterfrom
Rani2909:fix/x86-idle-hlt-hosted
Aug 29, 2026
Merged

srpatcha merged 1 commit into
embeddedos-org:masterfrom
Rani2909:fix/x86-idle-hlt-hosted

Conversation

@Rani2909

Copy link
Copy Markdown
Contributor

Reviewer feedback addressed

I understood the concern about removing hlt. Instead of removing it, this PR gates the instruction so hosted Windows builds cannot use the privileged hlt instruction, while genuine freestanding x86_64 targets retain hlt.

This change is intentionally split from PR #60 as requested, so the package and heap test improvements can be reviewed and landed independently.

Summary

This PR addresses review item #3 from PR #60.

The x86_64 idle path previously used the privileged hlt instruction without distinguishing between hosted Windows builds and genuine freestanding targets. This can cause a fault when EoS is built as a hosted Windows application.

Changes

  • Keep hlt available for genuine freestanding x86_64 targets.
  • Exclude Windows host builds from the hlt path.
  • Require a freestanding build before emitting hlt.
  • Preserve the existing ARM and RISC-V wfi paths.
  • Keep hosted/simulation builds on the safe idle path.

Validation

  • Windows simulation build completed successfully.
  • CTest: 21/21 tests passed.
  • git diff --check: passed.
  • Only kernel/src/task.c is included in this PR.

Scope

This PR contains only the x86_64 hlt gating fix requested in review item #3.

The test_package.c and test_heap.c improvements remain in PR #60.

@srpatcha

Copy link
Copy Markdown
Member

Right problem, and the guard is one character away from working

Agreed on the diagnosis — hlt is ring-0, so the idle task SIGSEGVs under a hosted libc, and gating it is the correct fix.

But __STDC_HOSTED__ is always defined by GCC and Clang. It is 1 hosted and 0 freestanding; it is never undefined. So !defined(__STDC_HOSTED__) is false in both modes:

$ gcc -fsyntax-only t.c
warning: "defined(__STDC_HOSTED__) is TRUE here"
warning: "__STDC_HOSTED__ != 0 (hosted)"

$ gcc -ffreestanding -fsyntax-only t.c
warning: "defined(__STDC_HOSTED__) is TRUE here"     ← still defined
warning: "__STDC_HOSTED__ == 0 (freestanding)"

The host crash goes away, but only because the hlt branch is now unreachable on every build — including a real bare-metal x86 target, where the idle task would spin instead of halting. That is the opposite of what an idle task is for, and it is the kind of thing nobody notices until a battery number comes in wrong.

Testing the value rather than its definedness is the whole fix:

 #elif (defined(__x86_64__) || defined(_M_X64)) && !defined(_WIN32) && \
-      !defined(__STDC_HOSTED__)
+      !__STDC_HOSTED__

Which gives the intended split — preprocessing kernel/src/task.c:

hlt occurrences
gcc -E (hosted) 0
gcc -ffreestanding -E 1

I have this change built and verified locally (ctest 24/24) but could not push it to your fork from here, so it is yours to apply. Worth a short comment on the branch too — the reason a freestanding build is the bare-metal one is not obvious to the next reader.

Happy to merge as soon as that lands.

@srpatcha
srpatcha merged commit 19bf583 into embeddedos-org:master Aug 29, 2026
6 of 23 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.

2 participants