From patchwork Sat Jan 18 23:04:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 37438 Received: (qmail 4579 invoked by alias); 18 Jan 2020 23:04:40 -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 4567 invoked by uid 89); 18 Jan 2020 23:04:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=gnutoolchaingerritosciio, 124118, gnutoolchain-gerrit.osci.io, UD:gnutoolchain-gerrit.osci.io X-HELO: mail-wm1-f65.google.com Received: from mail-wm1-f65.google.com (HELO mail-wm1-f65.google.com) (209.85.128.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 18 Jan 2020 23:04:37 +0000 Received: by mail-wm1-f65.google.com with SMTP id d73so10821378wmd.1 for ; Sat, 18 Jan 2020 15:04:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=yr39w9Js61niIVwVpgm/qawLLksBPJZ6Cs+8ssnE98k=; b=ZScxpIs0nPCckyoJcSPH0uSJcMgXrO8h11uhxvk66onJV7nJvCo48c4Iqx7dq5CTbQ bLSyhvm82V6Gx5/YbbwNXt1wnCCxpg3kjQSFg93Z1Q0YRqSyxZT1QRcTk+bS1MXoAbL6 cc6lw+hgKWYoA9RZWOozMixOnTCv4uz4AUAI8rEGNVvuMOmoEVOfEkpnHs5D1hvxgosd q14qGuTQPuNYcLtjE8IKD5AQFY3XTvFT6zmfTmo3iWIIj4sC9upptZvK3fkfODIY9N8O 3PKjwVXK1cUtL8rCPJXjovYcRx/CmyycVyL7Q4l4ixAu92560E3AvH/oMImrvoNNDR6c H4ww== Return-Path: Received: from localhost (host86-191-239-73.range86-191.btcentralplus.com. [86.191.239.73]) by smtp.gmail.com with ESMTPSA id m10sm40780777wrx.19.2020.01.18.15.04.34 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 18 Jan 2020 15:04:34 -0800 (PST) Date: Sat, 18 Jan 2020 23:04:33 +0000 From: Andrew Burgess To: Sergio Durigan Junior Cc: gdb-patches@sourceware.org, Joel Brobecker Subject: Re: [PATCH] Add gdb.fortran/vla-stride.exp and report a bug (was: Re: [review] gdb/fortran: array stride support) Message-ID: <20200118230433.GC3865@embecosm.com> References: <87sgkivgx2.fsf_-_@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87sgkivgx2.fsf_-_@redhat.com> X-Fortune: The only perfect science is hind-sight. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Sergio Durigan Junior [2020-01-13 22:46:33 -0500]: > On Thursday, November 14 2019, Andrew Burgess wrote: > > > Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/627 > > ...................................................................... > > > > gdb/fortran: array stride support > > [...] > > Hey Andrew, > > I found a problem with this patch, and I'd like to know if you've > noticed this as well. I first encountered the problem while doing > downstream work on Fedora GDB for Fedora Rawhide; as you are probably > aware, we carry *a lot* of local Fortran VLA patches on Fedora GDB (if > you're not aware about this, feel free to get in touch with me and I'll > be more than happy to explain the situation to you). However, I am able > to reproduce the problem on upstream GDB as well. > > On Fedora GDB, we carry a testcase called gdb.fortran/vla-stride.exp. I'm > attaching it to this message. One of its tests fails with: > > (gdb) print pvla > Cannot access memory at address 0x426000 > FAIL: gdb.fortran/vla-stride.exp: print single-element Sergio, First, apologies for not replying sooner, I completely missed this mail. My bad! Thanks for the bug report. Yes I'm aware that Fedora carries some Fortran patches, its on my (ever growing) todo list that I should take a look at them one day. It kind-of sucks that my top of tree GDB is sometimes not as good as my slightly older distro-installed GDB! I put together a patch for this issue (see below) it passes your test case on my machine (with no other regressions), but it would be neat if you could confirm it resolves the issue for you. This particular case is about negative array strides, which in general (right now) wont work in GDB ... however, this particular case is special, it's a 1 element array with a negative array stride, that we can do, if we don't deliberately sabotage ourselves by treating the negative stride as unsigned. As for negative array strides in general, I'm working on this, but it's a big change and I don't know when it will be finished. Anyway, patch below. Feedback welcome. I'll push this in a week or so if I don't get any negative feedback. One question - I included your testcase in this patch, I just wanted to check that this is OK for upstream (w.r.t. copyright assignment, etc)? Thanks, Andrew --- commit 6cedcaebf82d3a1b4e6defd5a07b02c0807a29af Author: Andrew Burgess Date: Sat Jan 18 22:38:29 2020 +0000 gdb/fortran: Support negative array stride in one limited case This commit adds support for negative Fortran array stride in one limited case, that is the case of a single element array with a negative array stride. The changes in this commit will be required in order for more general negative array stride support to work correctly, however, right now other problems in GDB prevent negative array strides from working in the general case. The reason negative array strides don't currently work in the general case is that when dealing with such arrays, the base address for the objects data is actually the highest addressed element, subsequent elements are then accessed with a negative offset from that address. Currently GDB supports positive type sizes, and having the base address of an object being its lowest address. I am working on a patch series to add more general negative array stride support, however, this is a much larger piece of work. The changes here can be summarised as, stop treating signed values as unsigned, specifically, the array stride, and offsets calculated using the array stride. The test for this issue was posted to the list by Sergio: https://sourceware.org/ml/gdb-patches/2020-01/msg00360.html Change-Id: I9087c767d1640946a0b876ea0920481e18ffed7c diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 1d5bfd4bc20..d1201b1df9a 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -1223,7 +1223,7 @@ create_array_type_with_stride (struct type *result_type, && !type_not_allocated (result_type))) { LONGEST low_bound, high_bound; - unsigned int stride; + int stride; /* If the array itself doesn't provide a stride value then take whatever stride the range provides. Don't update BIT_STRIDE as @@ -1241,9 +1241,18 @@ create_array_type_with_stride (struct type *result_type, In such cases, the array length should be zero. */ if (high_bound < low_bound) TYPE_LENGTH (result_type) = 0; - else if (stride > 0) - TYPE_LENGTH (result_type) = - (stride * (high_bound - low_bound + 1) + 7) / 8; + else if (stride != 0) + { + /* Ensure that the type length is always positive, even in the + case where (for example in Fortran) we have a negative + stride. It is possible to have a single element array with a + negative stride in Fortran (this doesn't mean anything + special, it's still just a single element array) so do + consider that case when touching this code. */ + LONGEST element_count = abs (high_bound - low_bound + 1); + TYPE_LENGTH (result_type) + = ((abs (stride) * element_count) + 7) / 8; + } else TYPE_LENGTH (result_type) = TYPE_LENGTH (element_type) * (high_bound - low_bound + 1); diff --git a/gdb/testsuite/gdb.fortran/vla-stride.exp b/gdb/testsuite/gdb.fortran/vla-stride.exp new file mode 100644 index 00000000000..15573e22cb3 --- /dev/null +++ b/gdb/testsuite/gdb.fortran/vla-stride.exp @@ -0,0 +1,44 @@ +# Copyright 2016-2020 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 . + +standard_testfile ".f90" + +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} \ + {debug f90 quiet}] } { + return -1 +} + +if ![runto MAIN__] then { + perror "couldn't run to breakpoint MAIN__" + continue +} + +gdb_breakpoint [gdb_get_line_number "re-reverse-elements"] +gdb_continue_to_breakpoint "re-reverse-elements" +gdb_test "print pvla" " = \\\(1, 2, 3, 4, 5, 6, 7, 8, 9, 10\\\)" \ + "print re-reverse-elements" +gdb_test "print pvla(1)" " = 1" "print first re-reverse-element" +gdb_test "print pvla(10)" " = 10" "print last re-reverse-element" + +gdb_breakpoint [gdb_get_line_number "odd-elements"] +gdb_continue_to_breakpoint "odd-elements" +gdb_test "print pvla" " = \\\(1, 3, 5, 7, 9\\\)" "print odd-elements" +gdb_test "print pvla(1)" " = 1" "print first odd-element" +gdb_test "print pvla(5)" " = 9" "print last odd-element" + +gdb_breakpoint [gdb_get_line_number "single-element"] +gdb_continue_to_breakpoint "single-element" +gdb_test "print pvla" " = \\\(5\\\)" "print single-element" +gdb_test "print pvla(1)" " = 5" "print one single-element" diff --git a/gdb/testsuite/gdb.fortran/vla-stride.f90 b/gdb/testsuite/gdb.fortran/vla-stride.f90 new file mode 100644 index 00000000000..22b8a65278e --- /dev/null +++ b/gdb/testsuite/gdb.fortran/vla-stride.f90 @@ -0,0 +1,29 @@ +! Copyright 2016-2020 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 . + +program vla_stride + integer, target, allocatable :: vla (:) + integer, pointer :: pvla (:) + + allocate(vla(10)) + vla = (/ (I, I = 1,10) /) + + pvla => vla(10:1:-1) + pvla => pvla(10:1:-1) + pvla => vla(1:10:2) ! re-reverse-elements + pvla => vla(5:4:-2) ! odd-elements + + pvla => null() ! single-element +end program vla_stride diff --git a/gdb/valarith.c b/gdb/valarith.c index 79b148602bb..be0e0731bee 100644 --- a/gdb/valarith.c +++ b/gdb/valarith.c @@ -187,7 +187,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound { struct type *array_type = check_typedef (value_type (array)); struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type)); - ULONGEST elt_size = type_length_units (elt_type); + LONGEST elt_size = type_length_units (elt_type); /* Fetch the bit stride and convert it to a byte stride, assuming 8 bits in a byte. */ @@ -199,7 +199,7 @@ value_subscripted_rvalue (struct value *array, LONGEST index, LONGEST lowerbound elt_size = stride / (unit_size * 8); } - ULONGEST elt_offs = elt_size * (index - lowerbound); + LONGEST elt_offs = elt_size * (index - lowerbound); if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)