Skip to content

Commit 778b773

Browse files
committed
Warn on implicit pointer conversion from nontrivial inout values.
Fixes a usability problem with implicit inout conversion to raw pointers. For example, supporting this conversion turns out to be bad: void read_void(const void *input); func foo(data: inout Data) { read_void(&data) } People understandably expect Foundation.Data to have the same sort of implicit conversion as Array. But it does something very wrong instead. We could have added an an attribute to Data and other copy-on-write containers to selectively suppress implicit conversion. But there is no good reason to allow implicit conversion from any non-trivial type. It is extremely dangerous, and almost always accidental. Note that this problem becomes worse now that the compiler views imported `char *` arguments as raw pointers. For example: void read_char(const char *input); var object: AnyObject = ... read_void(&object1) This seems like a good time to correct this old Swift 3 behavior. Plan: Add a warning now. Convert it to an error in Swift 6 language mode based on feedback. Fixes rdar://97963116 (It's really easy to accidentally corrupt a Data object with the & operator)
1 parent af36fa7 commit 778b773

File tree

4 files changed

+70
-0
lines changed

4 files changed

+70
-0
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,5 +807,15 @@ ERROR(sil_movekillscopyablevalue_move_applied_to_unsupported_move, none,
807807
"_move applied to value that the compiler does not support checking",
808808
())
809809

810+
// Implicit inout-to-UnsafeRawPointer conversions
811+
WARNING(nontrivial_to_rawpointer_conversion,none,
812+
"cannot implicitly convert an inout value of type %0 to expected "
813+
"argument type %1 because %2 may contain an object reference.",
814+
(Type, Type, Type))
815+
816+
WARNING(nontrivial_string_to_rawpointer_conversion,none,
817+
"cannot implicitly convert an inout String to expected argument type %0.",
818+
(Type))
819+
810820
#define UNDEFINE_DIAGNOSTIC_MACROS
811821
#include "DefineDiagnosticMacros.h"

include/swift/AST/KnownProtocols.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ PROTOCOL(CaseIterable)
7474
PROTOCOL(SIMD)
7575
PROTOCOL(SIMDScalar)
7676
PROTOCOL(BinaryInteger)
77+
PROTOCOL(FixedWidthInteger)
7778
PROTOCOL(RangeReplaceableCollection)
7879
PROTOCOL(SerialExecutor)
7980
PROTOCOL(GlobalActor)

lib/IRGen/GenMeta.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5832,6 +5832,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
58325832
case KnownProtocolKind::SIMD:
58335833
case KnownProtocolKind::SIMDScalar:
58345834
case KnownProtocolKind::BinaryInteger:
5835+
case KnownProtocolKind::FixedWidthInteger:
58355836
case KnownProtocolKind::ObjectiveCBridgeable:
58365837
case KnownProtocolKind::DestructorSafeContainer:
58375838
case KnownProtocolKind::SwiftNewtypeWrapper:

lib/SILGen/SILGenExpr.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5691,19 +5691,74 @@ RValue RValueEmitter::visitInOutToPointerExpr(InOutToPointerExpr *E,
56915691
return RValue(SGF, E, ptr);
56925692
}
56935693

5694+
/// Implicit conversion from a nontrivial inout type to a raw pointer are
5695+
/// dangerous. For example:
5696+
///
5697+
/// func bar(_ p: UnsafeRawPointer) { ... }
5698+
/// func foo(object: inout AnyObject) {
5699+
/// bar(&object)
5700+
/// }
5701+
///
5702+
/// These conversions should be done explicitly.
5703+
///
5704+
static void diagnoseImplicitRawConversion(Type sourceTy, Type pointerTy,
5705+
SILLocation loc,
5706+
SILGenFunction &SGF) {
5707+
// Array conversion does not always go down the ArrayConverter
5708+
// path. Recognize the Array source type here both for ArrayToPointer and
5709+
// InoutToPointer cases and diagnose on the element type.
5710+
Type eltTy = sourceTy->isArrayType();
5711+
if (!eltTy)
5712+
eltTy = sourceTy;
5713+
5714+
if (SGF.getLoweredType(eltTy).isTrivial(SGF.F))
5715+
return;
5716+
5717+
auto *SM = SGF.getModule().getSwiftModule();
5718+
if (auto *fixedWidthIntegerDecl = SM->getASTContext().getProtocol(
5719+
KnownProtocolKind::FixedWidthInteger)) {
5720+
if (SM->conformsToProtocol(eltTy, fixedWidthIntegerDecl))
5721+
return;
5722+
}
5723+
5724+
PointerTypeKind kindOfPtr;
5725+
auto pointerElt = pointerTy->getAnyPointerElementType(kindOfPtr);
5726+
assert(!pointerElt.isNull() && "expected an unsafe pointer type");
5727+
5728+
// The element type may contain a reference. Disallow conversion to a "raw"
5729+
// pointer type. Consider Int8/UInt8 to be raw pointers. Trivial element types
5730+
// are filtered out above, so Int8/UInt8 pointers can't match the source
5731+
// type. But the type checker may have allowed these for direct C calls, in
5732+
// which Int8/UInt8 are equivalent to raw pointers..
5733+
if (!(pointerElt->isVoid() || pointerElt->isInt8() || pointerElt->isUInt8()))
5734+
return;
5735+
5736+
if (sourceTy->isString()) {
5737+
SGF.SGM.diagnose(loc, diag::nontrivial_string_to_rawpointer_conversion,
5738+
pointerTy);
5739+
} else {
5740+
SGF.SGM.diagnose(loc, diag::nontrivial_to_rawpointer_conversion, sourceTy,
5741+
pointerTy, eltTy);
5742+
}
5743+
}
5744+
56945745
/// Convert an l-value to a pointer type: unsafe, unsafe-mutable, or
56955746
/// autoreleasing-unsafe-mutable.
56965747
ManagedValue SILGenFunction::emitLValueToPointer(SILLocation loc, LValue &&lv,
56975748
PointerAccessInfo pointerInfo) {
56985749
assert(pointerInfo.AccessKind == lv.getAccessKind());
56995750

5751+
diagnoseImplicitRawConversion(lv.getSubstFormalType(),
5752+
pointerInfo.PointerType, loc, *this);
5753+
57005754
// The incoming lvalue should be at the abstraction level of T in
57015755
// Unsafe*Pointer<T>. Reabstract it if necessary.
57025756
auto opaqueTy = AbstractionPattern::getOpaque();
57035757
auto loweredTy = getLoweredType(opaqueTy, lv.getSubstFormalType());
57045758
if (lv.getTypeOfRValue().getASTType() != loweredTy.getASTType()) {
57055759
lv.addSubstToOrigComponent(opaqueTy, loweredTy);
57065760
}
5761+
57075762
switch (pointerInfo.PointerKind) {
57085763
case PTK_UnsafeMutablePointer:
57095764
case PTK_UnsafePointer:
@@ -5817,6 +5872,9 @@ SILGenFunction::emitArrayToPointer(SILLocation loc, ManagedValue array,
58175872
firstSubMap, secondSubMap, CombineSubstitutionMaps::AtIndex, 1, 0,
58185873
genericSig);
58195874

5875+
diagnoseImplicitRawConversion(accessInfo.ArrayType, accessInfo.PointerType,
5876+
loc, *this);
5877+
58205878
SmallVector<ManagedValue, 2> resultScalars;
58215879
emitApplyOfLibraryIntrinsic(loc, converter, subMap, array, SGFContext())
58225880
.getAll(resultScalars);

0 commit comments

Comments
 (0)