From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 0D06E3858D28; Tue, 14 Dec 2021 09:50:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0D06E3858D28 From: "aburgess at redhat dot com" To: gdb-prs@sourceware.org Subject: [Bug gdb/28681] Wrong pretty-printed unique_ptr value when using "finish" Date: Tue, 14 Dec 2021 09:50:33 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gdb X-Bugzilla-Component: gdb X-Bugzilla-Version: HEAD X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: aburgess at redhat dot com X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: unassigned at sourceware dot org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gdb-prs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-prs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Dec 2021 09:50:34 -0000 https://sourceware.org/bugzilla/show_bug.cgi?id=3D28681 --- Comment #5 from Andrew Burgess --- Sure. My original comment was all I knew at the time, but I've looked into= it a little more since then. The call stack when the assert triggers is: amd64_return_value amd64_classify amd64_classify_aggregate amd64_has_unaligned_fields In amd64_has_unaligned_fields we have this loop: for (int i =3D 0; i < type->num_fields (); i++) { struct type *subtype =3D check_typedef (type->field (i).type ()); /* Ignore static fields, empty fields (for example nested empty structures), and bitfields (these are handled by the caller). */ if (field_is_static (&type->field (i)) || (TYPE_FIELD_BITSIZE (type, i) =3D=3D 0 && TYPE_LENGTH (subtype) =3D=3D 0) || TYPE_FIELD_PACKED (type, i)) continue; int bitpos =3D type->field (i).loc_bitpos (); /* ... snip ... */ } And it is when we ask for the loc_bitpos that the assert triggers, as the field's type is no FIELD_LOC_KIND_BITPOS, but is instead, FIELD_LOC_KIND_DWARF_BLOCK. In the case we're handling, the field being processed is the field that describes the virtual base of the type being processed. I originally starting playing with is_dynamic_type, but noticed that the ty= pe in question doesn't return true for is_dynamic_type. This seemed weird, as clearly the offset needs resolving. But then I found some code in is_dynamic_type_internal which specifically detects the exact case I'm look= ing at, and ensures that such a case is NOT enough to make a type appear dynami= c.=20 The comment on this code says: /* Do not consider C++ virtual base types to be dynamic due to the field's offset being dynamic; these are handled via other means. */ But, frustratingly, doesn't indicate what that other means is, or where I m= ight find it. However, I might be able to side step this issue for now. The patch I'm playing with adds a check in amd64_classify_aggregate, if the aggregate is not trivially_copyable then it should be passed in memory. No further checks are required. I initially added the trivially_copyable check as the last check of the if condition, like this: if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type) || !language_pass_by_reference (type).trivially_copyable) { theclass[0] =3D theclass[1] =3D AMD64_MEMORY; return; } However, while looking at the assertion case, I noticed that the type with a virtual base was not trivially_copyable, so, if I reorder the checks like t= his: if (TYPE_LENGTH (type) > 16 || !language_pass_by_reference (type).trivially_copyable || amd64_has_unaligned_fields (type)) { theclass[0] =3D theclass[1] =3D AMD64_MEMORY; return; } Then, problem solved! Or, at least, problem kicked down the road! I'm sti= ll experimenting, but, so far, I've not been able to create an example that bo= th uses virtual base classes, and is trivially_copyable. My plan for now is to place the checks in the "correct" order, and add a so= lid comment explaining that the order is important (plus the test will fail if = the order is ever changed). Things that would help are: 1. Suggestions for how amd64_has_unaligned_fields can be improved in orde= r to handle virtual base class fields, e.g. how do we obtain the field offset for those fields, or 2. Confirmation that in C++ types with a virtual base class, are, by definition, never trivially_copyable, this would imply my solution is actua= lly not such a hack after all, or 3. An example of a class with a virtual base class, that is trivially copyable. Ideally, the class would be less than 8-bytes in size so that it= can be passed in a single register, such an example would then fail in my patch= ed GDB. --=20 You are receiving this mail because: You are on the CC list for the bug.=