Closed Bug 1722134 Opened 3 years ago Closed 3 years ago

Modify build system for rlbox windows/android support

Categories

(Firefox Build System :: General, enhancement, P3)

Desktop
Windows
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shravanrn, Assigned: shravanrn)

References

(Blocks 1 open bug)

Details

We are currently expanding the use of rlbox wasm library sandboxing to the windows platform. As part of this effort, the build system must be modified to allow the wasm lib sandboxing on windows. For this the following changes are needed.

  1. mach bootstrap should bring in a copy of wasi-sdk and should bring a version of clang that has WebAssembly support. This is currently supported on Linux (See Bug 1615595); Windows would need similar support. For some reason mach bootstrap does not seem to bring in the wasi-sdk or a clang with WebAssembly support. This was supported at some point, but seems to have regressed (However CI builds seem to be fine). This should either be fixed as part of this bug or separately.

  2. Eliminate some safety checks in toolkit/moz.configure and python/mozbuild/mozbuild/frontend/data.py that disallow the wasm sandbox configuration on Windows platform (See here and here). As part of this bug, we can also eliminate the restriction to use RLBox on Mac Arch64 in toolkit/mozapps/installer/unify.py# (See here)

  3. Modify wasm library name in python/mozbuild/mozbuild/frontend/emitter.py. For some reason libname is None on Windows for these two lines which causes a crash as a later part of the build config. I'm not sure why this behavior is different on Windows compared to Linux, since they are both processing the same moz.build files. The workaround I used for this wasm to replace libname with wasm_lib for these two files.

  4. This one may not be a bug --- Wasm_CC and WASM_CXX seem to be invoked with some VS specific compiler switches. I think this maybe a function of the builds using clang-cl on windows for the regular build, and my choice to use the clang interface that came with the wasi-sdk. I think as long as there is consistency on what is used, it should be fine. If regular files are compiled with clang, but the wasm files are compiled with clang-cl, at a minimum the following flags -Z7, -Oy-, -Fo vs -o for output has to be changed. These flags are passed through the config and eventually fed through here and a few other targets in this file.

  5. This one is not exactly a bug, but possibly a minor improvement. I notices that wasm files are compiled with -Os instead of -O3 or similar. Since most of firefox is not compiled with -Os we probably want to make this consistent.

  6. wasi-sdk config Windows threads error--- Lastly there is a strange error that occurs when using the wasi-sdk on Windows. There is a compilation error complaining about the fact that __STDCPP_THREADS__ and _LIBCPP_HAS_NO_THREADS are defined. I am not exactly sure where this error comes from --- whether its compiler arguments or something else. I'll follow up more about this and post information as I have it. As a workaround, I comment out the following lines in .mozbuild/wasi-sysroot/share/wasi-sysroot/include/c++/v1/__config

// #if defined(__STDCPP_THREADS__) && defined(_LIBCPP_HAS_NO_THREADS)
// #error _LIBCPP_HAS_NO_THREADS cannot be set when __STDCPP_THREADS__ is set.
// #endif

@tom, glandium (fyi @bholley): I think this task may need some cycles from someone who understands the build system better than me. Would you be able to help find an owner for this / would you have cycles to help with this? :)

Flags: needinfo?(tom)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(bholley)

Glandium is the obvious person to help here.

Flags: needinfo?(bholley)
Depends on: 1722439
Summary: Modify build system for rlbox windows support → Modify build system for rlbox windows/android support
  1. For android the rlbox.so has to be included in mobile/android/installer/package-manifest.in also
Depends on: 1723445
Depends on: 1723447
Depends on: 1723623
  1. For android we need to add wasi-sysroot package to taskcluster/ci/build/android.yml to pull in dependencies for android wasm build (Similar changes may be needed for windows also)
Flags: needinfo?(tom)
Flags: needinfo?(mh+mozilla)

This was fixed via all the dependencies and possibly more.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.