From patchwork Fri Mar 27 19:22:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 5854 Received: (qmail 53675 invoked by alias); 27 Mar 2015 19:22:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 53665 invoked by uid 89); 27 Mar 2015 19:22:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 27 Mar 2015 19:22:36 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 2F899350044; Fri, 27 Mar 2015 19:22:35 +0000 (UTC) Received: from host1.jankratochvil.net (ovpn-116-21.ams2.redhat.com [10.36.116.21]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2RJMVoY019805 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 27 Mar 2015 15:22:34 -0400 Date: Fri, 27 Mar 2015 20:22:31 +0100 From: Jan Kratochvil To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Revert: [obv] Code cleanup: Move print_command_1 expr variable scope Message-ID: <20150327192231.GA21247@host1.jankratochvil.net> References: <20150326174630.GA3349@host1.jankratochvil.net> <5515AC41.1040907@ericsson.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5515AC41.1040907@ericsson.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes Hi Simon, On Fri, 27 Mar 2015 20:15:13 +0100, Simon Marchi wrote: > I think this patch is wrong. Starting with that commit (f30d5c7), > some tests (e.g. mi-break.exp) started to fail for me, because > of gdb segfaulting. Backtrace here: http://paste.ubuntu.com/10690836/ > > The address of expr is passed to the cleanup. When the cleanup is ran, > expr is no longer in scope, so what is at that address is probably not > safe to use anymore. That's my guess. yes, you are sure right, I have reverted it now. Sorry I made that commit somehow automatically, not expecting it may have any side effects. Thanks, Jan Simon Marchi: I think this patch is wrong. Starting with that commit (f30d5c7), some tests (e.g. mi-break.exp) started to fail for me, because of gdb segfaulting. The address of expr is passed to the cleanup. When the cleanup is ran, expr is no longer in scope, so what is at that address is probably not safe to use anymore. That's my guess. gdb/ChangeLog 2015-03-27 Jan Kratochvil Revert: 2015-03-26 Jan Kratochvil Code cleanup. * printcmd.c (print_command_1): Move expr variable scope. --- gdb/ChangeLog | 7 +++++++ gdb/printcmd.c | 3 +-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 72940b0..6c6b94e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2015-03-27 Jan Kratochvil + + Revert: + 2015-03-26 Jan Kratochvil + Code cleanup. + * printcmd.c (print_command_1): Move expr variable scope. + 2015-03-27 Joel Brobecker * dtrace-probe.c (dtrace_process_dof_probe): Initialize expr to NULL. diff --git a/gdb/printcmd.c b/gdb/printcmd.c index a1451f8..deb501a 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -946,6 +946,7 @@ validate_format (struct format_data fmt, const char *cmdname) static void print_command_1 (const char *exp, int voidprint) { + struct expression *expr; struct cleanup *old_chain = make_cleanup (null_cleanup, NULL); char format = 0; struct value *val; @@ -968,8 +969,6 @@ print_command_1 (const char *exp, int voidprint) if (exp && *exp) { - struct expression *expr; - expr = parse_expression (exp); make_cleanup (free_current_contents, &expr); val = evaluate_expression (expr);