From patchwork Tue Jun 17 17:26:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 1536 Received: (qmail 29913 invoked by alias); 17 Jun 2014 17:26:23 -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 29903 invoked by uid 89); 17 Jun 2014 17:26:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS 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 ESMTP; Tue, 17 Jun 2014 17:26:21 +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 (8.14.4/8.14.4) with ESMTP id s5HHQG8o013782 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 17 Jun 2014 13:26:17 -0400 Received: from host2.jankratochvil.net (ovpn-116-65.ams2.redhat.com [10.36.116.65]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5HHQCgY024130 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Tue, 17 Jun 2014 13:26:14 -0400 Date: Tue, 17 Jun 2014 19:26:11 +0200 From: Jan Kratochvil To: Keven Boell Cc: Keven Boell , gdb-patches@sourceware.org, sanimir.agovic@intel.com Subject: Re: [PATCH 04/23] vla: make dynamic fortran arrays functional. Message-ID: <20140617172611.GA5176@host2.jankratochvil.net> References: <1401861266-6240-1-git-send-email-keven.boell@intel.com> <1401861266-6240-5-git-send-email-keven.boell@intel.com> <20140616210229.GB20395@host2.jankratochvil.net> <53A04723.8050802@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <53A04723.8050802@linux.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes On Tue, 17 Jun 2014 15:48:19 +0200, Keven Boell wrote: > Am 16.06.2014 23:02, schrieb Jan Kratochvil: > > On Wed, 04 Jun 2014 07:54:07 +0200, Keven Boell wrote: > >> diff --git a/gdb/valarith.c b/gdb/valarith.c > >> index 8e863e3..bddb9db 100644 > >> --- a/gdb/valarith.c > >> +++ b/gdb/valarith.c > >> @@ -200,7 +200,14 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound) > >> > >> if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type) > >> && elt_offs >= TYPE_LENGTH (array_type))) > >> - error (_("no such vector element")); > >> + { > >> + if (TYPE_NOT_ASSOCIATED (array_type)) > >> + error (_("no such vector element because not associated")); > >> + else if (TYPE_NOT_ALLOCATED (array_type)) > >> + error (_("no such vector element because not allocated")); > >> + else > >> + error (_("no such vector element")); > >> + } > >> > >> if (VALUE_LVAL (array) == lval_memory && value_lazy (array)) > >> v = allocate_value_lazy (elt_type); > > > > I find here the patch is incomplete. Earlier this function has: > > unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound); > > > > and in some cases - specifically with the 64-bit inferior objects patch, the > > *bitpos* patches from: > > http://pkgs.fedoraproject.org/cgit/gdb.git/tree/ > > one occasionally gets for TYPE_NOT_ALLOCATED inferior variables: > > Value out of range. > > instead of the more correct: > > no such vector element because not allocated > > > > Because for TYPE_NOT_ALLOCATED inferior variable the LOWERBOUND value read > > from the inferior is bogus and 'index - lowerbound' will not fit in 'int'. > > This change just adds some more information for the already existing > "no such vector element" message. To me the change you are describing > sounds more like a generic fix for this function or am I wrong? I am not sure if the fix I suggest belongs to this patch series part but it definitely belongs to this patch series. Beforehand if this function value_subscripted_rvalue was called the parameter 'lowerbound' had to be always valid. But by introducting the TYPE_NOT_ALLOCATED functionality 'lowerbound' can be passed invalid and so GDB should not touch it if the array/type is TYPE_NOT_ALLOCATED. Otherwise this patchset has a regression against my former VLA patchset on unallocated Fortran arrays where instead of expected no such vector element because not allocated it sometimes - depending on what is read from uninitialized inferior memory, therefore not always - prints instead: Value out of range. I had to apply the attached fix. I am not completely sure this is the right fix - IMO when the array is not allocated GDB should not try to read the array bound(s) from inferior memory as there is an undefined value that time. Jan --- ./gdb/valarith.c 2014-06-17 19:05:09.286336188 +0200 +++ ./gdb/valarith.c 2014-06-17 19:14:46.752072463 +0200 @@ -195,10 +195,19 @@ value_subscripted_rvalue (struct value * struct type *array_type = check_typedef (value_type (array)); struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type)); unsigned int elt_size = TYPE_LENGTH (elt_type); - unsigned int elt_offs = longest_to_int (index - lowerbound); + unsigned int elt_offs; LONGEST elt_stride = TYPE_BYTE_STRIDE (TYPE_INDEX_TYPE (array_type)); struct value *v; + if (TYPE_NOT_ASSOCIATED (array_type)) + error (_("no such vector element because not associated")); + if (TYPE_NOT_ALLOCATED (array_type)) + error (_("no such vector element because not allocated")); + + if (index < lowerbound || (int) (index - lowerbound) != index - lowerbound) + error (_("no such vector element")); + elt_offs = longest_to_int (index - lowerbound); + if (elt_stride > 0) elt_offs *= elt_stride; else if (elt_stride < 0) @@ -210,16 +219,9 @@ value_subscripted_rvalue (struct value * else elt_offs *= elt_size; - if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type) - && elt_offs >= TYPE_LENGTH (array_type))) - { - if (TYPE_NOT_ASSOCIATED (array_type)) - error (_("no such vector element because not associated")); - else if (TYPE_NOT_ALLOCATED (array_type)) - error (_("no such vector element because not allocated")); - else - error (_("no such vector element")); - } + if (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type) + && elt_offs >= TYPE_LENGTH (array_type)) + error (_("no such vector element")); if (VALUE_LVAL (array) == lval_memory && value_lazy (array)) v = allocate_value_lazy (elt_type);