From patchwork Wed Jan 25 17:23:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Metcalf X-Patchwork-Id: 19024 Received: (qmail 12324 invoked by alias); 25 Jan 2017 17:24:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 12284 invoked by uid 89); 25 Jan 2017 17:24:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=funky, modifier, forbid, offer X-HELO: EUR01-HE1-obe.outbound.protection.outlook.com Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=cmetcalf@mellanox.com; Subject: Re: [PATCH] tst-setcontext2: avoid bug from compiler optimization To: Torvald Riegel References: <1484330513-61379-1-git-send-email-cmetcalf@mellanox.com> <8d532404-c012-c999-c60e-8d33a2932afb@mellanox.com> <1485343437.4311.156.camel@redhat.com> CC: From: Chris Metcalf Message-ID: <6c06a49b-8f4d-ad5f-53c1-984bd90a687b@mellanox.com> Date: Wed, 25 Jan 2017 12:23:41 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1485343437.4311.156.camel@redhat.com> X-ClientProxiedBy: DM5PR18CA0056.namprd18.prod.outlook.com (10.168.111.146) To DB6PR0501MB2760.eurprd05.prod.outlook.com (10.172.226.12) X-MS-Office365-Filtering-Correlation-Id: 2b942dd0-234b-4cb5-ef9e-08d44546efcb X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001); SRVR:DB6PR0501MB2760; X-Microsoft-Exchange-Diagnostics: 1; DB6PR0501MB2760; 3:C2ZYoOY8cb/mn01CRRBvwohoB27VvgOQaAKTO+hdh643SGK10VGUTZQZZi0bsgzpVi3Dc/71VLjxFeKSoFTfasIKKo/ssUMW6u1ytIQ2dbvyx6eH7q97NNjg0ZBi/W1n8UZR/4pgcyshyP+Q/KX5d7EAXizTIWHMnZ4qInQKdqf0nkdMx32P25YlHtr5BO62baVVJ/o4dFzF6qnB7OE4cHQuRkzXG8VyIQDXbEEVeAQBEezjfUOCYVNr35tzFKMaK+JN33Qi/8GzfLVblQJOJA==; 25:EiLxBaZVZpXIiVtVqudE8cbO8TY6tYVtsQHjnbiOQMbEhedBuE7rvPvkFA4F8GRIYe2AHTpC1X5GbrmFdoZnDrxOBAAh0wgM/KU9385Nf6bv8+kvC1eZcrywNb1az4n1Z6zyaoHtqoq5jjSBIyHKWPc29qiaO0Yomj0Y7K0WTV88c22rMcXTo2HCQo+SU6cEnbvhyJ+Beqgf7KSLtylKvcdZCqiKRZq4hTM82pfwhCT9f9w5yOpS4vYUtWHV7I65WcZbtKYHR6nIrX7ctn9DAxLzBYFIpu8eyigxU41ue42gMkxpvEZL9q7QNC/2HW+zUjwrNDYQxnQ9Ih24HckVDn+plSvJ7HdQd4Ntg6YmhzFtHkfVv3EfLI999w7BwyEJpdIp3dX+srV0WSlH+WcYbUvsijWzllA4EZyaPEDJmnEdjpEzeydF367/DChJJzoLaLax0Vg+MWvDIZ4uEgd+JQ== X-Microsoft-Exchange-Diagnostics: 1; DB6PR0501MB2760; 31:yMtTbLRQyfL4dMTLsN4XzPn3E15tQrjNWwUN2oelNkc7YR7CmbC2y+oEMRurjq5saNhuiBSMgoWEVW3im4yQ4XUHOm98Yw/PDZURuswDEtpFTtZ/X/x0Kmto8JhVxs0ra0aLe7Dvz1pPgz1oDxhTdUxdlYHypC0HpuD3hmxHb6gBuGosTSknMCwTixSowo8AM/rcdjsYgJ4kk85oXoifSrTzBgbgyfJUuz+tiSfKx9IkhjbCwaGcwmYJfABNgRahGTzyEeh+1muBvVHpwnUqPA==; 20:q8GahIL8p1s2TMMVLgih+gP7K9fKMRah1JWFstzc4DWPEi+DxrtEZqIJ/sRQgWsxutW5dhEgxplWFBZmhawS8/8x46Z2oy5NpenGrM/pWGOBhIcVmZjFrM9PP0O4rJHb6Ln3R4cdVV5whWkogBZ/hx0maOJlNxHIEVqm/+uW1vSOghTWf3HxGz4oAEgC1HfrJYvMz58z3LMCklBGR6a4K/OwpikP/K3ZCKCM7VnADNZ89SM31Jid9Hnh8P2E3Cg7umpQtELOpWeReKgC/RY1GWKjk1oIuh7yVE6EC2xOt2ifKKmwNzmlRr4GCNcPxmaWdL3egW20XcIjsc/FFMYKtoS7UI6Xtnq2aziu6QZwHyKeNpndmK2RGy7CEgxs2zfcdGdRM54YrSzqXotbR9eVQE05eTRuqxg3DiFn0BDKESMq+NqYt1psMzeJ4BMKqFysz8Ef58KCV8X0t5FTi9DPz3+jdX7Z3+ggAUuDG0+HlDHRUFi3YxfzznXijkWilGD7 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(190756311086443)(171992500451332); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026)(6041248)(20161123560025)(20161123562025)(20161123555025)(20161123564025)(6072148); SRVR:DB6PR0501MB2760; BCL:0; PCL:0; RULEID:; SRVR:DB6PR0501MB2760; X-Microsoft-Exchange-Diagnostics: 1; DB6PR0501MB2760; 4:GArE4mq+KAmyoBA7C5JrGuu0XkwF11SrEaYlegNOzBaqAJM1dfFg5msvqzP6h86MXQd4clhtN5A+rXmhogfZ/G7BtQHx8Jv9n5gVrsPM9c86oc0tivDgzAFAScK8QvQVnttfF+7b3VQVxkfGggElQzfPtPkCC4j9NT158jmRxTqwWx8m8HVsnggVccFr8M6ZpD7yPLYFHkWcHLjnjPwKneyu15ZELr8064vy9ILfE6QmeZiJgqpSKVSLMbIkTF5jfkUvBCJXCJXwc/QXhIASXIanXsCjJY39/+Y7hTbGRT1TerlOsq1T6gT9bPHy0JsEDlHhy1jIDjltuzzpePhsHHvrj9tLgP/rdhdX+fcWdtIwyu/Kc9tkmfVIoR9sECqqKDQXwVU1VBCnJ24K3hc9oOw6nch7o8W5o6wVp/YvGi8ehM24jsnjlpBpVCr8ghYbAtEkAJYdIs1g7oYxf3slkpuO2Iey2Q1CzCXGnDmGExF9f0AJqFdkXy9lTxWxngSbP1DizKGKyvKnlckrbZsliDRwSl6RDMBs5/XNDQBzO1fzkvezNxlSGXKzTJarPNpVYllAY6fuArXLRetdVmmvKvviO/E9G/xR5DeJ5e2iApxYsklK6Mt6K/YRCGdXpyKO1niWicfvdmrtKD4o8cUllnz8E1jbP3EAo4JcoE+x4V4= X-Forefront-PRVS: 01986AE76B X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(6049001)(7916002)(39410400002)(39850400002)(39860400002)(39840400002)(39450400003)(377454003)(189002)(199003)(24454002)(377424004)(31686004)(81166006)(53936002)(42186005)(47776003)(23676002)(68736007)(3846002)(4326007)(106356001)(6116002)(2906002)(50466002)(36756003)(86362001)(8676002)(92566002)(64126003)(81156014)(105586002)(31696002)(83506001)(65956001)(65806001)(76176999)(66066001)(54356999)(50986999)(305945005)(25786008)(97736004)(6916009)(33646002)(101416001)(230700001)(5660300001)(110136003)(65826007)(6486002)(229853002)(4001350100001)(6306002)(38730400001)(7736002)(6666003)(90366009)(189998001)(2950100002)(77096006)(18886065003); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR0501MB2760; H:[10.15.7.185]; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjZQUjA1MDFNQjI3NjA7MjM6N1VqODE4bFNUbzBnQnJ0THRFajhhR3Nu?= =?utf-8?B?SjE0OUhqT0xldlZwWmJLSVp6UWw2MzVVTmMyUFp6dzF0ZGNYdzFuamZFcEl6?= =?utf-8?B?NC9UQzJBdk5kSnNPVU1jYytRUlVMSmIrc1J3bm9PNmFmdGZnNnVQWEF4ZjNv?= =?utf-8?B?Q0g0bFFzS3o4Nk1QTFV5UHp4d3oyV2J3bytxQy9CQWtMaGJwL1hNb2V0QVd2?= =?utf-8?B?RzZGSkhGeDB3cHphWkY2bDN3L3VEd0JDYmpTZmM0N0x3UzIraFR6bGVmcWQy?= =?utf-8?B?elpIYzRYUmlSQ2xSblJJekJ5dmRkY0RGdy9hcFZrWHFFQUhaZHhYc3BxaGJj?= =?utf-8?B?YjdMakVxWnpGOFQ4bFhuUFFXbkR2dXh0dXBVQS9JVUR1endUcnVUODlwbHpY?= =?utf-8?B?dDJoeTBHREU2YXV2dWk0WnNQeFJFcXdOZ2psWmNMNStvODFkS2pyTFZOMHhW?= =?utf-8?B?R2VzRE9kc2EvbU5RVXpDMVhpZ1VEYjczRDZadzl3UmFmcVZwRjNjcWVkMXB4?= =?utf-8?B?ZXpsSTlMWUcrQTlLQmgrK3ZSelIySERSVnBQSjQrcFBycnU3S3BBaWhUMnNM?= =?utf-8?B?aFFpbWVCTlpuQkc4K085UkNpZWZIUUE0RnQwU3NTWXNNbytZeklzL3lwZHNt?= =?utf-8?B?Y0xmc0F1MHFYWE1ScXRTc1liY0FaZllIN3FibDBrdkVJaEJvVGoyek4vd1dx?= =?utf-8?B?RnVZdmNHYmZTMzJJNURpV3h4NDB4cDZVbVlCb1V3QnB3bzF0RUNra1BzZVdx?= =?utf-8?B?TWpveXZYSDVVclRnbUU3aXQybkRjakdFZEJ2VzFhbTZ3TDgrUE9BaFJNUnp6?= =?utf-8?B?MHQ1eExwWG8ydjBmcW5KeXNOUHJHdFRHMXl4dUpKc0ZTLzBwNitBUHQ2WXFw?= =?utf-8?B?TXVzMjYvai9ySkJvQ1daTlNHd2hNa2xCYkxqekRxWlFQY0U5Y2tuSlN5a2Ew?= =?utf-8?B?NE92TUJIZE0zdzdIZ1QvVnhiRDNCMXZQeVFtb0pyYlNucnFGeXJ1SjJEdnI3?= =?utf-8?B?cjRzN2xOeXl3bXo0eDluZzhjZmVtaHprVTcwT3BjQkcxaDJlREVIQnVIQlF2?= =?utf-8?B?SXpNVzlwSkVZZ0E0KzBienpBbzBsY0ZlZ2FsYmhpcTVCWGpTeFlYeUVOS1JV?= =?utf-8?B?L3lMYStxR3oxT3hhSW1zUklyeklpdnpoUU1lcWxXakxxTUwxaUVVaTlqR1l5?= =?utf-8?B?WWxqS3VTYk5IUmM1TURHMW9ZMmZ6Y3VNL1NTOFBablI4Vkl6K1BaYmoydTRK?= =?utf-8?B?OU0vUnpTYmtVbHNLY1hzR3N2MEw2ZTZYS2Vqa0tYcDd2NldGRCtQTXhGYVhU?= =?utf-8?B?SXRibmlwMUxScE54ZnhGSm1WUzhJWS9uK0JBVVdOaUlRaHBvY2t3OUdtQVJU?= =?utf-8?B?RXVVTzhNK0VFRjNGTjNPdlJDV1JSTVVYTEVTNDRWQzJmTzZGOS9WZm85ay9S?= =?utf-8?B?K2JxWUxhQWU0VzRETEE2clRIMWN6RTZXZmJITlNCVGluaUZ3Rll3aWFTaks5?= =?utf-8?B?OEF5U0NkbDVrRjJ0QWN0c0dkbVo0R3Rta1BVVS9yV1hJOC9XQ0pDMndVSk1W?= =?utf-8?B?aHh5a21GK3dMZU8ySWtOWjdGYVhER1FoeWJEQnQzMTc5UmR3Nzdpd0I0cWxE?= =?utf-8?B?Vm1ZQm1Zc0UwSy9VSkovUmpaQUQ3S3F1dW56bDR6ejNCSzVYaGVSM2V2dnF4?= =?utf-8?B?b1JzdWVwRWhxTW1uWmxaMjZ2ZkQ0MlRJYlpRclJ5ZEphUUVMRmhJT09yZm9w?= =?utf-8?B?V01HZW9xcUYrUHlCc3FBUnBJc0tPUHFBUm9lOTEremdRVlJKMFdpeGxjcTNz?= =?utf-8?B?REIwZy9WZTAvOVYxN24ycVRSTEY3aWJjM2phajZmTjlCUnpic1RTcSsvZFZk?= =?utf-8?B?QWUxSmE0SFAzby9oWFd5aHRKbXVDd21uVkdOb1JndC92b3lQRkJKajduT2Iz?= =?utf-8?B?dE9hOTI4bVZNd0RqUzB2RnFvckpEYkIwTUpob00vcFY2ODRxS1NlVTcrZ0dK?= =?utf-8?B?eU9aTVlXQ1NBUkxuYUtPSURCc1VKZEVYZ0FpdlJ3OUZyTlRYNHZGbk4rWmw1?= =?utf-8?B?MldRbVplcU9RYWpGY3BzOWFHZ0xoT2ZWalBQa20rVnFvelM4WkFQaEI3cWIw?= =?utf-8?B?SEJtZz09?= X-Microsoft-Exchange-Diagnostics: 1; DB6PR0501MB2760; 6:fQtm4B0s5z2c/sBuveSrqsGESkB1S93db+RVXrST8J+//lxONG5ahaBPsECDwsA5+dTsYSpMPhLJzKmoVzinkU71kYZ1r6SQpNed4gU0hZSCPhUFrf0Pc5JDkxAWn3uI8s03YU5e/ImgyPNEuNaYPf9xiRJiH3O4+tBCIGG4EgcIhNc/7OIaKGFyu6EaljOlfs6jGIIkM2NTIhqTTWp1KTo2idUjnW6vtS8X+7JulK1sMW1tnNfHVesIAM2rjvNj+zEx+F+tEJ31HMnqKkjIM3K+6BLcXHQHmAgQmRo6iF/kt2E2dXjcR+pOlj5KeDDuHqw3kpsTdtM80EAtJYeVQZekkXNNxeAo+B2HESguYwvgrNQmSUtAMPi7z28QWM7HFgOz3z9RmzyHG+vYLeYaONB/bUmbFSX8FeWn62c2OnoW/msm2mL/uC0ahwUNGJyLad3MglIM4GAlewwgsfWeIA==; 5:yd0dttM34v0tPXja0UbiomYIJO6UbabGtzEyxGnUvtw0wAnVD/10Ue6fdn4/hiVGplDkEtXDKmuughkEukOu6RiHRaukmxFTk4u5E9ndEk1ihZvaRkVLFCeyZBKj2VlxqapKvOD1VrmJsgOEdhftu3EIWHqSJKk3A30XNpnEzKg=; 24:deUvj7yXAim9yQpGe/7i8dKzhBxIoNgTrABoT/garFpAapxgcPwNBQTw25S078DmdnM+D8++AiW7qHqZJWHg4uyS38xeJ8IL+04bsS57AkA= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; DB6PR0501MB2760; 7:9PWzteQYxYqRgJpsRcdILE/acrGL3AlmUJpBGPqvA+ayIGufeW86zo3SmQDJy1GfyZqzLkLljzDEZ18A0keS9pnEG7XXMZPaUbBSnkvFM0doniAIvWe1cASD9ReszxhpgoasMmMyI75yQ/yAmm9qec7enMB9YsOhVp6YPdsPeLEXXXRJEAuaW8fIfQk9nrBRlG86JT/oyBMt+oErFyDVbmZMO1nSs3GDio6SoKqfT9e2bhTnqGlIGGBfSY3RUxivyrPOzBN0B209BP9qs/8J+iWnaunRomNQFaxqtwwikGxhEss6kDlEA+EvUNBUabXcPPESP1gcog5lbXxjbPZP8IhXSbW8jN9r+j7KrM67I3F0W521p3s9N4mxMUrYY36dbMqVJbW+MTRWbl2SRR2rgcwDwxTRdB1hhgQFUUKh1cA0N/3QVGI3qv8bzXU5PInjc9vLqp+Lv8vhQ+rE/nx8MQ== X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jan 2017 17:23:53.2133 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0501MB2760 On 1/25/2017 6:23 AM, Torvald Riegel wrote: > On Tue, 2017-01-24 at 19:35 -0500, Chris Metcalf wrote: >> Ping! I will plan to commit this later this week if no one objects; it seems >> like a straightforward bug avoidance. >> >> On 1/13/2017 1:01 PM, Chris Metcalf wrote: >>> With an uninitialized oldctx, the compiler is free to observe that >>> the only path that sets up a value in oldctx is through the >>> "if (global == 2)" arm, in which arm we apparently return 0 without >>> referencing oldctx again. >>> >>> Then, after the "if" cascade, the compiler can inline the "check" >>> function and then observe that the sigset_t "set" variable there >>> is only used locally, before any apparent uses of oldctx, and as a >>> result it can decide to use the same stack region for both variables. >>> Unfortunately this has the effect of clobbering oldctx when we call >>> sigprocmask, and results in the test failing. >>> >>> By initializing oldctx at the top, we let the compiler know that it >>> has a value that has to be preserved down to the part of the code >>> after the "if" cascade, and it won't try to place another variable >>> in that same part of the stack. > The compiler would also know what the initial value is, which it could > store somewhere else, which then would still allow for reuse of a stack > slot. Good point. > I agree with Florian that the compiler needs to be made aware that > getcontext can return twice, or something to that effect. This would > tell it that it has to reason about the lifetimes of variables > differently. The problem is that "returns_twice" doesn't offer the semantics we want. It ensures that register-allocated variables are handled properly, i.e. everything is saved to the stack frame prior to calling the function. But here the issue is that the stack frame itself isn't being set up in a way that actually works. And in practice, tagging getcontext and swapcontext with attribute((returns_twice)) does not fix the bug. (It does seem like doing so isn't a bad idea, but it is beyond the scope of fixing this one test bug.) Another way to fix the problem is to make the context variables function static, which should forbid the compiler from doing anything funky with them. (Although do_test itself is static, it is called from main, and the compiler has to assume main could get called again and expect to find the updated context variables still updated, so it can't trickily ignore the static modifier or anything like that, I think.) commit 5a054bf335e350e96e2a38b5d2573f4f26a2185a Author: Chris Metcalf Date: Fri Jan 13 12:50:50 2017 -0500 tst-setcontext2: avoid bug from compiler optimization With an uninitialized oldctx, the compiler is free to observe that the only path that sets up a value in oldctx is through the "if (global == 2)" arm, in which arm we apparently return 0 without referencing oldctx again. Then, after the "if" cascade, the compiler can inline the "check" function and then observe that the sigset_t "set" variable there is only used locally, before any apparent uses of oldctx, and as a result it can decide to use the same stack region for both variables. Unfortunately this has the effect of clobbering oldctx when we call sigprocmask, and results in the test failing. By making oldctx (and ctx) have static scope, we forbid the compiler from performing this optimization. The compiler will be required to allocate them to separate memory so that they would have their updated values if the do_test function were to be invoked a second time. (We know that doesn't happen, but the compiler can't prove it solely by examination of this compilation unit.) Seen on tilegx with gcc 4.8 at -O3. diff --git a/stdlib/tst-setcontext2.c b/stdlib/tst-setcontext2.c index 07fb974c4684..c937f6c396db 100644 --- a/stdlib/tst-setcontext2.c +++ b/stdlib/tst-setcontext2.c @@ -87,7 +87,7 @@ handler (int __attribute__ ((unused)) signum) static int do_test (void) { - ucontext_t ctx, oldctx; + static ucontext_t ctx, oldctx; struct sigaction action; pid_t pid;