[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

Sat Apr 24 16:40:39 PDT 2021

dblaikie added a comment.

In D101237#2714962 <https://reviews.llvm.org/D101237#2714962>, @jankratochvil wrote:

> In D101237#2714953 <https://reviews.llvm.org/D101237#2714953>, @dblaikie wrote:
>
>> Given that no_unique_address changes the layout of data structures ( https://clang.llvm.org/docs/AttributeReference.html#no-unique-address ) adding it to a member that doesn't actually have it specified I believe would break layout of structures
>
> Not so because LLDB sends `FieldOffsets` (from `DW_AT_data_member_location`s) to clang. I believe it should work but then I understand it is a bit bold change. clang is already forced for the DWARF-compliant layout by `FieldOffsets`.
>
> Before this patch LLDB still sends `FieldOffsets` to clang but clang wants a larger layout because it does not see the `[[no_unique_address]]` attribute. And therefore it asserts then because the clang offsets cannot fit the `FieldOffsets` from `DW_AT_data_member_location`s.
>
> Thanks for this possible catch but I hope the patch does solve that.
>
>> (have you tried it on the lldb test suite/experimented with examples? I would expect a lot of tests to fail if I'm understanding this correctly)
>
> Sure the clang+llvm+lldb testsuites do PASS with this patch.  (Otherwise I would not send this patch for approval, at least I would mark it by RFC/WIP which I did not.)

Fair enough - I'd be a bit worried - but if it all seems to work, I don't have any further knowledge to suggest it's invalid given the offsets are derived from the authoritative offsets given in the DWARF.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101237/new/

https://reviews.llvm.org/D101237