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 https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
