From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53405) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLz3e-0002dq-9O for qemu-devel@nongnu.org; Tue, 17 Sep 2013 13:27:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLz3W-0006Va-TH for qemu-devel@nongnu.org; Tue, 17 Sep 2013 13:27:26 -0400 Received: from [2a03:4000:1::4e2f:c7ac:d] (port=36301 helo=v220110690675601.yourvserver.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLz3W-0006VU-Iy for qemu-devel@nongnu.org; Tue, 17 Sep 2013 13:27:18 -0400 Message-ID: <523890E9.5030000@weilnetz.de> Date: Tue, 17 Sep 2013 19:27:05 +0200 From: Stefan Weil MIME-Version: 1.0 References: <1379437402-30804-1-git-send-email-sw@weilnetz.de> <52388EC1.6030108@web.de> In-Reply-To: <52388EC1.6030108@web.de> Content-Type: multipart/alternative; boundary="------------030003000206090209000500" Subject: Re: [Qemu-devel] [PATCH] cpu-exec: Fix compiler warning (-Werror=clobbered) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-trivial , qemu-devel , Anthony Liguori This is a multi-part message in MIME format. --------------030003000206090209000500 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Am 17.09.2013 19:17, schrieb Jan Kiszka: > On 2013-09-17 19:03, Stefan Weil wrote: >> 'cpu' and 'env' are not modified after sigsetjmp. Therefore they will >> still have their last value after longjmp restored the stack context. >> >> The code which should "reload" both variables causes a compiler warnin= g: >> >> cpu-exec.c:204:15: error: >> variable =E2=80=98cpu=E2=80=99 might be clobbered by =E2=80=98longjmp=E2= =80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] >> cpu-exec.c:202:28: error: >> argument =E2=80=98env=E2=80=99 might be clobbered by =E2=80=98longjmp=E2= =80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered] >> >> Remove this unneeded code. >> >> Signed-off-by: Stefan Weil >> --- >> >> Jan, >> >> could you please review this patch which removes code added by you earlier? >> I have run tests with the old code and assertions to see whether the values >> were really smashed. They never were, and from the documentation of setjmp >> I'd not expect that they ever might be. >> >> The patch is needed to fix a compiler warning with -Wextra. > > This used to fix a real, deadly crash. Therefore a reversion can't be > trivial by definition. Unfortunately, I don't recall which compiler > version and concrete scenario were involved back then. > > Anyway - did anything change in the code structure around since then? > Does anything ensure that this "optimization" is not longer performed b= y > the compiler? > > I'll try to understand the warnings meanwhile. > > Jan > The code changed a lot since that time, e.g. setjmp was replaced by sigsetjmp. Maybe you had a broken compiler which could be forced to do the right thi= ng by that code? Stefan --------------030003000206090209000500 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Am 17.09.2013 19:17, schrieb Jan Kiszka:
> On 2013-09-17 19:03, Stefan Weil wrote:
>> 'cpu' and 'env' are not modified after sigsetjmp. Therefore they will
>> still have their last value after longjmp restored the stack context.
>>
>> The code which should "reload" both variables causes a compiler warning:
>>
>> cpu-exec.c:204:15: error:
>> variable =E2=80=98cpu=E2=80=99 might be clobbered by =E2=80= =98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered]
>> cpu-exec.c:202:28: error:
>> argument =E2=80=98env=E2=80=99 might be clobbered by =E2=80= =98longjmp=E2=80=99 or =E2=80=98vfork=E2=80=99 [-Werror=3Dclobbered]
>>
>> Remove this unneeded code.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> Jan,
>>
>> could you please review this patch which removes code added by you earlier?
>> I have run tests with the old code and assertions to see whether the values
>> were really smashed. They never were, and from the documentation of setjmp
>> I'd not expect that they ever might be.
>>
>> The patch is needed to fix a compiler warning with -Wextra.
>
> This used to fix a real, deadly crash. Therefore a reversion can't be
> trivial by definition. Unfortunately, I don't recall which compiler
> version and concrete scenario were involved back then.
>
> Anyway - did anything change in the code structure around since then?
> Does anything ensure that this "optimization" is not longer performed by
> the compiler?
>
> I'll try to understand the warnings meanwhile.
>
> Jan
>

The code changed a lot since that time, e.g. setjmp was replaced by sigsetjmp.

Maybe you had a broken compiler which could be forced to do the right thing
by that code?

Stefan


--------------030003000206090209000500--