From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9224 invoked by alias); 24 Apr 2013 15:22:26 -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 9207 invoked by uid 89); 24 Apr 2013 15:22:26 -0000 X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-oa0-f45.google.com (HELO mail-oa0-f45.google.com) (209.85.219.45) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 24 Apr 2013 15:22:25 +0000 Received: by mail-oa0-f45.google.com with SMTP id o17so1831170oag.32 for ; Wed, 24 Apr 2013 08:22:24 -0700 (PDT) X-Received: by 10.60.84.33 with SMTP id v1mr6206691oey.36.1366816944282; Wed, 24 Apr 2013 08:22:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.60.13.133 with HTTP; Wed, 24 Apr 2013 08:21:44 -0700 (PDT) In-Reply-To: <87haiwngpr.fsf@fleche.redhat.com> References: <87haiwngpr.fsf@fleche.redhat.com> From: Hui Zhu Date: Wed, 24 Apr 2013 19:13:00 -0000 Message-ID: Subject: Re: [PATCH/7.6] Fix wrong release (maybe crash GDB) in build_target_command_list To: Tom Tromey , Yao Qi Cc: gdb-patches ml , Joel Brobecker Content-Type: text/plain; charset=ISO-8859-1 X-SW-Source: 2013-04/txt/msg00744.txt.bz2 On Wed, Apr 24, 2013 at 10:30 PM, Tom Tromey wrote: >>>>>> "Hui" == Hui Zhu writes: > > Hui> 2013-04-24 Hui Zhu > > Hui> * breakpoint.c (build_target_command_list): Change loc->cond_bytecode > Hui> to loc->cmd_bytecode. > > Is it possible to make a test case for this? I tried but looks free conditions cannot crash GDB. :( > > The patch looks good. > > I am curious about this code in build_target_command_list: > > aexpr = parse_cmd_to_aexpr (bl->address, > loc->owner->extra_string); > loc->cmd_bytecode = aexpr; > > if (!aexpr) > continue; > > The "continue" seems to mean that null_command_or_parse_error will not > be set in the "parse error" case. I think it is right, because even if one of breakpoint loc doesn't have commands. GDB still need send other commands to target. > > Also, parse_cmd_to_aexpr calls 'error' in a few spots but then in > another spot is careful not to. This seems somewhat odd. This function looks have something still not right. I am just working on 15180 to make it better. On 04/24/2013 09:29 PM, Hui Zhu wrote: - if (!loc->cond_bytecode) + if (!loc->cmd_bytecode) Nit: "if (loc->cmd_bytecode == NULL)" is better. Fixed. Thanks, Hui > > Tom 2013-04-24 Hui Zhu * breakpoint.c (build_target_command_list): Change loc->cond_bytecode to loc->cmd_bytecode. --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2343,11 +2343,11 @@ build_target_command_list (struct bp_loc { /* Only go as far as the first NULL bytecode is located. */ - if (!loc->cond_bytecode) + if (loc->cmd_bytecode == NULL) return; - free_agent_expr (loc->cond_bytecode); - loc->cond_bytecode = NULL; + free_agent_expr (loc->cmd_bytecode); + loc->cmd_bytecode = NULL; } } }