[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] cpu-exec(): also reload CPUClass *cc after long
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH] cpu-exec(): also reload CPUClass *cc after longjmp return |
Date: |
Mon, 07 Oct 2013 09:28:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 |
Am 05.10.2013 23:45, schrieb Juergen Lock:
> On Sat, Oct 05, 2013 at 08:06:22PM +0200, Stefan Weil wrote:
>> Am 05.10.2013 19:54, schrieb Juergen Lock:
>>> On Fri, Oct 04, 2013 at 09:15:37AM +0200, Jan Kiszka wrote:
>>>> On 2013-10-03 18:05, Peter Maydell wrote:
>>>>> On 3 October 2013 23:09, Juergen Lock <address@hidden> wrote:
>>>>>> Local variable CPUClass *cc needs to be reloaded after return from
>>>>>> longjmp
>>>>>> too. (This fixes the mips-softmmu crash observed on FreeBSD when qemu is
>>>>>> built with clang.)
>>>>>>
>>>>>> Signed-off-by: Juergen Lock <address@hidden>
>>>>>> Found-by: Dimitry Andric <address@hidden>
>>>>>>
>>>>>> --- a/cpu-exec.c
>>>>>> +++ b/cpu-exec.c
>>>>>> @@ -681,6 +681,10 @@ int cpu_exec(CPUArchState *env)
>>>>>> * local variables as longjmp is marked 'noreturn'. */
>>>>>> cpu = current_cpu;
>>>>>> env = cpu->env_ptr;
>>>>>> +#if !(defined(CONFIG_USER_ONLY) && \
>>>>>> + (defined(TARGET_M68K) || defined(TARGET_PPC) ||
>>>>>> defined(TARGET_S390X)))
>>>>>> + cc = CPU_GET_CLASS(cpu);
>>>>>> +#endif
>>>>> This is a c compiler or libc bug -- the C standard says that this
>>>>> local variable should not be trashed by the longjmp. We were
>>>>> actually discussing removing the current workarounds there...
>>>> But we didn't decide if we should stop supporting the affected compiler
>>>> versions.
>>>>
>>>> Does this issue also exist with the latest clang version available for
>>>> your platform?
>>>>
>>> It happens with up to date clang as it's in FreeBSD 10.0-current
>>> which is due for a release soon. I think the clang folks are looking
>>> into this issue but I don't know if a fix will make it into the
>>> release... (For now I've added the workaround to the FreeBSD
>>> qemu-devel port.)
>>>
>>> Thanx,
>>> Juergen
>>
>>
>> Could you try whether QEMU crashes when it was configured with
>> TCG interpreter (--enable-tcg-interpreter)? If it does not crash, it
>> might be that TCG does not save / restore enough registers.
>>
> Still crashes the same.
Practical bugfix beats theoretical optimization, so I'm queuing the
patch (w/ message tweaked) until someone comes up with a better one:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu
Thanks,
Andreas
>
>> Which register is used for the local variable 'cc'?
>>
> Here is the original debug log with part of the disassembly:
>
> http://people.freebsd.org/~nox/tmp/qemu-1.6.0-mips-softmmu-crash.txt
>
> (I wrote the comment at the top before I knew cc needs to be reloaded...)
>
> So apparently cc gets loaded from the stack before the crash: -0x40(%rbp)
>
> Thanx,
> Juergen
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg