Page MenuHomePhabricator

Add an option to allow JumpInstrTables to set unnamed_addr and jumptable on all address-taken functions
AbandonedPublic

Authored by tmroeder on Jun 12 2014, 1:52 PM.

Details

Reviewers
None
Summary

This small patch adds a new command flag -jump-table-all and an associated TargetOptions field JumpTableAll, and it modifies JumpInstrTables to add unnamed_addr and jumptable to all address-taken functions if the flag is set.

This is important for applications like my implementation of control-flow integrity, which want to operate over all address-taken functions and not just the ones that happened to have been marked jumptable.

Diff Detail

Event Timeline

tmroeder updated this revision to Diff 10371.Jun 12 2014, 1:52 PM
tmroeder retitled this revision from to Add an option to allow JumpInstrTables to set unnamed_addr and jumptable on all address-taken functions.
tmroeder updated this object.
tmroeder edited the test plan for this revision. (Show Details)
tmroeder set the repository for this revision to rL LLVM.
tmroeder added a subscriber: Unknown Object (MLST).
jfb added a subscriber: jfb.Jun 30 2014, 9:56 AM
jfb added inline comments.
include/llvm/CodeGen/CommandFlags.h
223

Say that unnamed_addr and jumptable are attributes that you're adding.

lib/CodeGen/JumpInstrTables.cpp
271

You don't need the nullptr here, it's the default value already.

test/CodeGen/X86/jump_table_all.ll
33

It's better to use CHECK-NEXT above. As-is, the g@PLT could be before f@PLT and the test would pass.

Should you also check for the absence of other __llvm_jump_instr_table_* ? Or is that covered in other tests?

Thanks for the comments. After the recent discussion with Nick about this issue on llvm-commits, I'm dropping -jump-table-all and will submit a patch to clang that adds support for an -ffcfi flag that will add unnamed_addr and jumptable to all functions instead. Please see D4167 for the FCFI LLVM side.

tmroeder abandoned this revision.Jul 10 2014, 2:44 PM

Abandoning after it was decided that this problem would be address by a flag in clang.

amanone added a subscriber: amanone.Nov 6 2014, 6:35 AM