From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94752 invoked by alias); 13 Apr 2016 15:16:05 -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 94560 invoked by uid 89); 13 Apr 2016 15:16:03 -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 autolearn=ham version=3.3.2 spammy=remotely 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 13 Apr 2016 15:15:58 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1aqMWK-0006Cd-9t from Luis_Gustavo@mentor.com ; Wed, 13 Apr 2016 08:15:56 -0700 Received: from [172.30.5.211] (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Wed, 13 Apr 2016 08:15:55 -0700 Reply-To: Luis Machado Subject: Re: [PATCH v2 1/2] Debugging without a binary (regression) References: <1460489060-17307-1-git-send-email-lgustavo@codesourcery.com> <1460489060-17307-2-git-send-email-lgustavo@codesourcery.com> <570E5F05.3020100@redhat.com> To: Pedro Alves , From: Luis Machado Message-ID: <570E62AA.1060808@codesourcery.com> Date: Wed, 13 Apr 2016 15:16:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <570E5F05.3020100@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00286.txt.bz2 On 04/13/2016 10:00 AM, Pedro Alves wrote: > On 04/12/2016 08:24 PM, Luis Machado wrote: >> When we attempt to debug a process using GDBserver in standard remote mode >> without a symbol file on GDB's end, we may run into an issue where GDB cuts >> the connection attempt short due to an error. The error is caused by not >> being able to open a symbol file, like so: >> >> -- >> >> (gdb) set sysroot >> (gdb) tar rem :2345 >> Remote debugging using :2345 >> /proc/23769/exe: Permission denied. >> (gdb) i r >> The program has no registers now. >> (gdb) >> >> It should've been like this: >> >> (gdb) set sysroot >> (gdb) tar rem :2345 >> Remote debugging using :2345 >> 0xf7ddb2d0 in ?? () > > The warning output is missing here, right? > It is, because i did not update it after the patch. :-) Should be fixed in v3. >> (gdb) i r >> eax 0x0 0 >> ecx 0x0 0 >> edx 0x0 0 >> ebx 0x0 0 >> esp 0xffffdfa0 0xffffdfa0 >> ebp 0x0 0x0 >> esi 0x0 0 >> edi 0x0 0 >> eip 0xf7ddb2d0 0xf7ddb2d0 >> eflags 0x200 [ IF ] >> cs 0x33 51 >> ss 0x2b 43 >> ds 0x0 0 >> es 0x0 0 >> fs 0x0 0 >> gs 0x0 0 >> (gdb) >> > >> void >> @@ -142,6 +161,7 @@ exec_file_locate_attach (int pid, int from_tty) >> { >> char *exec_file, *full_exec_path = NULL; >> struct cleanup *old_chain; >> + struct gdb_exception prev_err; >> >> /* Do nothing if we already have an executable filename. */ >> exec_file = (char *) get_exec_file (0); >> @@ -182,9 +202,46 @@ exec_file_locate_attach (int pid, int from_tty) >> >> old_chain = make_cleanup (xfree, full_exec_path); >> >> - exec_file_attach (full_exec_path, from_tty); >> - symbol_file_add_main (full_exec_path, from_tty); >> + /* exec_file_attach and symbol_file_add_main may throw an error if the file >> + cannot be opened either locally or remotely. >> + >> + This happens for example, when the file is first found in the local >> + sysroot (above), and then disappears (a TOCTOU race), or when it doesn't >> + exist in the target filesystem, or when the file does exist, but >> + is not readable. >> + >> + Even without a symbol file, the remote-based debugging session should >> + continue normally instead of ending abruptly. Hence we catch thrown >> + errors/exceptions in the following code. */ >> + TRY >> + { >> + exec_file_attach (full_exec_path, from_tty); >> + } >> + CATCH (err, RETURN_MASK_ERROR) >> + { >> + if (err.message != NULL) >> + warning ("%s", err.message); >> + >> + prev_err = err; >> + >> + /* Save message so it doesn't get trashed by the catch problem > > "the catch problem below" should be either the "the caught > problem below", or "the catch below", I think. > Yeah, i got things mixed up when coming up with it. "the catch below" is what i wanted to say. >> + below. */ >> + prev_err.message = xstrdup (err.message); >> + } >> + END_CATCH >> + >> + TRY >> + { >> + symbol_file_add_main (full_exec_path, from_tty); >> + } >> + CATCH (err, RETURN_MASK_ERROR) >> + { >> + if (!exception_print_same (prev_err, err)) >> + warning ("%s", err.message); >> + } >> + END_CATCH >> >> + xfree ((void *) prev_err.message); > > This will reference an uninitialized prev_err.message if the > first TRY doesn't throw. Initialize it with: > > struct gdb_exception prev_err = exception_none; > > Done. > Also, it will leak prev_err.message on Ctrl-C/QUIT, which is > not caught by RETURN_MASK_ERROR. prev_err.message needs to be > freed with a cleanup. Say: > > struct gdb_exception prev_err = exception_none; > > old_chain = make_cleanup (xfree, full_exec_path); > > make_cleanup (free_current_contents, &prev_err.message); > > TRY > { > ... > } > CATCH (err, RETURN_MASK_ERROR) > { > if (err.message != NULL) > warning ("%s", err.message); > > prev_err = err; > prev_err.message = xstrdup (err.message); > } > ... > > do_cleanups (old_chain); > Thanks. I've fixed these. I'll get v3 out once the testcase gets reviewed. > Thanks, > Pedro Alves >