Skip to content

CMake plumbing to build the loader (2) - #14

Open
illwieckz wants to merge 16 commits into
illwieckz/mingw-2from
illwieckz/cmake
Open

illwieckz wants to merge 16 commits into
illwieckz/mingw-2from
illwieckz/cmake

Conversation

@illwieckz

Copy link
Copy Markdown
Member

Replay of:

GitHub doesn't allow me to re-open it…

@illwieckz illwieckz added the enhancement New feature or request label Jun 21, 2026
@illwieckz illwieckz changed the title Write a CMakeLists.txt to build the NaCl loader Write a CMakeLists.txt to build the NaCl loader (2) Jun 21, 2026
@illwieckz illwieckz changed the title Write a CMakeLists.txt to build the NaCl loader (2) CMake plumbing for the loader (2) Jun 21, 2026
@illwieckz illwieckz changed the title CMake plumbing for the loader (2) CMake plumbing to build loader (2) Jun 22, 2026
@illwieckz illwieckz mentioned this pull request Jun 22, 2026
@illwieckz illwieckz changed the title CMake plumbing to build loader (2) CMake plumbing to build the loader (2) Jun 22, 2026

@slipher slipher 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.

Some things we aren't going to use that could be removed: MIPS, Android, and "zero-based sandbox".

set(LD_EMUL "armelf_linux_eabi")
set(RESERVE_TOP "0x40002000")

if (YOKAI_CXX_COMPILER_CLANG_COMPATIBILITY)

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.

You have two different spellings of CLANG_COMPATIBILITY here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure to get what you mean.

Comment thread cmake/NaClFlags.cmake Outdated
endif()

# From SetUpLinuxEnvMips() from (root)/SConstruct.
if (YOKAI_TARGET_SYSTEM_LINUX_COMPATIBILITY AND YOKAI_TARGET_ARCH_MIPSEL)

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.

Why is there mips stuff? We shouldn't waste our time on stuff we're not going to use.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reason why there was mips stuff is that when I ported it, I translated the scons files line by line by hand and I didn't want to miss anything, just in case.

I added a commit to remove that mips stuff from CMake.

)

if (YOKAI_CXX_COMPILER_MSVC)
list(APPEND PLATFORM_INPUTS "win/nacl_time.c")

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.

The Windows one worked for me with MinGW. It's probably better to use this as the *nix compatibility layers might be lossy.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Was it MinGW on Windows?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, maybe I just need -lwinmm. It builds but not link.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That was that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

Comment thread src/shared/platform/CMakeLists.txt Outdated

if (YOKAI_CXX_COMPILER_MSVC)
list(APPEND PLATFORM_FLAGS "/D_CRT_RAND_S")
list(APPEND PLATFORM_FLAGS "/D_UNICODE")

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.

These defines act for MinGW too. That's probably why you had to add explicit W suffixes to some function names.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Didn't know MinGW could receive MinGW-style /X options. 🙂️

@illwieckz illwieckz Oct 5, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, maybe you meant to use -D 🤣️

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, the flag passing was also broken.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

Comment thread src/shared/platform/CMakeLists.txt Outdated
"linux/nacl_semaphore.c"
)

#TODO: kernel_version = list(map(int, platform.release().split('.', 2)[:2]))

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.

Let's not do it :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure! 😅️

It's just that I translated from scons line by line and commented out things like that, with the TODO: flag as a generic flag to find them if I needed to review them later.…

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Cleaned-up.

Comment thread src/trusted/desc/CMakeLists.txt Outdated
add_library(nrd OBJECT "nrd_xfer.c")
list(APPEND NRD_XFER_LIBS nrd)

if (NOT YOKAI_TARGET_SYSTEM_WINDOWS)

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.

this should be NOT MSVC for the aliasing flag

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

list(APPEND NRD_FLAGS "-fno-strict-aliasing") # This was only a C flag in build.scons
list(APPEND NRD_FLAGS "-Wno-missing-field-initializers")
string(REPLACE ";" " " NRD_FLAGS_STRING "${NRD_FLAGS}")
set_target_properties(nrd PROPERTIES COMPILE_FLAGS "${NRD_FLAGS_STRING}")

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.

COMPILE_FLAGS is supposedly deprecated. Maybe if you use TARGET_COMPILE_OPTIONS this string replacement nonsense won't be needed?

# sel_ldr binary
# TODO(robertm): see who really needs them and remove
if (YOKAI_TARGET_SYSTEM_WINDOWS)
# FIXME: Unused for now.

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.

Probably it builds without that due to CMAKE_C(XX)_STANDARD_LIBRARIES.

Comment thread CMakeLists.txt
endif()

if (BUILD_NACL_LOADER AND YOKAI_TARGET_SYSTEM_WINDOWS)
option(FORCE_NO_TRUSTED_BUILD "Prevent use of trusted toolchain." OFF)

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.

This doesn't seem to have any relevance when the build uses only one toolchain to begin with.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed.


# Using this optimization when building with GCC breaks the program on armhf:
# > Illegal instruction
if (NOT YOKAI_CXX_COMPILER_CLANG_COMPATIBILITY AND (YOKAI_TARGET_ARCH_ARMHF OR YOKAI_TARGET_ARCH_ARMEL))

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.

I think ARMEL is not really a valid target platform since the sandbox expects to be able to use a specific set up CPU instructions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

armel is just armhf with hard-float being optional. In the end the armel topic is probably just the Android topic.

@illwieckz
illwieckz changed the base branch from master to illwieckz/mingw-2 October 5, 2026 23:43
@illwieckz

Copy link
Copy Markdown
Member Author

Some things we aren't going to use that could be removed: MIPS, Android, and "zero-based sandbox".

All of that was there because I didn't want to overlook anything when tranlasting from scons to cmake.

I NUKED added commits to NUKE the MIPS support and the zero-based sandbox thing in CMake.

The Android stuff, I'm less convinced.

The only known working implementation is for i686 MSVC.

- detection was already disabled and marked at not working with amd64 MSVC,
- detection was already disabled on amd64 MinGW because amd64 Windows was tested, not just amd64 MSVC,
- there is no MinGW implementation in the code,
- this disables detection on MinGW explicitely whatever the architecture.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants