From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 74268 invoked by alias); 6 Dec 2016 10:15:43 -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 74243 invoked by uid 89); 6 Dec 2016 10:15:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=Hx-languages-length:2999, reveals, dispose, watch X-HELO: mail-wm0-f65.google.com Received: from mail-wm0-f65.google.com (HELO mail-wm0-f65.google.com) (74.125.82.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Dec 2016 10:15:31 +0000 Received: by mail-wm0-f65.google.com with SMTP id m203so20443596wma.3 for ; Tue, 06 Dec 2016 02:15:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=4nIJX5XZl8bxTPC9Wz36UeQFT3wLWYx2G98kfpeybAo=; b=NZ2wtmIOhZG/AltjrkowJdRC6tAfoxmBMeJ8ZasI9/vqBukVOXX29uhvpMtkzN59UT 66bmmhhOwqvSS0k/Ie5WGkgOrqA8rLNwmOLhEXqLppje7v9iKsjYPvYMRkiks+5NC2CU fqNGhI1ojEmK6wXY525O0HMjppa1jLtzh8ySFTVz+2w0fWxaK7aMLTQo8wBxPqXKfCDB N1yO1wpj/yAr8FcHmk3bvpUUceKtPrcfUyCleczO/JUfkyp8ucv4L1ktU7np5isxOaVZ xtoKrxqHhvEsnCqf4QbnioMeb/7EX4CfSU68buVWe5FYG/g38FolQUzt3d5hRviva2SB mTDg== X-Gm-Message-State: AKaTC02IprEtZrIFrN35O/IMvzMWl7IA9WoV1Qjyr+2re/3qIflXDOcXJ/yJS1swAwNUzg== X-Received: by 10.28.136.80 with SMTP id k77mr1856150wmd.57.1481019329571; Tue, 06 Dec 2016 02:15:29 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id bj1sm24979524wjc.17.2016.12.06.02.15.27 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 06 Dec 2016 02:15:29 -0800 (PST) Date: Tue, 06 Dec 2016 10:15:00 -0000 From: Yao Qi To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Assert on lval_register Message-ID: <20161206101513.GA28789@E107787-LIN> References: <1480517757-11733-1-git-send-email-yao.qi@linaro.org> <20161130211637.BA94710BCB5@oc8523832656.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20161130211637.BA94710BCB5@oc8523832656.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00149.txt.bz2 On 16-11-30 22:16:37, Ulrich Weigand wrote: > > This also reveals a design issue in frame_register_unwind, that is > > arguments addrp and realnump are mutually exclusive, we either use > > addrp (for lval_memory), or use realnump (for lval_register). This > > can be done in a separate patch. > > I think we should simply get rid of frame_register_unwind. Callers > should be changed to use frame_unwind_register_value directly, and > just operate on the value. > Yeah, that is what I was trying to do, but we should be careful on the value management in the end of frame_register_unwind, /* Dispose of the new value. This prevents watchpoints from trying to watch the saved frame pointer. */ release_value (value); value_free (value); > > *lvalp = VALUE_LVAL (value); > > *addrp = value_address (value); > > - *realnump = VALUE_REGNUM (value); > > + if (*lvalp == lval_register) > > + *realnump = VALUE_REGNUM (value); > > But as long as the above change is not done yet, maybe we ought to > at least provide a defined value (e.g. -1), to avoid callers maybe > making use of uninitialized variables? > OK, how about the patch below? -- Yao (齐尧) >From 581e1b80ea631d5bcfb5fc64b07a36fd71f5d55e Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Thu, 24 Nov 2016 09:08:00 +0000 Subject: [PATCH] Assert on lval_register This patch adds asserts where the value's lval must be lval_register. This triggers an error in frame_register_unwind because VALUE_REGNUM is used but value's lval is not lval_register. This also reveals a design issue in frame_register_unwind, that is arguments addrp and realnump are mutually exclusive, we either use addrp (for lval_memory), or use realnump (for lval_register). This can be done in a separate patch. gdb: 2016-12-06 Yao Qi * frame.c (frame_register_unwind): Set *realnump if *lvalp is lval_register. * value.c (deprecated_value_next_frame_id_hack): Assert value->lval is lval_register. (deprecated_value_regnum_hack): Likewise. diff --git a/gdb/frame.c b/gdb/frame.c index 5414cb3..00001bc 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1107,7 +1107,10 @@ frame_register_unwind (struct frame_info *frame, int regnum, *unavailablep = !value_entirely_available (value); *lvalp = VALUE_LVAL (value); *addrp = value_address (value); - *realnump = VALUE_REGNUM (value); + if (*lvalp == lval_register) + *realnump = VALUE_REGNUM (value); + else + *realnump = -1; if (bufferp) { diff --git a/gdb/value.c b/gdb/value.c index cc291cf..022900f 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -1576,12 +1576,14 @@ deprecated_value_internalvar_hack (struct value *value) struct frame_id * deprecated_value_next_frame_id_hack (struct value *value) { + gdb_assert (value->lval == lval_register); return &value->location.reg.next_frame_id; } int * deprecated_value_regnum_hack (struct value *value) { + gdb_assert (value->lval == lval_register); return &value->location.reg.regnum; }