From patchwork Tue Mar 25 15:22:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 272 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (caibbdcaabja.dreamhost.com [208.113.200.190]) by wilcox.dreamhost.com (Postfix) with ESMTP id 06EFE3600A9 for ; Tue, 25 Mar 2014 08:23:05 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14314964) id B122761EC0A1B; Tue, 25 Mar 2014 08:23:05 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id 76C6D61F9E41B for ; Tue, 25 Mar 2014 08:23:05 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; q=dns; s= default; b=aHXd+95XRbsqMuPs5dRB2zPicWX2IIkgz8d+FvP2tUlK2zf1mMzSf YZpiuVhYfx//RTyJI6LCba0OKcRZH+yMl8aWEElP866Y0drbKJQKNdA9CwtLlj1p 2iekOwaggYVwlmxofznms+Nge6sPuPDuS0/ccSSUGKVGxeGRxxa6fg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; s=default; bh=6GYx3L9AP7DOWQ+4hQV9N8aNEj0=; b=OQV5FpkpqkFVhehAIY/YwGks1Kz4 PRZ/YkPR9ubfUjzL16zEMBk2T6/ZDsoKdxqg9KI5YMtmiGQ25fJQ+sxWnthASsMW aFZwObJbt2RiVeQ3Pc8lWD6Bk17DEOwde6E4TILVBJBuy8tyAFbobW5hzS8bZ/Oe YD8Zq/pgMliGl8Y= Received: (qmail 8209 invoked by alias); 25 Mar 2014 15:23:02 -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 8194 invoked by uid 89); 25 Mar 2014 15:23:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 25 Mar 2014 15:23:00 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D6B91116799 for ; Tue, 25 Mar 2014 11:22:57 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id rgV3u-Wxsc-G for ; Tue, 25 Mar 2014 11:22:57 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 9FE22116676 for ; Tue, 25 Mar 2014 11:22:57 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 53B7FE07DC; Tue, 25 Mar 2014 08:22:58 -0700 (PDT) From: Joel Brobecker To: gdb-patches@sourceware.org Subject: FYI: [varobj] false type-changed status for reference to Ada array Date: Tue, 25 Mar 2014 08:22:51 -0700 Message-Id: <1395760971-29285-1-git-send-email-brobecker@adacore.com> X-DH-Original-To: gdb@patchwork.siddhesh.in Hello, Given the following variable... BT : Bounded := New_Bounded (Low => 1, High => 3); ... where type Bounded is defined as a simple unconstrained array: type Bounded is array (Integer range <>) of Integer; Creating a varobj for that variable, and immediately asking for varobj updates, GDB says that our varobj changed types! (gdb) -var-create bt * bt ^done,name="bt",numchild="3",value="[3]",type=" array (1 .. 3) of integer",has_more="0" (gdb) -var-update 1 * ^done,changelist=[{name="bt",value="[3]",in_scope="true",type_changed="true",new_type=" array (1 .. 3) of integer",new_num_children="3",has_more="0"}] The expected output for the -var-update command is, in this case: (gdb) -var-update 1 * ^done,changelist=[] The problem occurs because the ada-varobj module does not handle references, and while the references gets stripped when the varobj gets created, it doesn't when computing varobj updates. More specifically, when creating the varobj, varobj_create creates a new value which is a reference to a TYPE_CODE_ARRAY. It then calls install_new_value which calls coerce_ref with the following comment: /* We are not interested in the address of references, and given that in C++ a reference is not rebindable, it cannot meaningfully change. So, get hold of the real value. */ if (value) value = coerce_ref (value); This leaves the varobj's type component still a ref, while the varobj's value is now our array, without the ref. This explains why the "value" field in the varobj indicates an array with 3 elements "[3]" while the "type" field shows a ref to an array. Generally speaking, most users have said that showing the ref was a useful piece of information, so this patch is not touching this part. Next, when the user issues the -var-update request, varobj_update calls value_of_root to compute the varobj's new value as well as determine whether the value's type has changed or not. What happens in a nutshell is that it calls value_of_root_1 (which re-evaluates the expression and returns the corresponding new value), finds that the new value is not NULL, and thus asks whether it has mutated: else if (varobj_value_has_mutated (var, value, value_type (value))) This then indirectly delegates the determination to the language-specific callback, which fails, because it does not handle references. This patch fixes the issue by adjusting varobj_value_has_mutated to expect references, and strip them when seen. This allows the various language-specific implementations to remain unaware of references. gdb/ChangeLog: * varobj.c (varobj_value_has_mutated): If NEW_VALUE is a reference, strip the reference layer before calling the lang_ops value_has_mutated callback. gdb/testsuite/ChangeLog: * gdb.ada/mi_dyn_arr: New testcase. Tested on x86_64-linux. I will commit in a couple of days unless there are comments/objections. Thank you, diff --git a/gdb/testsuite/gdb.ada/mi_dyn_arr.exp b/gdb/testsuite/gdb.ada/mi_dyn_arr.exp new file mode 100644 index 0000000..434c21d --- /dev/null +++ b/gdb/testsuite/gdb.ada/mi_dyn_arr.exp @@ -0,0 +1,52 @@ +# Copyright 2014 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +load_lib "ada.exp" + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } { + return -1 +} + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +gdb_exit +if [mi_gdb_start] { + continue +} + +mi_delete_breakpoints +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load ${binfile} + +if ![mi_run_to_main] then { + fail "Cannot run to main, testcase aborted" + return 0 +} + +set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb] +mi_continue_to_line \ + "foo.adb:$bp_location" \ + "stop at start of main Ada procedure" + +mi_gdb_test "-var-create bt * bt" \ + "\\^done,name=\"bt\",numchild=\"3\",.*" \ + "Create bt varobj" + +mi_gdb_test "-var-update 1 *" \ + "\\^done,changelist=\\\[\\\]" \ + "list ggg1's children" diff --git a/gdb/testsuite/gdb.ada/mi_dyn_arr/foo.adb b/gdb/testsuite/gdb.ada/mi_dyn_arr/foo.adb new file mode 100644 index 0000000..4a9f358 --- /dev/null +++ b/gdb/testsuite/gdb.ada/mi_dyn_arr/foo.adb @@ -0,0 +1,24 @@ +-- Copyright 2014 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see . + +with Pck; use Pck; +procedure Foo is + -- The goal here is to have an array whose bounds are not + -- known at compile time. + BT : Bounded := New_Bounded (Low => 1, High => 3); +begin + Do_Nothing (BT'Address); -- STOP +end Foo; + diff --git a/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.adb b/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.adb new file mode 100644 index 0000000..c77c31e --- /dev/null +++ b/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.adb @@ -0,0 +1,30 @@ +-- Copyright 2014 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see . + +package body Pck is + function New_Bounded (Low, High : Integer) return Bounded is + Result : Bounded (Low .. High); + begin + for J in Low .. High loop + Result (J) := J; + end loop; + return Result; + end New_Bounded; + + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.ads b/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.ads new file mode 100644 index 0000000..2519fdd --- /dev/null +++ b/gdb/testsuite/gdb.ada/mi_dyn_arr/pck.ads @@ -0,0 +1,21 @@ +-- Copyright 2014 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see . + +with System; +package Pck is + type Bounded is array (Integer range <>) of Integer; + function New_Bounded (Low, High : Integer) return Bounded; + procedure Do_Nothing (A : System.Address); +end Pck; diff --git a/gdb/varobj.c b/gdb/varobj.c index bbe4213..10ef5b7 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -1649,7 +1649,16 @@ varobj_value_has_mutated (struct varobj *var, struct value *new_value, return 0; if (var->root->lang_ops->value_has_mutated) - return var->root->lang_ops->value_has_mutated (var, new_value, new_type); + { + /* The varobj module, when installing new values, explicitly strips + references, saying that we're not interested in those addresses. + But detection of mutation happens before installing the new + value, so our value may be a reference that we need to strip + in order to remain consistent. */ + if (new_value != NULL) + new_value = coerce_ref (new_value); + return var->root->lang_ops->value_has_mutated (var, new_value, new_type); + } else return 0; }