Skip to content

Commit daed4b3

Browse files
committed
SIL: Stricter assertions for references from within fragile functions
Let's abuse the [thunk] attribute to mean "serialize the body of this function but only if it is referenced from a [fragile] function."
1 parent 06b1629 commit daed4b3

File tree

7 files changed

+36
-66
lines changed

7 files changed

+36
-66
lines changed

include/swift/SIL/SILFunction.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,10 @@ class SILFunction
325325
/// Set the function's linkage attribute.
326326
void setLinkage(SILLinkage linkage) { Linkage = unsigned(linkage); }
327327

328+
/// Returns true if this function can be referenced from a fragile function
329+
/// body.
330+
bool hasValidLinkageForFragileRef() const;
331+
328332
/// Get's the effective linkage which is used to derive the llvm linkage.
329333
/// Usually this is the same as getLinkage(), except in one case: if this
330334
/// function is a method in a class which has higher visibility than the

include/swift/SILOptimizer/Utils/Local.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,6 @@ bool tryCheckedCastBrJumpThreading(SILFunction *Fn, DominanceInfo *DT,
174174

175175
void recalcDomTreeForCCBOpt(DominanceInfo *DT, SILFunction &F);
176176

177-
/// Checks if a symbol with a given linkage can be referenced from fragile
178-
/// functions.
179-
bool isValidLinkageForFragileRef(SILLinkage linkage);
180-
181177
/// A structure containing callbacks that are called when an instruction is
182178
/// removed or added.
183179
struct InstModCallbacks {

lib/SIL/SILFunction.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,31 @@ bool SILFunction::hasName(const char *Name) const {
510510
return getName() == Name;
511511
}
512512

513-
/// Helper method which returns true if the linkage of the SILFunction
514-
/// indicates that the objects definition might be required outside the
515-
/// current SILModule.
513+
/// Returns true if this function can be referenced from a fragile function
514+
/// body.
515+
bool SILFunction::hasValidLinkageForFragileRef() const {
516+
// Fragile functions can reference other fragile functions.
517+
if (isFragile())
518+
return true;
519+
520+
// Fragile functions can reference 'static inline' functions imported
521+
// from C.
522+
if (hasForeignBody())
523+
return true;
524+
525+
// Fragile functions can reference non-fragile shared functions only
526+
// if they are thunks.
527+
SILLinkage linkage = getLinkage();
528+
if (hasSharedVisibility(linkage))
529+
return isThunk();
530+
531+
// Otherwise, only public functions can be referenced.
532+
return hasPublicVisibility(linkage);
533+
}
534+
535+
/// Helper method which returns true if the linkage of the SILFunction
536+
/// indicates that the objects definition might be required outside the
537+
/// current SILModule.
516538
bool
517539
SILFunction::isPossiblyUsedExternally() const {
518540
return swift::isPossiblyUsedExternally(getLinkage(),

lib/SIL/SILVerifier.cpp

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -902,35 +902,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
902902
verifyLLVMIntrinsic(BI, BI->getIntrinsicInfo().ID);
903903
}
904904

905-
bool isValidLinkageForFragileRef(SILLinkage linkage) {
906-
switch (linkage) {
907-
case SILLinkage::Private:
908-
case SILLinkage::PrivateExternal:
909-
case SILLinkage::Hidden:
910-
case SILLinkage::HiddenExternal:
911-
return false;
912-
913-
case SILLinkage::Shared:
914-
case SILLinkage::SharedExternal:
915-
// This allows fragile functions to reference thunks, generated
916-
// methods on Clang types, and optimizer specializations.
917-
return true;
918-
919-
case SILLinkage::Public:
920-
case SILLinkage::PublicExternal:
921-
return true;
922-
}
923-
}
924-
925-
bool isValidLinkageForFragileRef(const SILFunction *RefF) {
926-
if (RefF->isFragile() ||
927-
RefF->isExternalDeclaration()) {
928-
return true;
929-
}
930-
931-
return isValidLinkageForFragileRef(RefF->getLinkage());
932-
}
933-
934905
/// Returns true if \p FRI is only used as a callee and will always be
935906
/// inlined at those call sites.
936907
static bool isAlwaysInlined(FunctionRefInst *FRI) {
@@ -955,7 +926,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
955926
if (F.isFragile()) {
956927
SILFunction *RefF = FRI->getReferencedFunction();
957928
require(isAlwaysInlined(FRI)
958-
|| isValidLinkageForFragileRef(RefF),
929+
|| RefF->hasValidLinkageForFragileRef(),
959930
"function_ref inside fragile function cannot "
960931
"reference a private or hidden symbol");
961932
}
@@ -966,7 +937,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
966937
if (F.isFragile()) {
967938
SILGlobalVariable *RefG = AGI->getReferencedGlobal();
968939
require(RefG->isFragile()
969-
|| isValidLinkageForFragileRef(RefG->getLinkage()),
940+
|| hasPublicVisibility(RefG->getLinkage()),
970941
"alloc_global inside fragile function cannot "
971942
"reference a private or hidden symbol");
972943
}
@@ -982,7 +953,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
982953
if (F.isFragile()) {
983954
SILGlobalVariable *RefG = GAI->getReferencedGlobal();
984955
require(RefG->isFragile()
985-
|| isValidLinkageForFragileRef(RefG->getLinkage()),
956+
|| hasPublicVisibility(RefG->getLinkage()),
986957
"global_addr inside fragile function cannot "
987958
"reference a private or hidden symbol");
988959
}

lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,7 @@ bool swift::canDevirtualizeClassMethod(FullApplySite AI,
473473
if (AI.getFunction()->isFragile()) {
474474
// function_ref inside fragile function cannot reference a private or
475475
// hidden symbol.
476-
if (!(F->isFragile() || isValidLinkageForFragileRef(F->getLinkage()) ||
477-
F->isExternalDeclaration()))
476+
if (!F->hasValidLinkageForFragileRef())
478477
return false;
479478
}
480479

lib/SILOptimizer/Utils/Local.cpp

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1481,28 +1481,6 @@ optimizeBridgedObjCToSwiftCast(SILInstruction *Inst,
14811481
return (NewI) ? NewI : AI;
14821482
}
14831483

1484-
bool swift::isValidLinkageForFragileRef(SILLinkage linkage) {
1485-
switch (linkage) {
1486-
case SILLinkage::Private:
1487-
case SILLinkage::PrivateExternal:
1488-
case SILLinkage::Hidden:
1489-
case SILLinkage::HiddenExternal:
1490-
return false;
1491-
1492-
case SILLinkage::Shared:
1493-
case SILLinkage::SharedExternal:
1494-
// This handles some kind of generated functions, like constructors
1495-
// of clang imported types.
1496-
// TODO: check why those functions are not fragile anyway and make
1497-
// a less conservative check here.
1498-
return true;
1499-
1500-
case SILLinkage::Public:
1501-
case SILLinkage::PublicExternal:
1502-
return true;
1503-
}
1504-
}
1505-
15061484
/// Create a call of _bridgeToObjectiveC which converts an _ObjectiveCBridgeable
15071485
/// instance into a bridged ObjC type.
15081486
SILInstruction *
@@ -1568,9 +1546,7 @@ optimizeBridgedSwiftToObjCCast(SILInstruction *Inst,
15681546
"Implementation of _bridgeToObjectiveC could not be found");
15691547

15701548
if (Inst->getFunction()->isFragile() &&
1571-
!(BridgedFunc->isFragile() ||
1572-
isValidLinkageForFragileRef(BridgedFunc->getLinkage()) ||
1573-
BridgedFunc->isExternalDeclaration()))
1549+
!BridgedFunc->hasValidLinkageForFragileRef())
15741550
return nullptr;
15751551

15761552
auto ParamTypes = BridgedFunc->getLoweredFunctionType()->getParameters();

lib/Serialization/SerializeSIL.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,8 @@ void SILSerializer::addReferencedSILFunction(const SILFunction *F,
271271
// have enough information at the time we emit the function to
272272
// know if it should be marked fragile or not.
273273
if (F->getLinkage() == SILLinkage::Shared && !DeclOnly) {
274+
assert(F->isThunk() && "Reference to non-thunk, non-fragile shared "
275+
"function from a fragile function");
274276
FuncsToEmit[F] = false;
275277
Worklist.push_back(F);
276278
return;

0 commit comments

Comments
 (0)