[cfe-dev] [llvm-dev] Zero length function pointer equality

Fri Mar 19 15:31:10 PDT 2021

On Fri, Mar 19, 2021 at 3:00 PM James Y Knight <jyknight at google.com> wrote:

> I'm not sure whether we'd want *every* unreachable to emit a trap, but I
> do think we should try not to let code fall out of one function and into a
> completely unrelated one.
>

Yeah, that was my gut reaction too - though TrapUnreachable doesn't inhibit
SimplifyCFG from eliminating unreachable blocks (can still leave
unreachable in a block after a call - because the call may or may not
return).

> That is: I'd propose that the last basic-block in every function should
> get a trap instruction added unless it already ends in a control transfer
> instruction
>

That was my initial theory, but given MachO defaults to TrapUnreachable and
it doesn't inhibit SimplifyCFG, so most unreachable blocks do get
eliminated - I'm sort of leaning towards that being sufficiently correct.

> (jmp, ret, or branch -- call doesn't count, since it may return).
>

I'm inclined to omit the trap after a call to a noreturn function for
brevity - even though it does leave the possibility of violating the
noreturn contract leading to that fallthrough UB.

>
> On Fri, Mar 19, 2021 at 5:12 PM David Blaikie via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Just writing it down in this thread - this issue's been discussed a bit
>> in this bug: https://bugs.llvm.org/show_bug.cgi?id=49599
>>
>> And yeah, I'm considering adopting MachO's default (TrapUnreachable +
>> NoTrapOnNoreturn) as the default for LLVM (will require some design
>> discussion, no doubt) since it seems to capture most of the functionality
>> desired. Maybe there are some cases where we have extra unreachables that
>> could've otherwise been optimized away/elided, but hopefully nothing
>> drastic.
>>
>> (some platforms still need the trap-on-noreturn - Windows+AArch64 and
>> maybe Sony, etc - happy for some folks to opt into that). I wonder whether
>> TrapUnreachable shouldn't even be an option anymore though, if it becomes
>> load bearing for correctness - or should it become a fallback option - "no
>> trap unreachable" maybe means nop instead of trap, in case your target
>> can't handle a trap sometimes (I came across an issue with AMDGPU not being
>> able to add traps to functions that it isn't expecting - the function needs
>> some special attribute to have a trap in it - but I guess it can be updated
>> to add that attribute if the function has an unreachable in it (though then
>> it has to recreate the no-trap-on-noreturn handling too when deciding
>> whether to add the attribute... ))
>>
>> On Mon, Jul 27, 2020 at 9:20 AM Robinson, Paul <paul.robinson at sony.com>
>> wrote:
>>
>>>
>>>
>>> > -----Original Message-----
>>> > From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Hans
>>> > Wennborg via llvm-dev
>>> > Sent: Monday, July 27, 2020 9:11 AM
>>> > To: David Blaikie <dblaikie at gmail.com>
>>> > Cc: llvm-dev <llvm-dev at lists.llvm.org>; Clang Dev <
>>> cfe-dev at lists.llvm.org>
>>> > Subject: Re: [llvm-dev] [cfe-dev] Zero length function pointer equality
>>> >
>>> > On Sat, Jul 25, 2020 at 3:40 AM David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>> > >
>>> > > Looks perfect to me!
>>> > >
>>> > > well, a couple of questions: Why a noop, rather than int3/ud2/etc?
>>> >
>>> > Probably no reason.
>>>
>>> FTR there is TargetOptions.TrapUnreachable, which some targets turn on
>>> (for X86 it's on for MachO and PS4), this turns 'unreachable' into ud2.
>>> Clearly it covers more than "empty" functions but is probably the kind
>>> of thing you're looking for.
>>> --paulr
>>>
>>> >
>>> > > Might be worth using the existing code that places such an
>>> instruction
>>> > > when building at -O0?
>>> >
>>> > I wasn't aware of that. Does it happen for all functions (e.g. I think
>>> > I got pulled into this due to functions with the naked attribute)?
>>> >
>>> > > & you mention that this causes problems on Windows - but ICF done by
>>> > > the Windows linker does not cause such problems? (I'd have thought
>>> > > they'd result in the same situation - two functions described as
>>> being
>>> > > at the same address?) is there a quick summary of why those two cases
>>> > > turn out differently?
>>> >
>>> > The case that we hit was that the Control Flow Guard table of
>>> > addresses in the binary ended up listing the same address twice, which
>>> > the loader didn't expect. It may be that the linker took care to avoid
>>> > that for ICF (if two ICF'd functions got the same address, only list
>>> > it once in the CFG table) but still didn't handle the "empty function"
>>> > problem.
>>> >
>>> > > On Fri, Jul 24, 2020 at 6:17 AM Hans Wennborg <hans at chromium.org>
>>> wrote:
>>> > > >
>>> > > > Maybe we can just expand this to always apply:
>>> > https://reviews.llvm.org/D32330
>>> > > >
>>> > > > On Fri, Jul 24, 2020 at 2:46 AM David Blaikie via cfe-dev
>>> > > > <cfe-dev at lists.llvm.org> wrote:
>>> > > > >
>>> > > > > LLVM can produce zero length functions from cases like this (when
>>> > > > > optimizations are enabled):
>>> > > > >
>>> > > > > void f1() { __builtin_unreachable(); }
>>> > > > > int f2() { /* missing return statement */ }
>>> > > > >
>>> > > > > This code is valid, so long as the functions are never called.
>>> > > > >
>>> > > > > I believe C++ requires that all functions have a distinct address
>>> > (ie:
>>> > > > > &f1 != &f2) and LLVM optimizes code on this basis (assert(f1 ==
>>> f2)
>>> > > > > gets optimized into an unconditional assertion failure)
>>> > > > >
>>> > > > > But these zero length functions can end up with identical
>>> addresses.
>>> > > > >
>>> > > > > I'm unaware of anything in the C++ spec (or the LLVM langref)
>>> that
>>> > > > > would indicate that would allow distinct functions to have
>>> identical
>>> > > > > addresses - so should we do something about this in the LLVM
>>> > backend?
>>> > > > > add a little padding? a nop instruction? (if we're adding an
>>> > > > > instruction anyway, perhaps we might as well make it an int3?)
>>> > > > >
>>> > > > > (I came across this due to DWARF issues with zero length
>>> functions &
>>> > > > > thinking about if/how this should be supported)
>>> > > > > _______________________________________________
>>> > > > > cfe-dev mailing list
>>> > > > > cfe-dev at lists.llvm.org
>>> > > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>> > _______________________________________________
>>> > LLVM Developers mailing list
>>> > llvm-dev at lists.llvm.org
>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210319/51d64da2/attachment.html>