From 2c926832bb4c860c5d5fcb8789b2ec7f11d2c046 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 4 Mar 2021 18:32:42 -0500 Subject: [PATCH] accel/tcg: Add URL of clang bug to comment about our workaround In cpu_exec() we have a longstanding workaround for compilers which do not correctly implement the part of the sigsetjmp()/siglongjmp() spec which requires that local variables which are not changed between the setjmp and the longjmp retain their value. I recently ran across the upstream clang bug report for this; add a link to it to the comment describing the workaround, and generally expand the comment, so that we have a reasonable chance in future of understanding why it's there and determining when we can remove it, assuming clang eventually fixes the bug. Remove the /* buggy compiler */ comments on the #else and #endif: they don't add anything to understanding and are somewhat misleading since they're sandwiching the code path for *non*-buggy compilers. Backports e6a41a045c298538d303cd8fe8d7ae29a0c066ad --- qemu/accel/tcg/cpu-exec.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/qemu/accel/tcg/cpu-exec.c b/qemu/accel/tcg/cpu-exec.c index c5fc8707..d4c986cd 100644 --- a/qemu/accel/tcg/cpu-exec.c +++ b/qemu/accel/tcg/cpu-exec.c @@ -551,18 +551,30 @@ int cpu_exec(struct uc_struct *uc, CPUState *cpu) /* prepare setjmp context for exception handling */ if (sigsetjmp(cpu->jmp_env, 0) != 0) { #if defined(__clang__) - /* Some compilers wrongly smash all local variables after - * siglongjmp. There were bug reports for gcc 4.5.0 and clang. + /* + * Some compilers wrongly smash all local variables after + * siglongjmp (the spec requires that only non-volatile locals + * which are changed between the sigsetjmp and siglongjmp are + * permitted to be trashed). There were bug reports for gcc + * 4.5.0 and clang. The bug is fixed in all versions of gcc + * that we support, but is still unfixed in clang: + * https://bugs.llvm.org/show_bug.cgi?id=21183 + * * Reload essential local variables here for those compilers. - * Newer versions of gcc would complain about this code (-Wclobbered). */ + * Newer versions of gcc would complain about this code (-Wclobbered), + * so we only perform the workaround for clang. + */ cpu = uc->current_cpu; env = cpu->env_ptr; cc = CPU_GET_CLASS(uc, cpu); -#else /* buggy compiler */ - /* Assert that the compiler does not smash local variables. */ +#else + /* + * Non-buggy compilers preserve these locals; assert that + * they have the correct value. + */ g_assert(cpu == uc->current_cpu); g_assert(cc == CPU_GET_CLASS(uc, cpu)); -#endif /* buggy compiler */ +#endif // Unicorn: commented out //tb_lock_reset(); }