From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35327 invoked by alias); 18 Oct 2016 18:11:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 35292 invoked by uid 89); 18 Oct 2016 18:11:46 -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_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=userfriendly, user-friendly, formerly, Comments X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Oct 2016 18:11:36 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1bwYrP-0002vB-1E from Luis_Gustavo@mentor.com for gdb-patches@sourceware.org; Tue, 18 Oct 2016 11:11:35 -0700 Received: from [134.86.127.136] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 18 Oct 2016 11:11:32 -0700 Subject: Re: [rfc] PR 20569, segv in follow_exec References: <57F6D57D.8070603@codesourcery.com> To: Sandra Loosemore , Reply-To: Luis Machado From: Luis Machado Message-ID: <50f4c7d8-44e3-4351-0b54-9cbaef64717a@codesourcery.com> Date: Tue, 18 Oct 2016 18:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <57F6D57D.8070603@codesourcery.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-01.mgc.mentorg.com (147.34.90.201) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00519.txt.bz2 On 10/06/2016 05:51 PM, Sandra Loosemore wrote: > As I noted in PR20569, several exec-related tests cause GDB to segv when > sysroot translation fails on the executable pathname reported by > gdbserver. The immediate cause of the segv is that follow_exec is > passing a NULL argument (the result of exec_file_find) to strlen, but as > I looked at the code in more detail it seemed like follow_exec simply > isn't prepared for the case where sysroot translation fails to locate > the new executable, and there is no obvious recovery strategy. > > I thought I could copy logic from the other caller of exec_file_find, > exec_file_locate_attach, but I think it's doing the wrong thing there as > well. Plus, from reading the code I found other bugs in both callers of > exec_file_find due to confusion between host and target pathnames. > > The attached patch attempts to fix all the bugs. In terms of the > testcases that were formerly segv'ing, GDB now prints a warning but > continues execution of the new program, so that the tests now mostly > FAIL instead. You could argue the FAILs are due to a legitimate problem > with the test environment setting up the sysroot translation > incorrectly, but I'm not sure continuing execution rather than leaving > the target stopped is the most user-friendly fallback behavior, either. > E.g. the GDB manual suggests that you can set a breakpoint on main and > GDB will stop on main of the newly exec'ed program, too, but it can't do > that if it can't find the symbol information, and there's no way for the > user to specify the executable file to GDB explicitly before it resumes > execution. But, seemed better not to complicate this > already-too-complicated patch further by trying to address that in the > first cut. > > Comments? Suggestions? Etc. > > -Sandra > > > gdb-segv2.log > > > 2016-10-06 Sandra Loosemore > > PR gdb/20569 > gdb/ > * exceptions.c (exception_print_same): Moved here from exec.c. > Fixed message comparison. > * exceptions.h (exception_print_same): Declare. > * exec_file_locate_attach (exception_print_same): Delete copy here. > (exec_file_locate_attach): Rename exec_file and full_exec_path > variables to avoid confusion between target and host pathnames. > Move pathname processing logic to exec_file_find. Do not return > early if pathname lookup fails; guard symbol_file_add_main call > instead. > * infrun.c (follow_exec): Split and rename execd_pathname variable > to avoid confusion between target and host pathnames. Replace > brokenpathname copy with cleanup to free malloc'ed string. Warn > if pathname lookup fails. Pass target pathname to > target_follow_exec, not hostpathname. Borrow exception-handling > logic from exec_file_locate_attach. > * solib.c (exec_file_find): Incorporate fallback logic for relative > pathnames formerly in exec_file_locate_attach. > > > gdb-segv2.patch > > I went through the patch and, although this code as a whole is a bit on the convoluted side, it looks reasonable to me. Segfaults are not supposed to happen, so allowing the session to continue is a good thing IMO. Sounds like a good candidate for master and even stable branches.