From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 68C11385842B; Thu, 23 Dec 2021 12:15:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 68C11385842B From: "cvs-commit at gcc dot gnu.org" To: gdb-prs@sourceware.org Subject: [Bug gdb/28681] Wrong pretty-printed unique_ptr value when using "finish" Date: Thu, 23 Dec 2021 12:15:57 +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: cvs-commit at gcc dot gnu.org 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: Thu, 23 Dec 2021 12:15:57 -0000 https://sourceware.org/bugzilla/show_bug.cgi?id=3D28681 --- Comment #8 from cvs-commit at gcc dot gnu.org --- The master branch has been updated by Andrew Burgess : https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Db1718fcdd1d2= a5c514f8ee504ba07fb3f42b8608 commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. Consider this test program: struct A { int i; A () { this->i =3D 0; } A (const A& a) { this->i =3D a.i; } }; A func (int i) { A a; a.i =3D i; return a; } int main () { A a =3D func (3); return a.i; } And this GDB session: $ gdb -q ex.x Reading symbols from ex.x... (gdb) b func Breakpoint 1 at 0x401115: file ex.cc, line 14. (gdb) r Starting program: /home/andrew/tmp/ex.x Breakpoint 1, func (i=3D3) at ex.cc:14 14 A a; (gdb) finish Run till exit from #0 func (i=3D3) at ex.cc:14 main () at ex.cc:23 23 return a.i; Value returned is $1 =3D { i =3D -19044 } (gdb) p a $2 =3D { i =3D 3 } (gdb) Notice how after the `finish` the contents of $1 are junk, but, when I immediately ask for the value of `a`, I get back the correct value. The problem here is that after the finish command GDB calls the function amd64_return_value to figure out where the return value can be found (on x86-64 targets anyway). This function makes the wrong choice for the struct A in our case, as sizeof(A) <=3D 8, then amd64_return_value decides that A will be returned in a register. GDB then reads the return value register an interprets the contents as an instance of A. Unfortunately, A is not trivially copyable (due to its copy constructor), and the sys-v specification for argument and return value passing, says that any non-trivial C++ object should have space allocated for it by the caller, and the address of this space is passed to the callee as a hidden first argument. The callee should then return the address of this space as the return value. And so, the register that GDB is treating as containing an instance of A, actually contains the address of an instance of A (in this case on the stack), this is why GDB shows the incorrect result. The call stack within GDB for where we actually go wrong is this: amd64_return_value amd64_classify amd64_classify_aggregate And it is in amd64_classify_aggregate that we should be classifying the type as AMD64_MEMORY, instead of as AMD64_INTEGER as we currently do (via a call to amd64_classify_aggregate_field). At the top of amd64_classify_aggregate we already have this logic: if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type)) { theclass[0] =3D theclass[1] =3D AMD64_MEMORY; return; } Which handles some easy cases where we know a struct will be placed into memory, that is (a) the struct is more than 16-bytes in size, or (b) the struct has any unaligned fields. All we need then, is to add a check here to see if the struct is trivially copyable. If it is not then we know the struct will be passed in memory. I originally structured the code 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; } This solved the example from the bug, and my small example above. So then I started adding some more extensive tests to the GDB testsuite, and I ran into a problem. I hit this error: gdbtypes.h:676: internal-error: loc_bitpos: Assertion `m_loc_kind =3D= =3D FIELD_LOC_KIND_BITPOS' failed. This problem is triggered from: amd64_classify_aggregate amd64_has_unaligned_fields field::loc_bitpos Inside the unaligned field check we try to get the bit position of each field. Unfortunately, in some cases the field location is not FIELD_LOC_KIND_BITPOS, but is FIELD_LOC_KIND_DWARF_BLOCK. An example that shows this bug is: struct B { short j; }; struct A : virtual public B { short i; A () { this->i =3D 0; } A (const A& a) { this->i =3D a.i; } }; A func (int i) { A a; a.i =3D i; return a; } int main () { A a =3D func (3); return a.i; } It is the virtual base class, B, that causes the problem. The base class is represented, within GDB, as a field within A. However, the location type for this field is a DWARF_BLOCK. I spent a little time trying to figure out how to convert the DWARF_BLOCK to a BITPOS, however, I realised that, in this case at least, conversion is not needed. The C++ standard says that a class is not trivially copyable if it has any virtual base classes. And so, in this case, even if I could figure out the BITPOS for the virtual base class fields, I know for sure that I would immediately fail the trivially_copyable check. So, lets just reorder the checks in amd64_classify_aggregate to: 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; } Now, if we have a class with virtual bases we will fail quicker, and avoid the unaligned fields check completely. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D28681 --=20 You are receiving this mail because: You are on the CC list for the bug.=