similar to: [RFC] CFI for indirect calls with ThinLTO

On Mon, May 15, 2017 at 6:44 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > Thanks for sending this out. A few comments below. > > On Mon, May 15, 2017 at 5:17 PM, Evgenii Stepanov via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >> >> Hi, >> >> this is a proposal for the implementation of CFI-icall [1] with ThinLTO. >> >>

On Tue, May 23, 2017 at 4:39 PM, Evgenii Stepanov <eugeni.stepanov at gmail.com > wrote: > On Tue, May 16, 2017 at 4:33 PM, Evgenii Stepanov > <eugeni.stepanov at gmail.com> wrote: > > On Mon, May 15, 2017 at 6:44 PM, Peter Collingbourne <peter at pcc.me.uk> > wrote: > >> Thanks for sending this out. A few comments below. > >> > >> On

The problem as I recall was that we do not want jumptable sections to appear before __cfi_check, so we gave these jump tables a name so that the bfd default linker script would sort them after __cfi_check. This is the relevant part of the default linker script: .text : { [...] *(.text.hot .text.hot.*) *(.text .stub .text.* .gnu.linkonce.t.*) } So I guess the idea was

Taking out that line breaks some LowerTypeTest tests, need to investigate. Runtime compiler-rt all passed. > On May 1, 2018, at 3:11 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > > The problem as I recall was that we do not want jumptable sections to appear before __cfi_check, so we gave these jump tables a name so that the bfd default linker script would sort them after

Jump table sections already have this prefix: ".text..L.cfi.jumptable.”, without forcing the name with setSection(). Is that good enough? > On May 1, 2018, at 1:16 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > > I think we just need to change the section name for non-MachO object formats because "__TEXT,__text,regular,pure_instructions" is the default text

I think we just need to change the section name for non-MachO object formats because "__TEXT,__text,regular,pure_instructions" is the default text section name in MachO. There seems to be a way to get the ".cfi" suffix on section names that doesn't involve setting a custom section name (which would invoke the "everything goes in the same section" behaviour), and

That seems to do the trick. I get “symbol ordering file: no such symbol: <blah>” warnings for symbols that didn’t get renamed. Maybe we'll have a separate order file for CFI builds... What about the comment above that line? > On May 1, 2018, at 11:46 AM, Peter Collingbourne <peter at pcc.me.uk> wrote: > > On Tue, May 1, 2018 at 11:29 AM, <dmitry.mikulin at

On Tue, May 1, 2018 at 11:29 AM, <dmitry.mikulin at sony.com> wrote: > It’s not the functions themselves, it’s the jump tables that end up in the > same section of the thin-link temporary object. If I have, say, foo and bar > and both of them need a jump table entry, both entries will be in the same > section of the temp object. The real functions will be renamed to foo.cfi >

It’s not the functions themselves, it’s the jump tables that end up in the same section of the thin-link temporary object. If I have, say, foo and bar and both of them need a jump table entry, both entries will be in the same section of the temp object. The real functions will be renamed to foo.cfi and bar.cfi. If I list “foo.cfi, foo, bar.cfi, bar” in the link order file, the linker (I use lld)

On Mon, Apr 30, 2018 at 1:05 PM, <dmitry.mikulin at sony.com> wrote: > Replacing direct calls to jump table entries with calls to real targets > recovers most of the performance loss in my benchmark. Dealing with link > order files is a bit cumbersome though: I can’t keep both, say “foo” and > “foo.cfi” next to each other in link order, because if both exist, the > linker

On Tue, May 1, 2018 at 11:10 AM, <dmitry.mikulin at sony.com> wrote: > > > > On Apr 30, 2018, at 6:04 PM, Peter Collingbourne <peter at pcc.me.uk> > wrote: > > > > On Mon, Apr 30, 2018 at 1:05 PM, <dmitry.mikulin at sony.com> wrote: > > Replacing direct calls to jump table entries with calls to real targets > recovers most of the performance

> On Apr 30, 2018, at 6:04 PM, Peter Collingbourne <peter at pcc.me.uk> wrote: > > On Mon, Apr 30, 2018 at 1:05 PM, <dmitry.mikulin at sony.com> wrote: > Replacing direct calls to jump table entries with calls to real targets recovers most of the performance loss in my benchmark. Dealing with link order files is a bit cumbersome though: I can’t keep both, say “foo” and

Hi Peter, We could probably tolerate a certain amount of unused jump table entries. However, I just realized that all non-inline imported calls end up going through a jump table entry. Is that correct? Initially I thought you meant calls promoted from indirect. While this can be fixed by replacing direct calls to jump tables with direct calls to real targets, I found other cases where ThinLTO+CFI

> We could probably tolerate a certain amount of unused jump table entries. However, I just realized that all non-inline imported calls end up going through a jump table entry. Is that correct? In fact it is all calls that go through a function pointer type that is used anywhere in the program for an indirect call, but depending on your program that could be very close to "yes".

For the test case below, I get the following IR for main() on entry to ThinLTO backend invocation of LowerTypeTestsModule::lower(). This is after I moved this pass down in the pipeline so it’s invoked after inlining. The declarations for foo() and bar() are read in at the time of module import, Importer.importFunctions() in lto::thinBackend(). They do not have type metadata attached to them. In

I’m looking at this problem again. In ThinLTO backend, the types of symbols like foo() and bar() get constructed from CfiFunctionDefs and CfiFunctionDelcs of the summary. The summary does not have the type metadata for these. That’s why we can’t do type matching in LowerTypeTestsModule to potentially eliminate type checks. Would it be reasonable to add type metadata to the CFI part of the summary

Replacing direct calls to jump table entries with calls to real targets recovers most of the performance loss in my benchmark. Dealing with link order files is a bit cumbersome though: I can’t keep both, say “foo” and “foo.cfi” next to each other in link order, because if both exist, the linker reorders the jump table next to the real function. This is not what we want when the goal is to get rid

Hi all, As promised, here is a brain dump on how I see CFI for vcalls working under ThinLTO. Most of this has been prototyped, so the design does appear to be sound. For context on how CFI currently works under regular LTO, please read: http://llvm.org/docs/TypeMetadata.html http://clang.llvm.org/docs/ControlFlowIntegrityDesign.html http://clang.llvm.org/docs/LTOVisibility.html ==== Summary

Hi Dmitry, the direct call patch seems like a good start; however, by falling through to the F->replaceUsesExceptBlockAddr(FDecl) at the bottom of LowerTypeTestsModule::importFunction() I believe it will replace all uses of a function (for a definition outside the TU it's defined in) with the direct function reference instead of just direct calls. Also, I believe the logic for

Regarding the orderfile, yes, I was thinking more about ordering the real functions. In that case it sounds like your best option may be to implement the optimization pass to make direct calls go directly to the real function. >From a performance perspective I don't think it would make much difference if there are unused jump table entries. Peter On Thu, Apr 19, 2018 at 6:09 PM, via