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

Tue Sep 7 15:51:55 PDT 2021

No, I haven't managed to make progress on this - I think I kept some notes
in a bug somewhere... (Hans you can look up b/179837110).

TrapUnreachable does sound like about the right thing - I think I hit a few
kinks in making that the default. (I'd like the zero length function issue
to be solved generically - across all targets, I don't think it's helpful
to allow it in any case due to the DWARF issues, the C++ issues, the
debuggability issues (falling off into another function rather than
trapping), compared to any small benefit to code size it might provide) -
maybe some targets (AMDGPU?) couldn't handle it because they can't emit a
trap into an arbitrary function (some function attribute was required for
that, I think?)

Oh, hmm, maybe I hit the AMDGPU issue when trying to do something more
manually targeting only the end of function case, not the trap-unreachable
broader feature. (Looking back at my various branches I see an AMDGPU
workaround/fixme in my "trap at end of function" branch, not in my
trap_unreachable branch)

On Tue, Sep 7, 2021 at 7:48 AM <paul.robinson at sony.com> wrote:

> I’m not aware of anyone picking this up, but then I haven’t been watching
> for it.
>
> LLVMTargetMachine supports a command-line “-trap-unreachable” so the Linux
> build could turn it on universally that way.  Some targets already have it
> turned on, and are willing to take the (fairly small IIRC) size hit.  Note
> that the flag tends to be set in target-specific code but the effect is
> implemented in generic code, so it should Just Work.  If Linux doesn’t like
> the size hit, then someone will have to implement one of the more targeted
> ideas, e.g., add a nop/trap only after the last block of a function
> (ideally but not necessarily omitting that if the function ends with a ret
> or unconditional branch).
>
> I’ll admit that Sony would benefit from the more targeted feature, as
> that’s the exact use-case we need, but we haven’t deemed the size cost of
> TrapUnreachable to be enough to warrant the additional effort.
>
> --paulr
>
>
>
> *From:* Hans Wennborg <hans at chromium.org>
> *Sent:* Tuesday, September 7, 2021 8:24 AM
> *To:* David Blaikie <dblaikie at gmail.com>; Nick Desaulniers <
> ndesaulniers at google.com>
> *Cc:* Robinson, Paul <paul.robinson at sony.com>; Clang Dev <
> cfe-dev at lists.llvm.org>; llvm-dev at lists.llvm.org
> *Subject:* Re: [llvm-dev] [cfe-dev] Zero length function pointer equality
>
>
>
> Did anything come of this?
>
>
>
> The Linux folks (cc Nick) keep running into issues where a function which
> ends with unreachable can fall through to an unrelated function, which
> confuses some machine code analyser they run (e.g.
> https://github.com/ClangBuiltLinux/linux/issues/1440
> <https://urldefense.com/v3/__https:/github.com/ClangBuiltLinux/linux/issues/1440__;!!JmoZiZGBv3RvKRSx!qYlbY1j6iDE6WGMU8OKnc3po09Ff8PvUduMgCzcCRcUv0ANZnBSJyDIWOhSKiqWzsg$>
> ).
>
>
>
> It seems TrapUnreachable would avoid such issues.
>
>
>
> On Fri, Mar 19, 2021 at 10:11 PM David Blaikie <dblaikie at gmail.com> 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
> <https://urldefense.com/v3/__https:/bugs.llvm.org/show_bug.cgi?id=49599__;!!JmoZiZGBv3RvKRSx!qYlbY1j6iDE6WGMU8OKnc3po09Ff8PvUduMgCzcCRcUv0ANZnBSJyDIWOhRT6p37JQ$>
>
> 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
> <https://urldefense.com/v3/__https:/reviews.llvm.org/D32330__;!!JmoZiZGBv3RvKRSx!qYlbY1j6iDE6WGMU8OKnc3po09Ff8PvUduMgCzcCRcUv0ANZnBSJyDIWOhRnihf1DQ$>
> > > >
> > > > 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
> <https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev__;!!JmoZiZGBv3RvKRSx!qYlbY1j6iDE6WGMU8OKnc3po09Ff8PvUduMgCzcCRcUv0ANZnBSJyDIWOhTTLrIurw$>
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> <https://urldefense.com/v3/__https:/lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev__;!!JmoZiZGBv3RvKRSx!qYlbY1j6iDE6WGMU8OKnc3po09Ff8PvUduMgCzcCRcUv0ANZnBSJyDIWOhRPBWWW9A$>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210907/01e8ff67/attachment.html>