From patchwork Tue Apr 16 18:28:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 32306 Received: (qmail 59866 invoked by alias); 16 Apr 2019 18:28:19 -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 59858 invoked by uid 89); 16 Apr 2019 18:28:18 -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, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=HTo:U*tromey, amd64-eval.cc, amd64evalexp, amd64-eval.exp 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; Tue, 16 Apr 2019 18:28:17 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 706085614F; Tue, 16 Apr 2019 14:28:15 -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 nE+S854J9ata; Tue, 16 Apr 2019 14:28:15 -0400 (EDT) Received: from murgatroyd (75-166-39-124.hlrn.qwest.net [75.166.39.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id EE52D5614E; Tue, 16 Apr 2019 14:28:14 -0400 (EDT) From: Tom Tromey To: Tom Tromey Cc: "Leszek Swirski via gdb-patches" , Leszek Swirski , palves@redhat.com Subject: Re: [PATCH v3] [amd64] Fix AMD64 return value ABI in expression evaluation References: <20190214151602.147300-1-leszeks@google.com> <20190214151810.149322-1-leszeks@google.com> <87v9zducov.fsf@tromey.com> Date: Tue, 16 Apr 2019 12:28:14 -0600 In-Reply-To: <87v9zducov.fsf@tromey.com> (Tom Tromey's message of "Tue, 16 Apr 2019 10:39:28 -0600") Message-ID: <87o955u7nl.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 >>> This ABI mismatch resulted in issues when calling a function that returns >>> a class of size <16 bytes which has a base class, including issues such >>> as the "this" pointer being incorrect (as it was passed as the second >>> argument rather than the first). Tom> I'm still looking into the problem, but this regressed an internal test Tom> case here at AdaCore. In particular, this patch doesn't seem to treat Tom> bitfields the same way that gcc does. The appended fixes the test case that's included, but I'm not really sure it's correct, since I am not sure that the test for "bit-field-ness" is correct here. Let me know what you think. Tom commit 93fb3e610bcd0ff5763334588bd84e41a4f73ef9 Author: Tom Tromey Date: Tue Apr 16 11:11:10 2019 -0600 Fix passing of struct with bitfields on x86-64 Commit 4aa866af ("Fix AMD64 return value ABI in expression evaluation") introduced a regression when calling a function with a structure that contains bitfields. Because the caller of amd64_has_unaligned_fields handles bitfields already, it seemed to me that the simplest fix was to ignore bitfields here. gdb/ChangeLog 2019-04-16 Tom Tromey * amd64-tdep.c (amd64_has_unaligned_fields): Ignore bitfields. gdb/testsuite/ChangeLog 2019-04-16 Tom Tromey * gdb.arch/amd64-eval.exp: Test bitfield return. * gdb.arch/amd64-eval.cc (struct Bitfields): New. (class Foo) : New method. (main): Call it. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ba1300d57ef..bc4a4b50ff5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2019-04-16 Tom Tromey + + * amd64-tdep.c (amd64_has_unaligned_fields): Ignore bitfields. + 2019-04-15 Leszek Swirski * amd64-tdep.c (amd64_classify_aggregate): Use cp_pass_by_reference diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index d4c96de5716..31791f9a9f1 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -555,11 +555,13 @@ amd64_has_unaligned_fields (struct type *type) int bitpos = TYPE_FIELD_BITPOS (type, i); int align = type_align(subtype); - /* Ignore static fields, or empty fields, for example nested - empty structures. */ + /* Ignore static fields, empty fields (for example nested + empty structures), and bitfields (these are handled by + the caller). */ if (field_is_static (&TYPE_FIELD (type, i)) || (TYPE_FIELD_BITSIZE (type, i) == 0 - && TYPE_LENGTH (subtype) == 0)) + && TYPE_LENGTH (subtype) == 0) + || TYPE_FIELD_PACKED (type, i)) continue; if (bitpos % 8 != 0) @@ -569,7 +571,7 @@ amd64_has_unaligned_fields (struct type *type) if (bytepos % align != 0) return true; - if (amd64_has_unaligned_fields(subtype)) + if (amd64_has_unaligned_fields (subtype)) return true; } } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index d46422635be..f9cc0259c56 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2019-04-16 Tom Tromey + + * gdb.arch/amd64-eval.exp: Test bitfield return. + * gdb.arch/amd64-eval.cc (struct Bitfields): New. + (class Foo) : New method. + (main): Call it. + 2019-04-15 Leszek Swirski * gdb.arch/amd64-eval.cc: New file. diff --git a/gdb/testsuite/gdb.arch/amd64-eval.cc b/gdb/testsuite/gdb.arch/amd64-eval.cc index a986a49db34..907233ff04a 100644 --- a/gdb/testsuite/gdb.arch/amd64-eval.cc +++ b/gdb/testsuite/gdb.arch/amd64-eval.cc @@ -63,6 +63,16 @@ struct UnalignedFieldsInBase : public UnalignedFields int32_t x2; }; +struct Bitfields +{ + Bitfields(unsigned int x, unsigned int y) + : fld(x), fld2(y) + {} + + unsigned fld : 7; + unsigned fld2 : 7; +}; + class Foo { public: @@ -101,6 +111,13 @@ public: return UnalignedFieldsInBase (x, y, x2); } + Bitfields + return_bitfields (unsigned int x, unsigned int y) + { + assert (this->tag == EXPECTED_TAG); + return Bitfields(x, y); + } + private: /* Use a tag to detect if the "this" value is correct. */ static const int EXPECTED_TAG = 0xF00F00F0; @@ -116,5 +133,6 @@ main (int argc, char *argv[]) foo.return_non_trivial_destructor(3); foo.return_unaligned(4, 5); foo.return_unaligned_in_base(6, 7, 8); + foo.return_bitfields(23, 74); return 0; // break-here } diff --git a/gdb/testsuite/gdb.arch/amd64-eval.exp b/gdb/testsuite/gdb.arch/amd64-eval.exp index c33777d2b41..beef46ad133 100644 --- a/gdb/testsuite/gdb.arch/amd64-eval.exp +++ b/gdb/testsuite/gdb.arch/amd64-eval.exp @@ -41,3 +41,5 @@ gdb_test "call foo.return_unaligned(78, 9.25)" \ " = {x = 78, y = 9.25}" gdb_test "call foo.return_unaligned_in_base(23, 4.5, 67)" \ " = { = {x = 23, y = 4.5}, x2 = 67}" +gdb_test "call foo.return_bitfields(23, 74)" \ + " = {fld = 23, fld2 = 74}"