Message ID | 8c08307a-94ad-92b8-9c8b-c713cad541fd@mathworks.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 55780 invoked by alias); 19 Oct 2017 15:59:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 55754 invoked by uid 89); 19 Oct 2017 15:59:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=struggling, Hx-languages-length:2990, H*M:92b8, H*M:9c8b X-HELO: NAM01-SN1-obe.outbound.protection.outlook.com Received: from mail-sn1nam01on0046.outbound.protection.outlook.com (HELO NAM01-SN1-obe.outbound.protection.outlook.com) (104.47.32.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 19 Oct 2017 15:59:41 +0000 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Mike.Gulick@mathworks.com; Received: from [172.28.194.135] (144.212.3.4) by BY2PR0501MB2037.namprd05.prod.outlook.com (10.163.197.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.156.4; Thu, 19 Oct 2017 15:59:37 +0000 Subject: Re: [RFC][PATCH] fix gdb segv when objfile can't be opened From: Mike Gulick <mike.gulick@mathworks.com> To: gdb-patches@sourceware.org References: <59E8B251.4050100@mathworks.com> Message-ID: <8c08307a-94ad-92b8-9c8b-c713cad541fd@mathworks.com> Date: Thu, 19 Oct 2017 11:59:33 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <59E8B251.4050100@mathworks.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: BN6PR1201CA0005.namprd12.prod.outlook.com (10.174.238.15) To BY2PR0501MB2037.namprd05.prod.outlook.com (10.163.197.24) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 01f2846e-f0b2-4552-5bdf-08d5170a669b X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254152)(2017052603199)(201703131423075)(201703031133081)(201702281549075); SRVR:BY2PR0501MB2037; X-Microsoft-Exchange-Diagnostics: 1; BY2PR0501MB2037; 3:dZ9Pb+pOd8PLSCc74Vr8uuWRui9+kECAxYcpjcTyNVL1E1RbSqOADZmYV5fZ8zbuGwis3ji0awqIr4/mZSarCslACTd1xWzJKnKmnh/GUOSmQSzbgUfX3nuBiAxmNxEWUbRMoEGZe5PHszz8SEGckZMpZtiUGWpkKjX/PuSpSX6e/Qph55+VFlx/Hqkt31UMEjfyGKML5gNWVtQRkWPZ5vsPMEV5L3tR2ZXa+5ZpLcJZrj0e+Rnhk0+jUUqUvVSp; 25:5RdgcDqVbllQHvxlf+gUJImrt1CfbWFmnZ8zboOSoctI0TuFWizIkfp47AW9rHXzCOJBLjYyPboukwpBliT2EIPt4dS39iN2XYsmXIBhdIoKYO943M1ShlyKW2FGAC+WqDo5D2pmUmSss9SLGvzeZ+nCK1MmfSHWLXkTrmwXUsxfKlaQzAlbUYasoHwml4On98KD7nrkl1KGoxgFMg6k9p8YHGP9ERIo5iU9ALJHwU2Yac9jIgGGH5GJL6XUJaECRGYRRse34H35twuWHXXR9xwhTcxkCYKSWel14QZqiur1urMkEjA+arJIMtexR6RHpH8cNsE97HutK5V2B5nHGQ==; 31:vmVWrUbwua5ZnzFQiG3lEUrqnZDfS9prlX+5YJhc9bJlqBSq7YLdqt9PF5uVzdknSxMHtXu6GgKQ1peQxI5XCtdh3tZECRSR08B0z+fkd/OtwcHnh28nSDVF7lebRwjZ8mdpJHKzYVMUDATGULYJaHYf3BZLxEk2CQIF0YS1tqKzkyre9UEAoZUqm0iYGShRsnPbda89jwovYHmPdLW1okTxh+5YXAxksyMr3aqmlcc= X-MS-TrafficTypeDiagnostic: BY2PR0501MB2037: X-Microsoft-Exchange-Diagnostics: 1; BY2PR0501MB2037; 20:wKQTfQKHAjPTbKxfNzUDJZYgYpfOZl18QlR9KDChCYPpWTvCYndjHSMcQCPz1tyvP6B+fqlcVgccvGzD3DjeD7z32FDju9sOBK0WKLmQlAFf6e/e3u+0N88Duw2vst7zNX98VQ1qgcXF9JkRjDw8Oy5vAYUjcrZRxFU5KYL8A4cJzPqjgVWKEEHjb7bf34llT21y03ZDyEh8rK+nrs0B8fvp13rZqlDRMj0EXCoVPOgj2KUNx5JQnTeh98vWWgLmN6mptEVkjCyhx/Jddi48m5JxF/iB8Agyek5ePPsFCFaqUbW07mlGugfsP354qsdlxhRqLFsb6s1Y5N1AeYryUzMhA+vXcrroaKtgT5sPQi8ITWj224R7hBj3f49pgJYPINgMALLqIvnB4rDafbOFvvGXOGrYAmMRtEKf2snitp0COxgsKFeTKetJFqbNMHVSk5cmWIgoq+ccP19WFVBQAFwzi7o8F1Ah5mRZRdhFv1N1F3J1DS8Z0IV+Y4Yao9Kz; 4:K/gTV7F4tsOr+N5ul/R2K4bjinVExm+WIB6h0hsNqe7cJLN6X5QrNbQ43E6bxJOBZY9DTHFLDmhrw6Hoj1LH1JCfudqb69ZIpnucpDebCxYDwtJUWmx4mHnZM7EoVFik0c7YdGLc859z33s+PvJ+JnhEnVQ33KvgRYG8RAfoUFXf5YN/xcV4mOmKLStWDFFmRtp6zVdTecTqulvd+D6AfRRR49cDffbbV/qvNmL15CHzDLn1d1oJHlai+NiIJtbT X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: <BY2PR0501MB2037D623AFFEAF9F73EFED99EC420@BY2PR0501MB2037.namprd05.prod.outlook.com> X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(100000703101)(100105400095)(93006095)(93001095)(3002001)(10201501046)(6041248)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123555025)(20161123560025)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:BY2PR0501MB2037; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:BY2PR0501MB2037; X-Forefront-PRVS: 0465429B7F X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(6009001)(346002)(376002)(189002)(199003)(77096006)(6486002)(305945005)(189998001)(2906002)(81166006)(81156014)(90366009)(8676002)(25786009)(86362001)(31696002)(7736002)(72206003)(6246003)(53546010)(50466002)(8936002)(65826007)(83506002)(16526018)(2950100002)(6916009)(97736004)(68736007)(105586002)(106356001)(3846002)(58126008)(16576012)(2351001)(5660300001)(101416001)(23676002)(6116002)(76176999)(54356999)(50986999)(230700001)(65806001)(65956001)(66066001)(47776003)(6666003)(36756003)(64126003)(33646002)(316002)(2361001)(31686004)(478600001)(229853002)(53936002); DIR:OUT; SFP:1101; SCL:1; SRVR:BY2PR0501MB2037; H:[172.28.194.135]; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: mathworks.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCWTJQUjA1MDFNQjIwMzc7MjM6T21PalozYUdxNldMNStDTkhOdGNnN1pY?= =?utf-8?B?MTAxeG84WFE1Z2JFUElsUHpsYmVENFVmN1lvTDRUamJFKzZRaytRbkY1Y1FT?= =?utf-8?B?c3ptQ25KM29wQTNJOExKNFJBNWo3K1JBVWlRc0g3SUU0Y3Mvc1ZicEViZlp4?= =?utf-8?B?bmFIdUdMRWZQUUt6cEtMZkYyRUVPNVhxNk81bFpXMFZqTHJrRGxaOXk3dlNQ?= =?utf-8?B?bWx4bG5HVVU5QlZ3Q3NEdUVmQmlmOU5SZG5Sd2dvRVJBcm5Ob1FQdjREVEda?= =?utf-8?B?YUZIUjlUVDBvb1VRSWZwNjVYQkl5R2I5bi93UUQwcWk5b3RHSlF0V1k1V2py?= =?utf-8?B?N3BxNmx2ZjRxVG9peXhpc056L3Vrb2d3OXdKMzRnaVRSYWsvT3lWMTJlUlcv?= =?utf-8?B?YkVpTGdHUWFCVk93TUFGcEdIOE9uYnNEek41ZWpXS2lRUWtzOUphdkNJa3FM?= =?utf-8?B?SkhjdmFLb29FNHE0b3VyWXg3MExMMjhuMVllMUNmcVpIN2tlblIrSUE3Umxt?= =?utf-8?B?T3YvNDhjWlJsUm5YS1VUbmZrTlRZdkNpS0lGdXJSeFc0bHZnSDhEb0YxaDZL?= =?utf-8?B?RmM3VmQvVGlmcUpmc2UwcWRxanBBYTZxWXdENmQzdWF2Y3JwTWNkQy9TamlT?= =?utf-8?B?Mm9ISFlWaytqTTR0NHRKMm9DbzQzLzRVdHZkTHdwWVdVRVBZYUQ2UEZ1OWhO?= =?utf-8?B?NGdaNXhnZjM4OElocGdrN0M0Yks1ekJVSWVOOE03SmlHdWdkOVBTcXU3Mm1w?= =?utf-8?B?S25zYzVoaFIwR3BRNVpHa2k5alB2QUVKL1M3U2d5NGhCYmhEejFqWmNYcCtT?= =?utf-8?B?VGpVMXYybmRDVzNkZ0x6STZSc3gxV3RxbklXZ1FXbTBqalUyVUlmajl4ZjBk?= =?utf-8?B?cys5V29RbFB3S1NDT2p0c0psM0dvZ0RaTG1IbkVJalFDN2JhSlVxT2F5QnNq?= =?utf-8?B?VDZUQmtZV2xJTkRWMkw4OWptcGdzMXhCSldRVnl1cWpCWjdyeENoaGdhZWhT?= =?utf-8?B?WmJTeFFhTklYbno1alFvUG4xeWZWbTRpVU40bTBLUkV4YUJCM1JYMWVxM3Fn?= =?utf-8?B?aE1OanhwbDZCTHBBcXJISWd4Zy9Eb1E5Nng1RW9lM2NNUG4vNk1Sa2Jsa1J6?= =?utf-8?B?Ym95NmpTanlHcDFTYU1pWEgwVTUwajBHZ2FJNFo1dlFvQ0k4K1ZibksvQnoy?= =?utf-8?B?SkJsenFuMVBselNpeXRYUlRDMWY0a09LMWp0M0wxQXFGSzZjcXpIbWw1Vld1?= =?utf-8?B?a2lUdmVMTkl0K2ZMNXVBV3BqdTc0REo3cEE1YzdvR2VGamlnaWl5QXBwYlRJ?= =?utf-8?B?WExndEJJaU96TmNzZXEzRlFsMW50cjlIZDNZK1lvUmpHTHozVDVYclptQ24v?= =?utf-8?B?UDVycEJCVkZQazkyd24yUnV6UElDc0dTb0ZaVk1sc245R0pxQm5LamVoNTV2?= =?utf-8?B?dHV5aW8vTlBSdUlQUmJKekJwVHRRK3FMcHJPSDVvSmpSN3RIYzkyeUQzclda?= =?utf-8?B?YmJnUGhEaENzRkpNRGxTWVdiRnV3MlV1eGJtMzhhVGpCQ0hHd3FBQWxlM2Ur?= =?utf-8?B?Wmp0OWk5Tk1SbUZhU24zV1F3N1FFa0gySWUxRkxBQUJBa0UzYmFoeUxWK3lQ?= =?utf-8?B?VmVPNXloQkl3VXhWQW8vUHpxYmtmSzlEd2ZCdkFaakY3QWhjL3V0alp0TFNK?= =?utf-8?B?NFV5RjB6eGNGc3NacitZY1N0KytMekowZ3ZXZ0NBSXJMbS9VVHduV1lBM2ty?= =?utf-8?B?dG52WU5IRUluWG1QMXRQcGdCd3ZGcUpIbXFISnZyUU1kbGlkZXRBK3B1THhr?= =?utf-8?B?SUZLL3c1akZDYytBZzV0cFpvRFY1dU9BRm5ZWURhT2M5NjMxdz09?= X-Microsoft-Exchange-Diagnostics: 1; BY2PR0501MB2037; 6:FyAfQ9WngZ0A1dQa1G5OZnIzJSgYxfTravFm1O0yn4qsCxw7xhUssgzmP3ueRcQka0cCpVhW+o53bZuE5wAQdtskEYpm98LiqIjxxPmbYArmfnr2TX50Dkh19KSE+WNM01Lhp0MzXFIPt/wz+OYQBTCP1qvDYg8LPBh4SpxrW+Vg9npX6LqVoKoZogx/SQRqd964ggZfa3oB0CVrlWlRY31S3klDyrxnfZe9jUeUZDLypgqJfeqkJELrRx82H3+rWLI5iRsWkWSHCtvmVKg66ZLt0VpYXBV3yTiq+jaDkYhG+nynsnAvypHLoZbi4jVXMY5aR9o16Ua6VjcWoLzdbA==; 5:Js111yN6O3iAYKhUF2C8INq+d3C1T5IX8fWGlfveQMGAwi8lruNbWI4Ria8FNbuEsOaUTxcbzfRT203J9BeWgOVoKakztYHjYe2Wv2g99jIq4jf+6YmjwYJG9MPU7RO97g1WAJTtF1RuHutFjTuOgA==; 24:Dr0SZhh6VqizQq34hqq1EWrpRp5vnxy0BVDUytr5mw602+eCsTwFlsbxxyFRrpavmBwpg++/7ilaeYmGep8XL1MdavSTtabSWeaaYw1mWVU=; 7:EJKOYdZJJw/C2B1LgygZn/pEInLFllOzcmFQxRDoSWf0xgIQa0fB9aEhd434q5030u7Rw0BSP48qkXVwdcliEPwZcukd1ZiJdtfx7LFmkZ4zPwWmwwySx2TOm4AYeUSKam+BIu5CtknFMiSaornwaNc5kwaCikuNrxoyVPYce1SpkmG3q+prqGyLrlJOgwlwPGOGRnAPnFj5EgyoVRqb6eJ7oRPlcIZrU+1wTxDG9mg= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: mathworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2017 15:59:37.8237 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 01f2846e-f0b2-4552-5bdf-08d5170a669b X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 99dd3a11-4348-4468-9bdd-e5072b1dc1e6 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR0501MB2037 |
Commit Message
Mike Gulick
Oct. 19, 2017, 3:59 p.m. UTC
I apologize for the improperly formatted patch -- I'm really struggling
to get thunderbird to behave as I want.
Here is an updated patch. I would have sent it with git send-email, but
I could not figure out the proper way to add this preface before the
patch (without it looking like part of the commit message).
---
From 5dee04076518554e4baae864569d6f4faee9b685 Mon Sep 17 00:00:00 2001
From: Mike Gulick <mgulick@mathworks.com>
Date: Wed, 18 Oct 2017 16:04:27 -0400
Subject: [PATCH] fix gdb segv when objfile can't be opened
This fixes PR 16577.
This patch changes gdb_bfd_map_section to issue a warning rather than an
error if it is unable to read the object file, and sets the size of the
section/frame that it attempted to read to 0 on error.
The description of gdb_bfd_map_section states that it will try to read
or map the contents of the section SECT, and if successful, the section
data is returned and *SIZE is set to the size of the section data. This
function was throwing an error and leaving *size as-is. Setting the
section size to 0 indicates to dwarf2_build_frame_info that there is no
data to read, otherwise it will try to read from an invalid frame
pointer.
Changing the error to a warning allows this to be handled gracefully.
Additionally, the error was clobbering the breakpoint output indicating
the current frame (function name, arguments, source file, and line number).
E.g.
Thread 3 "foo" hit Breakpoint 1, BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or directory
BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or directory
(gdb)
While the "BFD: reopening ..." messages will still appear interspersed in the
breakpoint output, the current frame info is now displayed:
Thread 3 "foo" hit Breakpoint 1, BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or directory
BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or directory
warning: Can't read data for section '.eh_frame' in file '/tmp/jna-1013829440/jna1875755897659885075.tmp'
do_something () at file.cpp:80
80 {
(gdb)
---
gdb/gdb_bfd.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
Comments
On 2017-10-19 11:59, Mike Gulick wrote: > I apologize for the improperly formatted patch -- I'm really struggling > to get thunderbird to behave as I want. > > Here is an updated patch. I would have sent it with git send-email, > but > I could not figure out the proper way to add this preface before the > patch (without it looking like part of the commit message). Hi Mike, Thanks, I was able to apply this version correctly. If I have a short comment that's not meant to be in the commit message, I usually include it in brackets like this: [Re-sending this patch because the first try was not formatted correctly.] If it's longer you can always end it with a line "Actual commit message:". Either way, it's not really a big deal, as long as it's clear. You can use the --annotate option of git-send-email to edit the message before sending it. > --- > From 5dee04076518554e4baae864569d6f4faee9b685 Mon Sep 17 00:00:00 2001 > From: Mike Gulick <mgulick@mathworks.com> > Date: Wed, 18 Oct 2017 16:04:27 -0400 > Subject: [PATCH] fix gdb segv when objfile can't be opened > > This fixes PR 16577. > > This patch changes gdb_bfd_map_section to issue a warning rather than > an > error if it is unable to read the object file, and sets the size of the > section/frame that it attempted to read to 0 on error. > > The description of gdb_bfd_map_section states that it will try to read > or map the contents of the section SECT, and if successful, the section > data is returned and *SIZE is set to the size of the section data. > This > function was throwing an error and leaving *size as-is. Setting the > section size to 0 indicates to dwarf2_build_frame_info that there is no > data to read, otherwise it will try to read from an invalid frame > pointer. > > Changing the error to a warning allows this to be handled gracefully. > Additionally, the error was clobbering the breakpoint output indicating > the current frame (function name, arguments, source file, and line > number). > E.g. > > Thread 3 "foo" hit Breakpoint 1, BFD: reopening > /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or > directory > > BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such > file or directory For some reason, I am not able to reproduce the crash using the instructions in the bug report, and gdb master. (gdb) up #1 0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) BFD: reopening ./badlib.so: No such file or directory BFD: reopening ./badlib.so: No such file or directory Can't read data for section '.eh_frame' in file './badlib.so' (gdb) Initial frame selected; you cannot go up. (gdb) Initial frame selected; you cannot go up. (gdb) Initial frame selected; you cannot go up. (gdb) bt #0 0x00007ffff78d52f0 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6 Would you be able to create a test case to reproduce it? We would need one to go in with the fix in the end anyway, and it's easier for reviewers if they can just run a test file rather than try to reproduce by hand. You can start by copying an existing solib test, like gdb.base/solib-display.exp. See here for more details about tests: http://sourceware.org/gdb/wiki/TestingGDB http://sourceware.org/gdb/wiki/GDBTestcaseCookbook Don't hesitate to ask here or on IRC if you need assistance. > (gdb) > > While the "BFD: reopening ..." messages will still appear interspersed > in the > breakpoint output, the current frame info is now displayed: > > Thread 3 "foo" hit Breakpoint 1, BFD: reopening > /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or > directory > > BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such > file or directory > > warning: Can't read data for section '.eh_frame' in file > '/tmp/jna-1013829440/jna1875755897659885075.tmp' > do_something () at file.cpp:80 > 80 { > (gdb) > --- > gdb/gdb_bfd.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c > index 29080b8..229f5ae 100644 > --- a/gdb/gdb_bfd.c > +++ b/gdb/gdb_bfd.c > @@ -705,9 +705,15 @@ gdb_bfd_map_section (asection *sectp, > bfd_size_type *size) > > data = NULL; > if (!bfd_get_full_section_contents (abfd, sectp, &data)) > - error (_("Can't read data for section '%s' in file '%s'"), > - bfd_get_section_name (abfd, sectp), > - bfd_get_filename (abfd)); > + { > + warning (_("Can't read data for section '%s' in file '%s'"), > + bfd_get_section_name (abfd, sectp), > + bfd_get_filename (abfd)); > + /* Section is invalid -- set size to 0 and return NULL */ > + descriptor->size = 0; > + *size = descriptor->size; > + return (const gdb_byte *) NULL; > + } > descriptor->data = data; > > done: I don't know if it is really this function's responsibility to clear *size in case of error, or it would be the callers responsibility to properly check for errors. But if the function doesn't throw anymore, the comment in gdb_bfd.h should be updated accordingly. Thanks, Simon
On 10/19/2017 01:54 PM, Simon Marchi wrote: > On 2017-10-19 11:59, Mike Gulick wrote: >> I apologize for the improperly formatted patch -- I'm really struggling >> to get thunderbird to behave as I want. >> >> Here is an updated patch. I would have sent it with git send-email, but >> I could not figure out the proper way to add this preface before the >> patch (without it looking like part of the commit message). > > Hi Mike, > > Thanks, I was able to apply this version correctly. > > If I have a short comment that's not meant to be in the commit message, I usually > include it in brackets like this: > > [Re-sending this patch because the first try was not formatted correctly.] > > If it's longer you can always end it with a line "Actual commit message:". Either way, it's not really a big deal, as long as it's clear. You can use the --annotate option of git-send-email to edit the message before sending it. > Thanks. >> --- >> From 5dee04076518554e4baae864569d6f4faee9b685 Mon Sep 17 00:00:00 2001 >> From: Mike Gulick <mgulick@mathworks.com> >> Date: Wed, 18 Oct 2017 16:04:27 -0400 >> Subject: [PATCH] fix gdb segv when objfile can't be opened >> >> This fixes PR 16577. >> >> This patch changes gdb_bfd_map_section to issue a warning rather than an >> error if it is unable to read the object file, and sets the size of the >> section/frame that it attempted to read to 0 on error. >> >> The description of gdb_bfd_map_section states that it will try to read >> or map the contents of the section SECT, and if successful, the section >> data is returned and *SIZE is set to the size of the section data. This >> function was throwing an error and leaving *size as-is. Setting the >> section size to 0 indicates to dwarf2_build_frame_info that there is no >> data to read, otherwise it will try to read from an invalid frame >> pointer. >> >> Changing the error to a warning allows this to be handled gracefully. >> Additionally, the error was clobbering the breakpoint output indicating >> the current frame (function name, arguments, source file, and line number). >> E.g. >> >> Thread 3 "foo" hit Breakpoint 1, BFD: reopening >> /tmp/jna-1013829440/jna2973250704389291330.tmp: No such file or >> directory >> >> BFD: reopening /tmp/jna-1013829440/jna2973250704389291330.tmp: No such >> file or directory > > For some reason, I am not able to reproduce the crash using the instructions in the bug report, and gdb master. > > (gdb) up > #1 0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6 > (gdb) > BFD: reopening ./badlib.so: No such file or directory > > BFD: reopening ./badlib.so: No such file or directory > > Can't read data for section '.eh_frame' in file './badlib.so' > (gdb) > Initial frame selected; you cannot go up. > (gdb) > Initial frame selected; you cannot go up. > (gdb) > Initial frame selected; you cannot go up. > (gdb) bt > #0 0x00007ffff78d52f0 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6 > #1 0x00007ffff78d525a in sleep () from /lib/x86_64-linux-gnu/libc.so.6 > > > Would you be able to create a test case to reproduce it? We would need one to go in with the fix in the end anyway, and it's easier for reviewers if they can just run a test file rather than try to reproduce by hand. You can start by copying an existing solib test, like gdb.base/solib-display.exp. See here for more details about tests: > > http://sourceware.org/gdb/wiki/TestingGDB > http://sourceware.org/gdb/wiki/GDBTestcaseCookbook > > Don't hesitate to ask here or on IRC if you need assistance. > I attached a new reproducer in the bug report that reproduces this problem in the current git master (and more closely follows the problem I was seeing in our large C++/Java application). This causes the error messages to be emitted when the breakpoint is hit, and the stepping forward triggers the segfault. I'll take a look at building a test case. >> (gdb) >> >> While the "BFD: reopening ..." messages will still appear interspersed in the >> breakpoint output, the current frame info is now displayed: >> >> Thread 3 "foo" hit Breakpoint 1, BFD: reopening >> /tmp/jna-1013829440/jna1875755897659885075.tmp: No such file or >> directory >> >> BFD: reopening /tmp/jna-1013829440/jna1875755897659885075.tmp: No such >> file or directory >> >> warning: Can't read data for section '.eh_frame' in file >> '/tmp/jna-1013829440/jna1875755897659885075.tmp' >> do_something () at file.cpp:80 >> 80 { >> (gdb) >> --- >> gdb/gdb_bfd.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c >> index 29080b8..229f5ae 100644 >> --- a/gdb/gdb_bfd.c >> +++ b/gdb/gdb_bfd.c >> @@ -705,9 +705,15 @@ gdb_bfd_map_section (asection *sectp, bfd_size_type *size) >> >> data = NULL; >> if (!bfd_get_full_section_contents (abfd, sectp, &data)) >> - error (_("Can't read data for section '%s' in file '%s'"), >> - bfd_get_section_name (abfd, sectp), >> - bfd_get_filename (abfd)); >> + { >> + warning (_("Can't read data for section '%s' in file '%s'"), >> + bfd_get_section_name (abfd, sectp), >> + bfd_get_filename (abfd)); >> + /* Section is invalid -- set size to 0 and return NULL */ >> + descriptor->size = 0; >> + *size = descriptor->size; >> + return (const gdb_byte *) NULL; >> + } >> descriptor->data = data; >> >> done: > > I don't know if it is really this function's responsibility to clear *size in case of error, or it would be the callers responsibility to properly check for errors. But if the function doesn't throw anymore, the comment in gdb_bfd.h should be updated accordingly. I had trouble figuring out what the 'error' function actually does (I couldn't find where it was defined). When I'm stepping through GDB in the debugger, the lines past 'error' never seem to get called. It's like 'error' throws an exception that is caught elsewhere. I was also unable to figure out why the error message isn't displayed. The new reproducer shows this issue. I wasn't sure if setting *size or even descriptor->size was the right thing to do, but it seemed reasonable to me since the comment in gdb_bfd.h states that this function updates *size. There's currently only one caller for 'gdb_bfd_map_section', so I have no problem updating *size there if that is preferred. Thanks, Mike
On 2017-10-19 15:39, Mike Gulick wrote: > I had trouble figuring out what the 'error' function actually does (I > couldn't find where it was defined). When I'm stepping through GDB in > the debugger, the lines past 'error' never seem to get called. It's > like 'error' throws an exception that is caught elsewhere. Indeed, "error" throws an exception. You should be able at least to step into the error function (although it's not particularly useful nor interesting). It is defined in common/errors.c. > I was also > unable to figure out why the error message isn't displayed. The new > reproducer shows this issue. I wasn't sure if setting *size or even > descriptor->size was the right thing to do, but it seemed reasonable to > me since the comment in gdb_bfd.h states that this function updates > *size. There's currently only one caller for 'gdb_bfd_map_section', so > I have no problem updating *size there if that is preferred. Actually it says "If successful, the section data is returned and *SIZE is set to the size of the section data;". And this is what I would generally expect from functions. Unless stated otherwise, the value of output parameters should be considered undefined if the function failed. So I would lean towards blaming the caller for not taking enough precautions. It trusts that gdb_bfd_map_section won't fail. Simon
On 10/19/2017 04:09 PM, Simon Marchi wrote: > Indeed, "error" throws an exception. You should be able at least to step into the error function (although it's not particularly useful nor interesting). It is defined in common/errors.c. > Thanks. I looked at the stack when stopped on 'error', and I see that there is a TRY/CATCH in 'frame_unwind_try_unwinder'. That doesn't handle the exception, and just re-throws it. It looks like the exception is ultimately caught in 'print_stack_frame', which explains why the stack frame is never printed when the exception is thrown (although it's still unclear why the message passed to 'error' isn't printed). Here is the top half of the stack: #0 gdb_bfd_map_section (sectp=0x31e4eb8, size=0x31ddc58) at gdb_bfd.c:708 #1 0x0000000000628b15 in dwarf2_read_section (objfile=0x31c0be0, info=0x31ddc48) at dwarf2read.c:2553 #2 0x0000000000628e8b in dwarf2_get_section_info (objfile=0x31c0be0, sect=DWARF2_EH_FRAME, sectp=0x31ddf20, bufp=0x31ddf10, sizep=0x31ddf18) at dwarf2read.c:2634 #3 0x0000000000611616 in dwarf2_build_frame_info (objfile=0x31c0be0) at dwarf2-frame.c:2222 #4 0x0000000000610262 in dwarf2_frame_find_fde (pc=0x7ffd1fc37ad0, out_offset=0x0) at dwarf2-frame.c:1707 #5 0x000000000060f874 in dwarf2_frame_sniffer (self=0xa2c480 <dwarf2_frame_unwind>, this_frame=0x2fceb60, this_cache=0x2fceb78) at dwarf2-frame.c:1337 #6 0x00000000006913cc in frame_unwind_try_unwinder (this_frame=0x2fceb60, this_cache=0x2fceb78, unwinder=0xa2c480 <dwarf2_frame_unwind>) at frame-unwind.c:106 #7 0x0000000000691598 in frame_unwind_find_by_frame (this_frame=0x2fceb60, this_cache=0x2fceb78) at frame-unwind.c:164 #8 0x000000000068fe3c in get_frame_type (frame=0x2fceb60) at frame.c:2625 #9 0x000000000077404e in print_frame_info (frame=0x2fceb60, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1) at stack.c:795 #10 0x00000000007729b0 in print_stack_frame (frame=0x2fceb60, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1) at stack.c:177 #11 0x00000000006d3ee5 in print_stop_location (ws=0x7ffd1fc37db0) at infrun.c:8041 #12 0x00000000006d3f5b in print_stop_event (uiout=0x30f1900) at infrun.c:8058 ... >> I was also >> unable to figure out why the error message isn't displayed. The new >> reproducer shows this issue. I wasn't sure if setting *size or even >> descriptor->size was the right thing to do, but it seemed reasonable to >> me since the comment in gdb_bfd.h states that this function updates >> *size. There's currently only one caller for 'gdb_bfd_map_section', so >> I have no problem updating *size there if that is preferred. > > Actually it says "If successful, the section data is returned and *SIZE is set to the size of the section data;". And this is what I would generally expect from functions. Unless stated otherwise, the value of output parameters should be considered undefined if the function failed. So I would lean towards blaming the caller for not taking enough precautions. It trusts that gdb_bfd_map_section won't fail. > I'm not sure how the gdb_bfd_map_section caller can pre-determine that it will fail. It looks like there may be situations where gdb_bfd_map_section doesn't actually need to read the file, so that would mean that simply checking if the object file is readable before calling gdb_bfd_map_section might not be a good way to address this. Outside of that, I don't see what pre-conditions I can check to determine if gdb_bfd_map_section is going to fail in this case. Do you have any suggestions? To add a little more info, the segfault happens after trying to run 'next' in the debugger after this error is thrown. Here is a snippet of the stack: #0 0x000000000083b3e1 in bfd_getl32 (p=0x0) at libbfd.c:557 #1 0x000000000060fb3e in read_initial_length (abfd=0x31805e0, buf=0x0, bytes_read_ptr=0x7ffd1fc3797c) at dwarf2-frame.c:1482 #2 0x0000000000610592 in decode_frame_entry_1 (unit=0x31ddf40, start=0x0, eh_frame_p=1, cie_table=0x7ffd1fc37b00, fde_table=0x7ffd1fc37af0, entry_type=EH_CIE_OR_FDE_TYPE_ID) at dwarf2-frame.c:1792 #3 0x00000000006111b5 in decode_frame_entry (unit=0x31ddf40, start=0x0, eh_frame_p=1, cie_table=0x7ffd1fc37b00, fde_table=0x7ffd1fc37af0, entry_type=EH_CIE_OR_FDE_TYPE_ID) at dwarf2-frame.c:2090 #4 0x00000000006116d1 in dwarf2_build_frame_info (objfile=0x31c0be0) at dwarf2-frame.c:2247 #5 0x0000000000610262 in dwarf2_frame_find_fde (pc=0x7ffd1fc37d30, out_offset=0x2fcec58) at dwarf2-frame.c:1707 #6 0x000000000060ead3 in dwarf2_frame_cache (this_frame=0x2fceb60, this_cache=0x2fceb78) at dwarf2-frame.c:1000 #7 0x000000000060f2d7 in dwarf2_frame_this_id (this_frame=0x2fceb60, this_cache=0x2fceb78, this_id=0x2fcebc0) at dwarf2-frame.c:1198 #8 0x000000000068baab in compute_frame_id (fi=0x2fceb60) at frame.c:505 #9 0x000000000068bbf7 in get_frame_id (fi=0x2fceb60) at frame.c:537 #10 0x00000000006bee4e in step_command_fsm_prepare (sm=0x3175e10, skip_subroutines=1, single_inst=0, count=1, thread=0x3162ac0) at infcmd.c:1056 #11 0x00000000006bef73 in step_1 (skip_subroutines=1, single_inst=0, count_string=0x0) at infcmd.c:1096 #12 0x00000000006bed3f in next_command (count_string=0x0, from_tty=1) at infcmd.c:969 ... dwarf2_build_frame_info first checks if unit->dwarf_frame_size is non-zero (it is), then it calls decode_frame_entry on unit->dwarf_frame_buffer. unit->dwarf_frame_buffer is null, which triggers the segfault. Adding checks in dwarf2_build_frame_info to check that dwarf_frame_buffer is non-zero (which needs to be done in two places) also prevents the crash. However, this doesn't address the issue that the stack frame info isn't printed when the initial breakpoint is hit (because gdb_bfd_map_section throws an error). It would be nice if this could be gracefully handled so that the current frame and source code line are printed instead of just dumping you back to a (gdb) prompt without knowing where you are. Since my patch changes the error to a warning, and updates the size pointer, it fixes both of these issues. I can imagine two alternatives: - Change the error to a warning, and make the caller of gdb_bfd_map_section check if the returned pointer is NULL, and if so set *size to 0. This requires the ugly-ish early return that I had to add in gdb_bfd_map_section. - Add a TRY/CATCH around dwarf2_read_section in dwarf2_get_section_info, that sets *sectp, *bufp, and *sizep to NULL. I'm not sure if I should then re-throw the exception. Thanks, Mike
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 29080b8..229f5ae 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -705,9 +705,15 @@ gdb_bfd_map_section (asection *sectp, bfd_size_type *size) data = NULL; if (!bfd_get_full_section_contents (abfd, sectp, &data)) - error (_("Can't read data for section '%s' in file '%s'"), - bfd_get_section_name (abfd, sectp), - bfd_get_filename (abfd)); + { + warning (_("Can't read data for section '%s' in file '%s'"), + bfd_get_section_name (abfd, sectp), + bfd_get_filename (abfd)); + /* Section is invalid -- set size to 0 and return NULL */ + descriptor->size = 0; + *size = descriptor->size; + return (const gdb_byte *) NULL; + } descriptor->data = data; done: