[PATCH] D126835: Round up zero-sized symbols to 1 byte in `.debug_aranges` (without breaking other logic).

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D126835#3583004 <https://reviews.llvm.org/D126835#3583004>, @dblaikie wrote:

> In D126835#3582639 <https://reviews.llvm.org/D126835#3582639>, @ayermolo wrote:
>
>> In D126835#3579779 <https://reviews.llvm.org/D126835#3579779>, @dblaikie wrote:
>>
>>> In D126835#3577753 <https://reviews.llvm.org/D126835#3577753>, @hans wrote:
>>>
>>>> When trying this on Chromium (in the context of https://crbug.com/1335630) we hit a densemap assert in llvm::DwarfDebug::emitDebugARanges()
>>>> See the attached reproducer.
>>>>
>>>> F23428761: string_piece_unittest-c72fb0.sh <https://reviews.llvm.org/F23428761>
>>>>
>>>> F23428765: string_piece_unittest-c72fb0.cpp <https://reviews.llvm.org/F23428765>
>>>
>>> Thanks for the reproducer, @hansw - sounds worth addressing before this moves forward.
>>>
>>> (I'm still sort of feeling like we should fix the underlying entities to be non-zero length always (even if they're zero length at the language level - they can still be non-zero length as an implementation detail providing unique addresses))
>>
>> Doesn't it rely on various feeders into llvm doing the right thing, or can this be done in some kind of language agnostic way?
>
> Possible it could require changes to producers if we wanted to make it invalid at the LLVM IR level to have a zero-length global variable (we already can't have a zero-length IR function - it has to have a terminator, even if it's "unreachable").
>
> But I don't think that's necessary/required - this could be implemented in the LLVM backend(s) (ideally as target-neutrally as possible) to emit a trap for any functions that end up not ending in a terminator instruction (including truly zero-length functions) (various slight variations of this support already exist in LLVM - but I wasn't able to stare at them long enough to get the particular variant I really wanted - but maybe that was a "perfect being the enemy of the good" situation and using one of the existing features (either the non-zero-length thing implemented for Windows (I think that just puts a zero rather than trap) or the "trap on unreachable" (that puts traps on even non-final unreachables, which is more pessimism than I was hoping for - but maybe low cost enough) that Sony and Apple use), extending it for global variables as well as for functions)

I'd still be highly encouraging of going somewhere in this ^ direction (& in a direction away from using .debug_aranges as well), but the patch looks OK for what it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126835