From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x844.google.com (mail-qt1-x844.google.com [IPv6:2607:f8b0:4864:20::844]) by sourceware.org (Postfix) with ESMTPS id 6CC3D3851C09 for ; Thu, 27 Aug 2020 19:58:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6CC3D3851C09 Received: by mail-qt1-x844.google.com with SMTP id y65so5624980qtd.2 for ; Thu, 27 Aug 2020 12:58:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HtiPtnUXC6Fk7QOzXiByMJwBKKdZ6rWd+UmG/rwdU7c=; b=BWp2d0uEjDK7L/KUBPUikY8M+6gTCkTmuJmfdTPKcmPbspJhtI54osfREK+J66dQ8L kmMHOBGsJ8mu/r8Y1ClPIGAtWeB+VVCNjqKK/7USgRnkT90IxwObsvwb3enzWTLyypPd JyVi8xWWYHadOrv8V+qHhx87bRtREzej4RSXGLqjrXAUlFlmbPnI0+4WRo0MyeX8JNP7 xxC/WqJ9XlCmVr7nf6tly07p0i4EHr875BaxSQoiUZYDlKibFfqOE/lkwCcOHCemNK/j qniEbIMNn0P1DuIz9t3+k9lz5ZC0EUwAFYBr0D2AP7u3NWpKjhw3OB+41JhCrTZQgGnz EMow== X-Gm-Message-State: AOAM530NbNXHPMU7zc/pu8DfjDihvMBZ19Unxh7seFGO5BwxoL6lfwnF 1ce9IdkLnSF4+lyZlYGqiAWxL57dme7fWg== X-Google-Smtp-Source: ABdhPJwXaapcMiaMbS13arS3N9WGCPgSodjT7BFIoL/vVw58iuQ/BV7QwG50f9YbDt3yEcZzWf4H2Q== X-Received: by 2002:ac8:44d5:: with SMTP id b21mr21166606qto.261.1598558306407; Thu, 27 Aug 2020 12:58:26 -0700 (PDT) Received: from ?IPv6:2804:7f0:8283:a3ee:1908:8b5e:50f8:c881? ([2804:7f0:8283:a3ee:1908:8b5e:50f8:c881]) by smtp.gmail.com with ESMTPSA id b84sm2766652qkc.99.2020.08.27.12.58.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Aug 2020 12:58:26 -0700 (PDT) Subject: Re: [PATCH] Reject ambiguous C++ field accesses To: Pedro Alves , gdb-patches@sourceware.org References: <20200827180251.20244-1-pedro@palves.net> From: Luis Machado Message-ID: Date: Thu, 27 Aug 2020 16:58:23 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200827180251.20244-1-pedro@palves.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Aug 2020 19:58:30 -0000 On 8/27/20 3:02 PM, Pedro Alves wrote: > The gdb.cp/ambiguous.exp testcase had been disabled for many years, > but recently it was re-enabled. However, it is failing everywhere. > That is because it is testing an old feature that is gone from GDB. > > The testcase is expecting to see an ambiguous field warning, like: > > # X is derived from A1 and A2; both A1 and A2 have a member 'x' > send_gdb "print x.x\n" > gdb_expect { > -re "warning: x ambiguous; using X::A2::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { > pass "print x.x" > } > -re "warning: x ambiguous; using X::A1::x. Use a cast to disambiguate.\r\n\\$\[0-9\]* = \[-\]*\[0-9\]*\r\n$gdb_prompt $" { > pass "print x.x" > } > -re ".*$gdb_prompt $" { fail "print x.x" } > timeout { fail "(timeout) print x.x" } > } > > However, GDB just accesses one of the candidates without warning or > error: > > print x.x > $1 = 1431655296 > (gdb) FAIL: gdb.cp/ambiguous.exp: print x.x > > (The weird number is because the testcase does not initialize the > variables.) > > The testcase come in originally with the big HP merge: > > +Sun Jan 10 23:44:11 1999 David Taylor > + > + > + The following files are part of the HP merge; some had longer > + names at HP, but have been renamed to be no more than 14 > + characters in length. > > Looking at the tree back then, we find that warning: > > /* Helper function used by value_struct_elt to recurse through baseclasses. > Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes, > and search in it assuming it has (class) type TYPE. > If found, return value, else return NULL. > > If LOOKING_FOR_BASECLASS, then instead of looking for struct fields, > look for a baseclass named NAME. */ > > static value_ptr > search_struct_field (name, arg1, offset, type, looking_for_baseclass) > char *name; > register value_ptr arg1; > int offset; > register struct type *type; > int looking_for_baseclass; > { > int found = 0; > char found_class[1024]; > value_ptr v; > struct type *vbase = NULL; > > found_class[0] = '\000'; > > v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase); > if (found > 1) > warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.", > name, found_class, name); > > return v; > } > > > However, in current GDB, search_struct_field does not handle the > ambiguous field case, nor is that warning found anywhere. Somehow it > got lost over the years. That seems like a regression, because the > compiler (as per language rules) rejects the ambiguous accesses as > well. E.g.: > > gdb.cp/ambiguous.cc:98:5: error: request for member 'x' is ambiguous > 98 | x.x = 1; > | ^ > gdb.cp/ambiguous.cc:10:7: note: candidates are: 'int A2::x' > 10 | int x; > | ^ > gdb.cp/ambiguous.cc:4:7: note: 'int A1::x' > 4 | int x; > | ^ > > > This patch restores the feature, though implemented differently and > with better user experience, IMHO. An ambiguous access is now an > error instead of a warning, and also GDB shows you the all candidates, > like: > > (gdb) print x.x > Request for member 'x' is ambiguous in type 'X'. Candidates are: > 'int A1::x' (X -> A1) > 'int A2::x' (X -> A2) > (gdb) print j.x > Request for member 'x' is ambiguous in type 'J'. Candidates are: > 'int A1::x' (J -> K -> A1) > 'int A1::x' (J -> L -> A1) > > Users can then fix their commands by casting or by specifying the > baseclass explicitly, like: > > (gdb) p x.A1::x > $1 = 1 > (gdb) p x.A2::x > $2 = 2 > (gdb) p ((A1) x).x > $3 = 1 > (gdb) p ((A2) x).x > $4 = 2 > (gdb) p j.K::x > $12 = 1 > (gdb) p j.L::x > $13 = 2 > (gdb) p j.A1::x > base class 'A1' is ambiguous in type 'J' > > The last error I've not touched; could be improved to also list the > baseclass candidates. > > The showing the class "path" for each candidate was inspired by GCC's > output when you try an ambiguous cast: > > gdb.cp/ambiguous.cc:161:8: error: ambiguous conversion from derived class 'const JVA1' to base class 'const A1': > class JVA1 -> class KV -> class A1 > class JVA1 -> class A1 > (A1) jva1; > ^~~~ > > I did not include the "class" word as it seemed unnecessarily > repetitive, but I can include it if people prefer it: > > (gdb) print j.x > Request for member 'x' is ambiguous in type 'J'. Candidates are: > 'int A1::x' (class J -> class K -> class A1) > 'int A1::x' (class J -> class L -> class A1) > > The testcase is adjusted accordingly. I also took the chance to > modernize it at the same time. > > Also, as mentioned above, the testcase doesn't currently initialize > the tested variables. This patch inializes them all, giving each > field a distinct value, so that we can be sure that GDB is accessing > the right fields / offsets. The testcase is extended accordingly. > > Unfortunately, this exposes some bugs, not addressed in this patch. > The bug is around a class that inherits from A1 directly and also > inherits from with two other distinct base classes that inherit > virtually from A1 in turn: > > print jva1.KV::x > $51 = 1431665544 > (gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::x > print jva1.KV::y > $52 = 21845 > (gdb) FAIL: gdb.cp/ambiguous.exp: all fields: print jva1.KV::y > > This shows how GDB is clearly reading the confusing the offset of the > _vptr.KV with the A1 offset: > > (gdb) print /x jva1 > $1 = { = { = {x = 0x2, y = 0x16}, _vptr.KV = 0x555555557b88 , i = 0x457}, ......} > ^^^^^^^^^^^^^^ > (gdb) print /x jva1.KV::x > $2 = 0x55557b88 > ^^^^^^^^^^ > (gdb) print /x jva1.KV::y > $3 = 0x5555 > ^^^^^^ > > (gdb) print /x (KV)jva1 > $4 = { = , _vptr.KV = 0x555555557b88 , i = 0x457} > (gdb) print /x (A1)(KV)jva1 > Cannot access memory at address 0x0 > > Since that's an orthogonal issue, I'm kfailing the tests that fail > because of it. (I'll file a bug soon, unless someone can fix it > quickly!) > > gdb/ChangeLog: > > * valops.c (struct struct_field_searcher): New. > (update_search_result): Rename to ... > (struct_field_searcher::update_result): ... this. Simplify > prototype. Record all found fields. > (do_search_struct_field): Rename to ... > (struct_field_searcher::search): ... this. Simplify prototype. > Maintain stack of visited baseclass path. Call update_result for > fields too. Keep searching fields in baseclasses instead of > stopping at the first found field. > (search_struct_field): Use struct_field_searcher. When looking > for fields, report ambiguous access attempts. > > gdb/testsuite/ChangeLog: > > * gdb.cp/ambiguous.cc (marker1): Delete. > (main): Initialize all the fields of the locals. Replace marker1 > call with a "set breakpoint here" marker. > * gdb.cp/ambiguous.exp: Modernize. Use gdb_continue_to_breakpoint > instead of running to marker1. Add tests printing all the > variables and all the fields of the variables. > (test_ambiguous): New proc, expecting the new GDB output when a > field access is ambiguous. Change all "warning: X ambiguous" > tests to use it. > --- > gdb/testsuite/gdb.cp/ambiguous.cc | 87 ++++++++-- > gdb/testsuite/gdb.cp/ambiguous.exp | 321 +++++++++++++++++++++---------------- > gdb/valops.c | 219 ++++++++++++++++++------- > 3 files changed, 418 insertions(+), 209 deletions(-) > > diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc > index 6ee7bc18ea9..a55686547f2 100644 > --- a/gdb/testsuite/gdb.cp/ambiguous.cc > +++ b/gdb/testsuite/gdb.cp/ambiguous.cc > @@ -1,9 +1,4 @@ > > -void marker1() > -{ > - return; > -} > - > class A1 { > public: > int x; > @@ -102,9 +97,81 @@ int main() > > i += k.i + m.w + a1.x + a2.x + a3.x + x.z + l.z + n.r + j.j; > > - marker1(); > - > + /* Initialize all the fields. Keep the order the same as in the > + .exp file. */ > + > + a1.x = 1; > + a1.y = 2; > + > + a2.x = 1; > + a2.y = 2; > + > + a3.x = 1; > + a3.y = 2; > + > + x.A1::x = 1; > + x.A1::y = 2; > + x.A2::x = 3; > + x.A2::y = 4; > + x.z = 5; > + > + l.x = 1; > + l.y = 2; > + l.z = 3; > + > + m.x = 1; > + m.y = 2; > + m.w = 3; > + > + n.A1::x = 1; > + n.A1::y = 2; > + n.A2::x = 3; > + n.A2::y = 4; > + n.w = 5; > + n.r = 6; > + n.z = 7; > + > + k.x = 1; > + k.y = 2; > + k.i = 3; > + > + j.K::x = 1; > + j.K::y = 2; > + j.L::x = 3; > + j.L::y = 4; > + j.i = 5; > + j.z = 6; > + j.j = 7; > + > + jv.x = 1; > + jv.y = 2; > + jv.i = 3; > + jv.z = 4; > + jv.jv = 5; > + > + jva1.KV::x = 1; > + jva1.KV::y = 2; > + jva1.LV::x = 3; > + jva1.LV::y = 4; > + jva1.z = 5; > + jva1.i = 6; > + jva1.jva1 = 7; > + > + jva2.KV::x = 1; > + jva2.KV::y = 2; > + jva2.LV::x = 3; > + jva2.LV::y = 4; > + jva2.A2::x = 5; > + jva2.A2::y = 6; > + jva2.z = 7; > + jva2.i = 8; > + jva2.jva2 = 9; > + > + jva1v.x = 1; > + jva1v.y = 2; > + jva1v.z = 3; > + jva1v.i = 4; > + jva1v.jva1v = 5; > + > + return 0; /* set breakpoint here */ > } > - > - > - > diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp > index 17fb29f5350..74783cd0725 100644 > --- a/gdb/testsuite/gdb.cp/ambiguous.exp > +++ b/gdb/testsuite/gdb.cp/ambiguous.exp > @@ -15,15 +15,9 @@ > > # This file is part of the gdb testsuite > > -# tests relating to ambiguous class members > -# Written by Satish Pai 1997-07-28 > - > -# This file is part of the gdb testsuite > - > -# > -# test running programs > -# > - > +# Print out various class objects' members and check that the error > +# about the field or baseclass being ambiguious is emitted at the > +# right times. Typo, ambiguious. > > if { [skip_cplus_tests] } { continue } > > @@ -47,180 +41,225 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ > return -1 > } > > -# > -# set it up at a breakpoint so we can play with the variable values > -# > +# Set it up at a breakpoint so we can play with the variable values. > + This old comment reads funny. Set what up? I'd rephrase a bit. > diff --git a/gdb/valops.c b/gdb/valops.c > index 0eb2b096211..0e030a03ecd 100644 > --- a/gdb/valops.c > +++ b/gdb/valops.c > @@ -1766,25 +1766,105 @@ typecmp (int staticp, int varargs, int nargs, > return i + 1; > } > > -/* Helper class for do_search_struct_field that updates *RESULT_PTR > - and *LAST_BOFFSET, and possibly throws an exception if the field > - search has yielded ambiguous results. */ > +/* Helper class for search_struct_field that keeps track of found > + results. See search_struct_field for description of > + LOOKING_FOR_BASECLASS. If LOOKING_FOR_BASECLASS is true, possibly > + throws an exception if the base class search has yielded ambiguous "throw" an exception instead? > + results. If LOOKING_FOR_BASECLASS is false, found fields are > + accumulated and the caller (search_struct_field takes) care of "takes" outside the parenthesis. > + throwing an error if the field search yielded ambiguous results. > + The latter is done that way so that the error message can include a > + list of all the found candidates. */ > + > +struct struct_field_searcher > +{ > + /* A found field. */ > + struct found_field > + { > + /* Patch to the structure where the field was found. */ Typo... Path. > + std::vector path; > + > + /* The field found. */ > + struct value *field_value; > + }; > + > + struct_field_searcher (const char *name, > + struct type *outermost_type, > + bool looking_for_baseclass) > + : m_name (name), > + m_looking_for_baseclass (looking_for_baseclass), > + m_outermost_type (outermost_type) > + { > + } > > -static void > -update_search_result (struct value **result_ptr, struct value *v, > - LONGEST *last_boffset, LONGEST boffset, > - const char *name, struct type *type) > + void search (struct value *arg1, LONGEST offset, > + struct type *type); > + > + const std::vector &fields () > + { > + return m_fields; > + } > + > + struct value *baseclass () > + { > + return m_baseclass; > + } > + > +private: > + /* Update results to include V, a found field/baseclass. */ > + void update_result (struct value *v, LONGEST boffset); > + > + /* The name of the field/baseclass we're searching for. */ > + const char *m_name; > + > + /* Whether we're looking for a baseclass, or a field. */ > + const bool m_looking_for_baseclass; > + > + /* The offset of base class containing the field/baseclass we last > + recorded. */ Maybe "offset into base class"? > + LONGEST m_last_boffset = 0; > + > + /* If looking for a baseclass, then the result is stored here. */ > + struct value *m_baseclass = nullptr; > + > + /* When looking for fields, the found candidates are stored > + here. */ > + std::vector m_fields; > + > + /* The type of the initial type passed to search_struct_field; this > + is used for error reporting when the lookup is ambiguous. */ > + struct type *m_outermost_type; > + > + /* The full path to the struct being inspected. E.g. for field 'x' > + defined in class B inherited by class A, we have A and B pushed > + on the path. */ > + std::vector m_struct_path; > +}; > + > +void > +struct_field_searcher::update_result (struct value *v, LONGEST boffset) > { > if (v != NULL) > { > - if (*result_ptr != NULL > - /* The result is not ambiguous if all the classes that are > - found occupy the same space. */ > - && *last_boffset != boffset) > - error (_("base class '%s' is ambiguous in type '%s'"), > - name, TYPE_SAFE_NAME (type)); > - *result_ptr = v; > - *last_boffset = boffset; > + if (m_looking_for_baseclass) > + { > + if (m_baseclass != nullptr > + /* The result is not ambiguous if all the classes that are > + found occupy the same space. */ > + && m_last_boffset != boffset) > + error (_("base class '%s' is ambiguous in type '%s'"), > + m_name, TYPE_SAFE_NAME (m_outermost_type)); > + > + m_baseclass = v; > + m_last_boffset = boffset; > + } > + else > + { > + /* The field is not ambiguous if it occupies the same > + space. */ > + if (m_fields.empty () || m_last_boffset != boffset) > + m_fields.push_back ({m_struct_path, v}); > + } > } > } > > @@ -1795,25 +1875,25 @@ update_search_result (struct value **result_ptr, struct value *v, > search_struct_field; this is used for error reporting when the > lookup is ambiguous. */ > > -static void > -do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > - struct type *type, int looking_for_baseclass, > - struct value **result_ptr, > - LONGEST *last_boffset, > - struct type *outermost_type) > +void > +struct_field_searcher::search (struct value *arg1, LONGEST offset, > + struct type *type) > { > int i; > int nbases; > > + m_struct_path.push_back (type); > + SCOPE_EXIT { m_struct_path.pop_back (); }; > + > type = check_typedef (type); > nbases = TYPE_N_BASECLASSES (type); > > - if (!looking_for_baseclass) > + if (!m_looking_for_baseclass) > for (i = type->num_fields () - 1; i >= nbases; i--) > { > const char *t_field_name = TYPE_FIELD_NAME (type, i); > > - if (t_field_name && (strcmp_iw (t_field_name, name) == 0)) > + if (t_field_name && (strcmp_iw (t_field_name, m_name) == 0)) > { > struct value *v; > > @@ -1821,7 +1901,8 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > v = value_static_field (type, i); > else > v = value_primitive_field (arg1, offset, i, type); > - *result_ptr = v; > + > + update_result (v, offset); > return; > } > > @@ -1845,7 +1926,6 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > represented as a struct, with a member for each > . */ > > - struct value *v = NULL; > LONGEST new_offset = offset; > > /* This is pretty gross. In G++, the offset in an > @@ -1859,16 +1939,7 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > && TYPE_FIELD_BITPOS (field_type, 0) == 0)) > new_offset += TYPE_FIELD_BITPOS (type, i) / 8; > > - do_search_struct_field (name, arg1, new_offset, > - field_type, > - looking_for_baseclass, &v, > - last_boffset, > - outermost_type); > - if (v) > - { > - *result_ptr = v; > - return; > - } > + search (arg1, new_offset, field_type); > } > } > } > @@ -1880,10 +1951,10 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > /* If we are looking for baseclasses, this is what we get when > we hit them. But it could happen that the base part's member > name is not yet filled in. */ > - int found_baseclass = (looking_for_baseclass > + int found_baseclass = (m_looking_for_baseclass > && TYPE_BASECLASS_NAME (type, i) != NULL > - && (strcmp_iw (name, > - TYPE_BASECLASS_NAME (type, > + && (strcmp_iw (m_name, > + TYPE_BASECLASS_NAME (type, > i)) == 0)); > LONGEST boffset = value_embedded_offset (arg1) + offset; > > @@ -1924,28 +1995,17 @@ do_search_struct_field (const char *name, struct value *arg1, LONGEST offset, > if (found_baseclass) > v = v2; > else > - { > - do_search_struct_field (name, v2, 0, > - TYPE_BASECLASS (type, i), > - looking_for_baseclass, > - result_ptr, last_boffset, > - outermost_type); > - } > + search (v2, 0, TYPE_BASECLASS (type, i)); > } > else if (found_baseclass) > v = value_primitive_field (arg1, offset, i, type); > else > { > - do_search_struct_field (name, arg1, > - offset + TYPE_BASECLASS_BITPOS (type, > - i) / 8, > - basetype, looking_for_baseclass, > - result_ptr, last_boffset, > - outermost_type); > + search (arg1, offset + TYPE_BASECLASS_BITPOS (type, i) / 8, > + basetype); > } > > - update_search_result (result_ptr, v, last_boffset, > - boffset, name, outermost_type); > + update_result (v, boffset); > } > } > > @@ -1960,12 +2020,55 @@ static struct value * > search_struct_field (const char *name, struct value *arg1, > struct type *type, int looking_for_baseclass) > { > - struct value *result = NULL; > - LONGEST boffset = 0; > + struct_field_searcher searcher (name, type, looking_for_baseclass); > > - do_search_struct_field (name, arg1, 0, type, looking_for_baseclass, > - &result, &boffset, type); > - return result; > + searcher.search (arg1, 0, type); > + > + if (!looking_for_baseclass) > + { > + const auto &fields = searcher.fields (); > + > + if (fields.empty ()) > + return nullptr; > + else if (fields.size () > 1) > + { > + std::string candidates; > + > + for (auto &&candidate : fields) > + { > + gdb_assert (!candidate.path.empty ()); > + > + struct type *field_type = value_type (candidate.field_value); > + struct type *struct_type = candidate.path.back (); > + > + std::string path; > + bool first = true; > + for (struct type *t : candidate.path) > + { > + if (first) > + first = false; > + else > + path += " -> "; > + path += t->name (); > + } > + > + candidates += string_printf ("\n '%s %s::%s' (%s)", > + TYPE_SAFE_NAME (field_type), > + TYPE_SAFE_NAME (struct_type), > + name, > + path.c_str ()); > + } > + > + error (_("Request for member '%s' is ambiguous in type '%s'." > + " Candidates are:%s"), > + name, TYPE_SAFE_NAME (type), > + candidates.c_str ()); > + } > + else > + return fields[0].field_value; > + } > + else > + return searcher.baseclass (); > } > > /* Helper function used by value_struct_elt to recurse through > > base-commit: 8c51f2f291a5459e1eabd000b2c52e5de52b4c56 > Otherwise LGTM. I exercised this for aarch64-linux and see no more failures and 10 KFAIL's.