[Lldb-commits] [PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source
Raphael Isemann via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Sun Jul 5 04:16:16 PDT 2020
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Thanks for tracking this down, this is a really nasty bug...
The fix itself is obviously fine, but I think I'm out of the loop regarding the testing strategy. We talked about adding a Clang test for this with the help of this layout overwrite JSON file. I assume that extending this to cover virtual bases turned out to be more complicated than expected? FWIW, I'm not necessarily the biggest fan of this Clang test option so I would be fine if we leave it as-is.
I think having an LLDB test is a good idea, but it's not clear why it's a Shell test. If I understand correctly this test requires running on arm64e (so, a remote test target), but from what I remember all the remote platform support is only in dotest.py? Also pretty much all other expression evaluation tests and the utilities for that are in the Python/API test infrastructure, so it's a bit out of place.
Also I think the test can be in general much simpler than an arm64-specific test. We get all base class offsets wrong in LLDB, so we can just write a simple test where you change the layout of some structs in a way that it doesn't fit the default layout. E.g., just putting `alignas` on a base class and then reading fields should be enough to trigger the same bug.
CHANGES SINCE LAST ACTION
More information about the lldb-commits