cpu.h: join MCIPS vCPU thread at teardown to fix use-after-free#94
Open
Alwalid Salama (alwalidsalama) wants to merge 6 commits into
Open
cpu.h: join MCIPS vCPU thread at teardown to fix use-after-free#94Alwalid Salama (alwalidsalama) wants to merge 6 commits into
Alwalid Salama (alwalidsalama) wants to merge 6 commits into
Conversation
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
4 times, most recently
from
July 15, 2026 23:03
d30dc23 to
5db0241
Compare
Alwalid Salama (alwalidsalama)
requested review from
Jerome Haxhiaj (jeromehaxhiaj-qti),
mahmdkamel and
Mark Burton (markfoodyburton)
and removed request for
Jerome Haxhiaj (jeromehaxhiaj-qti),
mahmdkamel and
Mark Burton (markfoodyburton)
July 15, 2026 23:12
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
3 times, most recently
from
July 16, 2026 08:04
5d5672a to
418678e
Compare
Alwalid Salama (alwalidsalama)
requested review from
mahmdkamel and
Mark Burton (markfoodyburton)
July 16, 2026 08:12
mahmdkamel
approved these changes
Jul 16, 2026
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
2 times, most recently
from
July 16, 2026 13:26
83da827 to
ba0f9a8
Compare
The MCIPS vCPU is a free-running MTTCG thread not gated to SystemC, and McipsSync had no on_destroy() hook. end_of_simulation() only requests halt+unplug asynchronously, so after sc_stop() the thread could still run in tcg_cpu_exec() and access the QemuInstance after it was freed, causing intermittent SIGSEGV/hangs in the mcips reset tests under CPU contention. Add McipsSync::on_destroy() to join the vCPU thread (cpu_remove_sync under the BQL) before the QemuInstance is destroyed. Safe here because the sim has stopped and runonsysc jobs are cancelled. Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
reset_cb() queues async_safe_run(reset(true)) then spins: m_cpu.kick(); wait(m_start_reset_done_ev). qemu_cpu_kick() broadcasts the vCPU's halt_cond without the BQL, so a bare kick can be lost against a vCPU entering qemu_cond_wait(), a vCPU already sleeping there (e.g. in WFI) never runs the queued job, so m_start_reset_done_ev never fires and reset_cb hangs (seen as intermittent 100s ctest timeouts on mcips+MULTI tests). Take the BQL around the kick so the vCPU is either already waiting (gets the broadcast) or sees the queued work when it re-acquires the BQL; release it before waiting so the vCPU can run the job. Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
6 times, most recently
from
July 16, 2026 18:07
047eb15 to
7304cc0
Compare
The Windows coroutine-test guard used `if(... ${num_cpu} in
WINDOWS_COROUTINE_ALLOWED_NUM_CPU ...)`, but CMake has no `in` operator.
Older CMake tolerated it; stricter newer CMake (clangarm64 runner) rejects
it with "Unknown arguments specified".
Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
2 times, most recently
from
July 16, 2026 21:45
f9be0bb to
b370b85
Compare
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
from
July 16, 2026 23:17
7a63adb to
7cdbf2d
Compare
Detaching the last suspending channel must wake a kernel already blocked in async_suspend(); SystemC's detach_suspending() clears the flag without posting the suspend semaphore. The cross-thread path already re-posts via async_request_update(); the same-thread path did not, so a same-thread detach of the last channel left the kernel asleep forever (mcips tests hung with 0 attached channels, main thread stuck in async_suspend). Post from the same-thread path too. Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
Alwalid Salama (alwalidsalama)
force-pushed
the
reset_test
branch
from
July 16, 2026 23:21
7cdbf2d to
48eb82a
Compare
Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
Signed-off-by: Alwalid Salama <asalama@qti.qualcomm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The MCIPS vCPU is a free-running MTTCG thread not gated to SystemC, and
McipsSync had no on_destroy() hook. end_of_simulation() only requests
halt+unplug asynchronously, so after sc_stop() the thread could still run
in tcg_cpu_exec() and access the QemuInstance after it was freed, causing
intermittent SIGSEGV/hangs in the mcips reset tests under CPU contention.
Add McipsSync::on_destroy() to join the vCPU thread (cpu_remove_sync under
the BQL) before the QemuInstance is destroyed. Safe here because the sim
has stopped and runonsysc jobs are cancelled.