[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL313096: [ubsan] Function Sanitizer: Don't require writable text segments (authored by vedantk). Changed prior to commit: https://reviews.llvm.org/D37597?vs=114932&id=114943#toc Repository: rL LLVM h

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk marked 4 inline comments as done. vsk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:461 + ? EncodedAddr + : Builder.CreateSExt(EncodedAddr, IntPtrTy); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy,

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: lib/CodeGen/CodeGenFunction.cpp:461 + ? EncodedAddr + : Builder.CreateSExt(EncodedAddr, IntPtrTy); + auto

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:445 + auto *GV = new llvm::GlobalVariable(CGM.getModule(), Addr->getType(), + /*isConstant=*/false, + llvm::GlobalValue::PrivateLinkag

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 114932. vsk marked 8 inline comments as done. vsk added a comment. Address review feedback. https://reviews.llvm.org/D37597 Files: lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/TargetInfo.cpp test/Cod

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:445 + auto *GV = new llvm::GlobalVariable(CGM.getModule(), Addr->getType(), + /*isConstant=*/false, + llvm::GlobalValue::PrivateLinkag

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-12 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:463 + auto *PCRelAsInt = + Builder.CreatePtrToInt(EncodedAddr, IntPtrTy, "encoded_addr.int"); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy, "func_addr.int"); vsk wrote: > pcc

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-07 Thread Vedant Kumar via Phabricator via cfe-commits
vsk updated this revision to Diff 114303. vsk edited the summary of this revision. vsk added a comment. Address review feedback: - Enable this change on all platforms. - Bump the function signature version. - Store the indirect RTTI pointer as an IntPtrTy. This saves a "ptrtoint" instruction. Tr

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-07 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. Thanks for your comments! Comment at: lib/CodeGen/CodeGenFunction.cpp:434 +llvm::Constant *Addr) { + if (!CGM.getTriple().isOSDarwin()) +return Addr; pcc wrote: > I think you can just do this

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-07 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:463 + auto *PCRelAsInt = + Builder.CreatePtrToInt(EncodedAddr, IntPtrTy, "encoded_addr.int"); + auto *FuncAsInt = Builder.CreatePtrToInt(F, IntPtrTy, "func_addr.int"); pcc wrote: > Mayb

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-07 Thread Peter Collingbourne via Phabricator via cfe-commits
pcc added a comment. Thanks. Once we apply this fix to other platforms this would seem to fix PR17633. We may also want to change the ubsan function signature in order to avoid bad pointer reads in case of version mismatches. Comment at: lib/CodeGen/CodeGenFunction.cpp:434 +

[PATCH] D37597: [ubsan] Function Sanitizer: Don't require writable text segments

2017-09-07 Thread Vedant Kumar via Phabricator via cfe-commits
vsk created this revision. This change will make it possible to use -fsanitize=function on Darwin and possibly on other platforms. It fixes an issue with the way RTTI is stored into function prologue data. On Darwin, addresses stored in prologue data can't require run-time fixups and must be PC-r