From patchwork Tue Mar 1 17:46:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Baumann X-Patchwork-Id: 8467981 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 9037DC0553 for ; Tue, 1 Mar 2016 17:48:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BD0A12015A for ; Tue, 1 Mar 2016 17:48:11 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CAB73200E5 for ; Tue, 1 Mar 2016 17:48:09 +0000 (UTC) Received: from localhost ([::1]:51441 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaoP3-0001rG-9K for patchwork-qemu-devel@patchwork.kernel.org; Tue, 01 Mar 2016 12:48:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaoOI-00017S-NH for qemu-devel@nongnu.org; Tue, 01 Mar 2016 12:48:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaoNj-0000Pe-SH for qemu-devel@nongnu.org; Tue, 01 Mar 2016 12:47:22 -0500 Received: from mail-bl2on0125.outbound.protection.outlook.com ([65.55.169.125]:37487 helo=na01-bl2-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaoNj-0000PY-G2 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 12:46:47 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector1; h=From:To:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=LT0E6loI7erM1YLGgHvmv0r+ggkFHypvgQ5thXVuIJM=; b=ZnyAeocVxomsMLNtoo2dz1lYmjqDAOcQHWfMjtrB5Ih7GRm2q4MYPyAaTEt1WLO2SKiTyhh2LptjLDnxvX4/n7bCTWzO9Bnj/PXhKf4roz2FG7YJVjrNK9hzWIw3KSbbexm6ASbNnT45oEwnFu25hquqcSUoP88FelrIVx5c6O4= Received: from BLUPR0301MB2034.namprd03.prod.outlook.com (10.164.22.24) by BLUPR0301MB2034.namprd03.prod.outlook.com (10.164.22.24) with Microsoft SMTP Server (TLS) id 15.1.415.20; Tue, 1 Mar 2016 17:46:46 +0000 Received: from BLUPR0301MB2034.namprd03.prod.outlook.com ([10.164.22.24]) by BLUPR0301MB2034.namprd03.prod.outlook.com ([10.164.22.24]) with mapi id 15.01.0415.022; Tue, 1 Mar 2016 17:46:46 +0000 From: Andrew Baumann To: Stefan Weil , Peter Maydell Thread-Topic: [Qemu-devel] [PATCH] Use special code for sigsetjmp only in cpu-exec.c Thread-Index: AQHRc3hXC1ir2Hc3DU2l9SrHR/59jZ9EW0CAgAAgH4CAAAfVgIAADrgAgABJ5cA= Date: Tue, 1 Mar 2016 17:46:45 +0000 Message-ID: References: <1456808869-20286-1-git-send-email-sw@weilnetz.de> <56D58309.10206@weilnetz.de> <56D595F4.50804@weilnetz.de> In-Reply-To: <56D595F4.50804@weilnetz.de> Accept-Language: en-AU, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: weilnetz.de; dkim=none (message not signed) header.d=none; weilnetz.de; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [2001:4898:80e8:6::724] x-ms-office365-filtering-correlation-id: 443b6d68-3012-48b8-ec0e-08d341f97563 x-microsoft-exchange-diagnostics: 1; BLUPR0301MB2034; 5:EdbxzDoUwuhDOZAnMjXVsEA0H9sL2B2rwGZ8E7wa/GBTGkC/rvdU/Mgl2PhZx9jfBtZyFcZoOh567jHCmcZQPSxw8DvHajFnCGA1Mt+3kqiOUFUtqfHe4s2CoCCGBw24m6e8HIMe7JNqyUwpEc/JcQ==; 24:QvaHZ/62SMXn0JeVhwkXs/mv3s6pzgy8BmLza0pU52xERd4uNQgEbfvj3GmgAt6hJ4Yvc9EyVMlc6ekrO36r4FIPldq9z91klyHmP6TuIxk= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR0301MB2034; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(61425038)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(61426038)(61427038); SRVR:BLUPR0301MB2034; BCL:0; PCL:0; RULEID:; SRVR:BLUPR0301MB2034; x-forefront-prvs: 086831DFB4 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(377454003)(24454002)(86362001)(5001770100001)(1096002)(92566002)(19580395003)(86612001)(76576001)(5002640100001)(11100500001)(19580405001)(74316001)(586003)(1220700001)(5001960100004)(81156009)(102836003)(93886004)(6116002)(99286002)(50986999)(76176999)(122556002)(54356999)(106116001)(3280700002)(2900100001)(77096005)(3660700001)(87936001)(10400500002)(10290500002)(5005710100001)(40100003)(5003600100002)(5008740100001)(2906002)(33656002)(4326007)(189998001)(8990500004)(2950100001)(3826002); DIR:OUT; SFP:1102; SCL:1; SRVR:BLUPR0301MB2034; H:BLUPR0301MB2034.namprd03.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2016 17:46:46.0266 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0301MB2034 X-detected-operating-system: by eggs.gnu.org: Windows 7 or 8 X-Received-From: 65.55.169.125 Cc: Paolo Bonzini , Peter Crosthwaite , QEMU Developer , Richard Henderson Subject: Re: [Qemu-devel] [PATCH] Use special code for sigsetjmp only in cpu-exec.c X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > From: Stefan Weil [mailto:sw@weilnetz.de] > Sent: Tuesday, 1 March 2016 5:16 AM > > Am 01.03.2016 um 13:22 schrieb Peter Maydell: > > On 1 March 2016 at 11:54, Stefan Weil wrote: > >> Am 01.03.2016 um 10:59 schrieb Peter Maydell: > >>> I don't understand this patch. Why doesn't it work to have > >>> sigsetjmp() be implemented the same way for every use that > >>> QEMU makes of it? > >> It does, as long as the "same way" is the correct one, namely > >> the one without stack unwinding. > >> > >> The current code used to work, but re-arranged include files > >> broke the working code somewhere in the past: > >> > >> include/sysemu/os-win32.h does the right thing at the > >> wrong place. Its correct definition of sigsetjmp is overwritten by > >> the definition from a Mingw-w64 system header file which > >> triggers stack unwinding. Stack unwinding is fatal for > >> QEMU's generated code. > >> > >> My patch makes sure that the critical code in cpu-exec.c > >> gets the correct definition of sigsetjmp. > > I think we should fix this by making sure that osdep.h > > does the right thing -- ie that it gives us the correct > > definition and prevents mingw's headers from overriding it > > with the wrong thing (by ensuring that the offending system > > header is included before we redefine things, or however > > necessary). This is what osdep.h's purpose is -- to hide > > annoying system-header workarounds and hacks rather than > > putting them in the rest of QEMU code. > > > >> In addition, it removes code which might or might not > >> change the default definition of sigsetjmp (depending > >> on the order of include files). Now all other files beside > >> cpu-exec.c will use the default behaviour with stack > >> unwinding. > > That seems wrong -- we should have the same behaviour for > > sigsetjmp/siglongjmp everywhere we use it. > > > > thanks > > -- PMM > > Technically there is nothing wrong with using different behaviour > for each setjmp or sigsetjmp. > > The "best" solution would be to add any prologue / epilogue which > is needed for stack unwinding to the generated code. Like that, > no tricks with redefinitions of setjmp / sigsetjmp would be necessary. > > As long as that solution is not available, I'd prefer the variant which > is implemented by my patch and keep the workaround close to > the single location where it is needed. > > Your alternate solution would require > inclusion of setjmp.h in include/sysemu/os-win32.h. Then every > compilation for Windows would get that header file, resulting > in a (small) overhead. In addition, there would be no stack > unwinding for any setjmp/longjmp which is not the standard > behaviour for Windows 64 bit (but which we had until it was > broken). I simply don't know whether this has unwanted > side effects (maybe for debugging or with crash dumps) - > that's the reason why I'd minimize the non-standard behaviour. FWIW, I don't see a big problem including setjmp.h from os-win32.h and then modifying the definition globally. The overhead you mention is just in compilation time, and it's pretty minor compared to all the other system header files already included. The lack of stack unwinding is also probably not an issue -- AFAIK nothing in qemu uses structured exception handling, and that is the only reason I'm aware of for needing to be able to unwind from a longjmp. I have been getting along fine with the following local fix: Cheers, Andrew --- a/include/sysemu/os-win32.h +++ b/include/sysemu/os-win32.h @@ -60,6 +60,7 @@ * If this parameter is NULL, longjump does no stack unwinding. * That is what we need for QEMU. Passing the value of register rsp (default) * lets longjmp try a stack unwinding which will crash with generated code. */ +# include # undef setjmp # define setjmp(env) _setjmp(env, NULL) #endif