From patchwork Mon Apr 23 17:45:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 26910 Received: (qmail 108050 invoked by alias); 23 Apr 2018 17:45:56 -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 107841 invoked by uid 89); 23 Apr 2018 17:45:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=observed, summarized, Integral, SPARC 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 ESMTP; Mon, 23 Apr 2018 17:45:36 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E268E117261; Mon, 23 Apr 2018 13:45:32 -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 Qs9z-FiGoKWk; Mon, 23 Apr 2018 13:45:32 -0400 (EDT) Received: from tron.gnat.com (tron.gnat.com [205.232.38.10]) by rock.gnat.com (Postfix) with ESMTP id D0923117250; Mon, 23 Apr 2018 13:45:32 -0400 (EDT) Received: by tron.gnat.com (Postfix, from userid 4233) id C1E7754C; Mon, 23 Apr 2018 13:45:32 -0400 (EDT) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Vladimir Mezentsev Subject: [RFA/commit] (SPARC/LEON) fix incorrect array return value printed by "finish" Date: Mon, 23 Apr 2018 13:45:29 -0400 Message-Id: <1524505529-79109-1-git-send-email-brobecker@adacore.com> Consider the code in the gdb.ada/array_return.exp testcase, which defines a function returning an array of 2 integers: type Data_Small is array (1 .. 2) of Integer; function Create_Small return Data_Small; When doing a "finish" from inside function Create_Small, we expect GDB to tell us that the return value was "(1, 1)". However, it currently prints the wrong value: (gdb) finish Run till exit from #0 pck.create_small () at /[...]/pck.adb:5 p () at /[...]/p.adb:10 10 Large := Create_Large; Value returned is $1 = (0, 0) This is a regression which I traced back to the following commit... | commit 1933fd8ee01ad2e74a9c6341bc40f54962a8f889 | Date: Fri May 19 03:06:19 2017 -0700 | Subject: gdb: fix TYPE_CODE_ARRAY handling in sparc targets ... which, despite what the subject says, is not really about TYPE_CODE_ARRAY handling, which is a bit of an implementation detail, but about the GNU vectors extension. The author of the patch equated TYPE_CODE_ARRAY with vectors, which is not correct. Vectors are TYPE_CODE_ARRAY types with the TYPE_VECTOR flag set. So at the very minimum, the patch should have been checking for both TYPE_CODE_ARRAY and TYPE_VECTOR. But, that's not the only thing that did not seem right to me. When looking at the ABI, and at the summary of the implementation in GCC of the calling conventions for that architecture: size argument return value small integer <4 int. reg. int. reg. word 4 int. reg. int. reg. double word 8 int. reg. int. reg. _Complex small integer <8 int. reg. int. reg. _Complex word 8 int. reg. int. reg. _Complex double word 16 memory int. reg. vector integer <=8 int. reg. FP reg. vector integer >8 memory memory float 4 int. reg. FP reg. double 8 int. reg. FP reg. long double 16 memory memory _Complex float 8 memory FP reg. _Complex double 16 memory FP reg. _Complex long double 32 memory FP reg. vector float any memory memory aggregate any memory memory The nice thing about the patch above is that it nicely factorized the code that determines how arguments are passed/returns. The bad news is that the implementation, particularly for the handling of arrays and vectors, doesn't seem to match the summary above. Hence, the regression we observed. So what I did was review and re-implement some of the predicate functions according to the summary above. Because dejagnu crashes all our Solaris machines real bad, I can't run the dejagnu testsuite there. So what I did was test the patch with AdaCore's testsuite against leon3-elf, no regression. I verified that this fixes the regression above while at the same time still passing gdb.base/gnu_vector.exp (I transposed that testcase to our testsuite), which is the testcase that was cited in the commit above as seeing some FAIL->PASS improvements. This patch also removes one assertion... gdb_assert (sparc_integral_or_pointer_p (type) || (TYPE_CODE (type) == TYPE_CODE_ARRAY && len <= 8)); ... because that assertion is really the "negative" of the other conditions written in the same "if, else if, else [assert]" block in this function. To me, this assertion forces us to maintain two versions of the same code, and is an unnecessary burden. In particular, the above is not the correct condition, and the ABI summary table above shows that we need a more complex condition to describe the situations where we expect arguments to be passed by register. gdb/ChangeLog: * sparc-tdep.c (sparc_structure_return_p): Re-implement to match the ABI as summarized in GCC's gcc/config/sparc/sparc.c. (sparc_arg_by_memory_p): Renamed from sparc_arg_on_registers_p. Re-implement to match the ABI as summarized in GCC's gcc/config/sparc/sparc.c. All callers updated. (sparc32_store_arguments): Remove assertion. OK to commit in master? Thanks! diff --git a/gdb/sparc-tdep.c b/gdb/sparc-tdep.c index 633bd68..be461a0 100644 --- a/gdb/sparc-tdep.c +++ b/gdb/sparc-tdep.c @@ -297,41 +297,61 @@ sparc_structure_or_union_p (const struct type *type) return 0; } -/* Check whether TYPE is returned on registers. */ +/* Return true if TYPE is returned by memory, false if returned by + register. */ static bool sparc_structure_return_p (const struct type *type) { - if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8) + if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)) { - struct type *t = check_typedef (TYPE_TARGET_TYPE (type)); + /* Float vectors are always returned by memory. */ + if (sparc_floating_p (check_typedef (TYPE_TARGET_TYPE (type)))) + return true; + /* Integer vectors are returned by memory if the vector size + is greater than 8 bytes long. */ + return (TYPE_LENGTH (type) > 8); + } - if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8) - return true; - return false; + if (sparc_floating_p (type)) + { + /* Floating point types are passed by register for size 4 and + 8 bytes, and by memory for size 16 bytes. */ + return (TYPE_LENGTH (type) == 16); } - if (sparc_floating_p (type) && TYPE_LENGTH (type) == 16) - return true; + + /* Other than that, only aggregates of all sizes get returned by + memory. */ return sparc_structure_or_union_p (type); } -/* Check whether TYPE is passed on registers. */ +/* Return true if arguments of the given TYPE are passed by + memory; false if returned by register. */ static bool -sparc_arg_on_registers_p (const struct type *type) +sparc_arg_by_memory_p (const struct type *type) { - if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_LENGTH (type) <= 8) + if (TYPE_CODE (type) == TYPE_CODE_ARRAY && TYPE_VECTOR (type)) { - struct type *t = check_typedef (TYPE_TARGET_TYPE (type)); - - if (sparc_floating_p (t) && TYPE_LENGTH (t) == 8) - return false; - return true; + /* Float vectors are always passed by memory. */ + if (sparc_floating_p (check_typedef (TYPE_TARGET_TYPE (type)))) + return true; + /* Integer vectors are passed by memory if the vector size + is greater than 8 bytes long. */ + return (TYPE_LENGTH (type) > 8); } - if (sparc_structure_or_union_p (type) || sparc_complex_floating_p (type) - || (sparc_floating_p (type) && TYPE_LENGTH (type) == 16)) - return false; - return true; + + /* Floats are passed by register for size 4 and 8 bytes, and by memory + for size 16 bytes. */ + if (sparc_floating_p (type)) + return (TYPE_LENGTH (type) == 16); + + /* Complex floats and aggregates of all sizes are passed by memory. */ + if (sparc_complex_floating_p (type) || sparc_structure_or_union_p (type)) + return true; + + /* Everything else gets passed by register. */ + return false; } /* Register information. */ @@ -606,7 +626,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs, struct type *type = value_type (args[i]); int len = TYPE_LENGTH (type); - if (!sparc_arg_on_registers_p (type)) + if (sparc_arg_by_memory_p (type)) { /* Structure, Union and Quad-Precision Arguments. */ sp -= len; @@ -627,9 +647,7 @@ sparc32_store_arguments (struct regcache *regcache, int nargs, } else { - /* Integral and pointer arguments. */ - gdb_assert (sparc_integral_or_pointer_p (type) - || (TYPE_CODE (type) == TYPE_CODE_ARRAY && len <= 8)); + /* Arguments passed via the General Purpose Registers. */ num_elements += ((len + 3) / 4); } }