Conversation
Add native_sim board configuration and support in the build script. This allows building and running tests on the host using Zephyr's native_sim target. native_sim leverages the POSIX architecture, but the libfuzzer support specifically requires CONFIG_ARCH_POSIX_LIBFUZZER to be set. Therefore, this wraps fuzzer-specific code in ipc.c and the build of fuzz.c behind this config to allow clean compilation on the standard native_sim board. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add native_sim board target to the sof-qemu-run scripts, and add an option to additionally run it under valgrind. The default build directory is set to ../build-native_sim Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
lgirdwood
requested review from
abonislawski,
dbaluta,
iuliana-prodan,
kv2019i,
lbetlej,
lyakh,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
Adds Zephyr native_sim support to enable running SOF Zephyr tests natively (including under Valgrind) and refines POSIX/libFuzzer integration so fuzz-specific code is only built/used when enabled.
Changes:
- Add a
native_simplatform target to the Zephyr build helper. - Make POSIX fuzzing sources/IPC hooks conditional on
CONFIG_ARCH_POSIX_LIBFUZZER. - Extend the run scripts to support
native_simexecution and optional--valgrind.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
zephyr/CMakeLists.txt |
Separates POSIX sources from fuzz-only sources so fuzz.c only builds when libFuzzer is enabled. |
src/platform/posix/ipc.c |
Gates fuzz ISR/IRQ plumbing behind CONFIG_ARCH_POSIX_LIBFUZZER. |
scripts/xtensa-build-zephyr.py |
Adds native_sim as a supported Zephyr platform config. |
scripts/sof-qemu-run.sh |
Adds --valgrind flag parsing and changes default build dir behavior for native_sim runs. |
scripts/sof-qemu-run.py |
Detects native_sim from CMakeCache.txt, supports running under Valgrind, and skips QEMU monitor steps for native_sim. |
app/boards/native_sim.conf |
Introduces a board-specific Kconfig fragment for native_sim. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| BUILD_DIR="../build-native_sim" | ||
| VALGRIND_ARG="" |
There was a problem hiding this comment.
This changes the default build directory from the previous ${1:-build} behavior to a hard-coded ../build-native_sim. That makes ./scripts/sof-qemu-run.sh (with no args) run a different target than before and can be surprising for existing QEMU workflows. Consider keeping the previous default (build) and/or selecting a native_sim default only when explicitly requested (e.g., via a flag), so the script remains backward compatible.
| #ifdef CONFIG_ARCH_POSIX_LIBFUZZER | ||
| // Not an ISR, called from the native_posix fuzz interrupt. Left | ||
| // alone for general hygiene. This is how a IPC interrupt would look | ||
| // if we had one. |
There was a problem hiding this comment.
Inside the #ifdef CONFIG_ARCH_POSIX_LIBFUZZER block, the extern declaration for posix_fuzz_buf/posix_fuzz_sz (currently extern uint8_t *posix_fuzz_buf, posix_fuzz_sz;) does not match the definitions in src/platform/posix/fuzz.c (const uint8_t *posix_fuzz_buf; size_t posix_fuzz_sz;). This mismatch can cause incorrect reads/writes (e.g., posix_fuzz_sz = 0; only updating 1 byte). Please split these into separate externs with the correct types (and const).
| @@ -185,8 +237,8 @@ def main(): | |||
| child.close(force=True) | |||
| except pexpect.EOF: | |||
| print("\n[sof-qemu-run] QEMU terminated before we could run monitor commands.") | |||
| else: | |||
| print("\n[sof-qemu-run] Process is no longer alive, cannot extract registers.") | |||
| else: | |||
| print("\n[sof-qemu-run] Process is no longer alive, cannot extract registers.") | |||
|
|
|||
There was a problem hiding this comment.
The else: at the end of the QEMU monitor interaction is currently attached to the try/except block (Python try ... except ... else), not to the if child.isalive(): check. As written, it will print "Process is no longer alive..." on the success path (no exception), and it doesn't actually guard the monitor commands when the process is dead. Please restructure this section so the liveness check wraps the monitor interaction (or move the else to pair with if child.isalive()).
| # Make absolute path just in case | ||
| # The shell script cd's into `args.build_dir` before executing us, so `args.build_dir` might be relative to the shell script's pwd. | ||
| # We resolve it relative to the python script's original invocation cwd. | ||
| build_dir = os.path.abspath(args.build_dir) | ||
|
|
There was a problem hiding this comment.
build_dir = os.path.abspath(args.build_dir) is resolved relative to the current working directory. When sof-qemu-run.sh does cd "$BUILD_DIR" and still passes --build-dir "$BUILD_DIR" (typically a relative path like build-qemu_xtensa), this becomes <builddir>/build-qemu_xtensa, and the script will fail to find CMakeCache.txt/zephyr/. Either stop cd-ing in the shell wrapper, pass an absolute path (compute it before cd), or have the Python script detect this case and fall back to os.getcwd() when args.build_dir doesn’t point at a valid Zephyr build directory.
|
|
||
| # Finally run the python script which will now correctly inherit 'west' from the sourced environment. | ||
| python3 "${SCRIPT_DIR}/sof-qemu-run.py" --build-dir "${BUILD_DIR}" | ||
| python3 "${SCRIPT_DIR}/sof-qemu-run.py" --build-dir "${BUILD_DIR}" $VALGRIND_ARG |
There was a problem hiding this comment.
The wrapper cds into BUILD_DIR and then still passes --build-dir "${BUILD_DIR}" to the Python runner. If BUILD_DIR is a relative path like build-qemu_xtensa, the Python script will resolve it relative to the new cwd and end up with <build-qemu_xtensa>/build-qemu_xtensa, breaking path lookups. Please either: (1) pass --build-dir . after the cd, (2) compute an absolute BUILD_DIR before cd, or (3) remove the cd and let the Python script run from its invocation directory.
| python3 "${SCRIPT_DIR}/sof-qemu-run.py" --build-dir "${BUILD_DIR}" $VALGRIND_ARG | |
| python3 "${SCRIPT_DIR}/sof-qemu-run.py" --build-dir . $VALGRIND_ARG |
| # We need to send Ctrl-A c to enter the monitor | ||
| if child.isalive(): | ||
| child.send("\x01c") # Ctrl-A c | ||
| try: |
There was a problem hiding this comment.
shouldn't this try block also be under if - indented?
| case $1 in | ||
| --valgrind) | ||
| VALGRIND_ARG="--valgrind" | ||
| shift |
There was a problem hiding this comment.
could just have a single shift after esac
Add support for native sim target and include being able to run under valgrind. This should support all cmocka tests as ztests meaning more/all can be removed. Will be added to CI soon.