Page MenuHomePhabricator

Emit function alias to data as a function symbol
ClosedPublic

Authored by eugenis on Dec 3 2015, 3:48 PM.

Details

Reviewers
dblaikie
pcc
Summary

CFI emits jump slots for indirect functions as a byte array constant, and declares function-typed aliases to these constants.
This change fixes AsmPrinter to emit these aliases as function symbols, and not data symbols.

It does it by looking at the pointee type of the alias, which feels like the wrong thing to do. Does anyone have any better ideas?

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 41814.Dec 3 2015, 3:48 PM
eugenis retitled this revision from to Emit function alias to data as a function symbol.
eugenis updated this object.
eugenis added reviewers: pcc, dblaikie.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: llvm-commits.

s/SFI/CFI/

eugenis updated this revision to Diff 41817.Dec 3 2015, 3:53 PM
eugenis updated this object.
eugenis updated this revision to Diff 41819.Dec 3 2015, 3:55 PM
pcc edited edge metadata.Dec 3 2015, 4:05 PM

Eventually there should be a flag or attribute on the GlobalAlias that specifies the type of symbol to create. That can probably happen later though. For now the source of truth is the pointee type, and I am aware that we are using it elsewhere (e.g. in the COFF writer).

lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1192

This will always be true (GlobalValue::getType returns a PointerType *).

1194

Is this check needed?

eugenis updated this revision to Diff 41825.Dec 3 2015, 4:16 PM
eugenis edited edge metadata.
eugenis added inline comments.
lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1192

Good point. Removed.

1194

It's not strictly necessary, but without it, we would insert extra

.type alias,@function

(in .S output) when the aliasee is already a function.

pcc added inline comments.Dec 3 2015, 4:26 PM
lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1194

It shouldn't do any harm to emit that directive redundantly, so I'd prefer to remove it, as it makes the code easier to reason about.

eugenis updated this revision to Diff 41827.Dec 3 2015, 4:30 PM
eugenis added inline comments.
lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1194

sure, done.

pcc accepted this revision.Dec 3 2015, 4:33 PM
pcc edited edge metadata.

LGTM with nit

lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1190

Comment is no longer accurate.

This revision is now accepted and ready to land.Dec 3 2015, 4:33 PM
eugenis updated this revision to Diff 41830.Dec 3 2015, 4:41 PM
eugenis edited edge metadata.
eugenis closed this revision.Dec 3 2015, 4:49 PM

r254674

lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1190

fixed