Skip to content

Commit bdeb58d

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 185e6fa commit bdeb58d

File tree

4 files changed

+69
-0
lines changed

4 files changed

+69
-0
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,5 +807,14 @@ 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+
"forming %1 to a variable of type %0; this is likely incorrect because %2 may contain "
813+
"an object reference.", (Type, Type, Type))
814+
815+
WARNING(nontrivial_string_to_rawpointer_conversion,none,
816+
"forming %0 to an inout variable of type String exposes the internal representation "
817+
"rather than the string contents.", (Type))
818+
810819
#define UNDEFINE_DIAGNOSTIC_MACROS
811820
#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
@@ -5983,6 +5983,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
59835983
case KnownProtocolKind::SIMD:
59845984
case KnownProtocolKind::SIMDScalar:
59855985
case KnownProtocolKind::BinaryInteger:
5986+
case KnownProtocolKind::FixedWidthInteger:
59865987
case KnownProtocolKind::ObjectiveCBridgeable:
59875988
case KnownProtocolKind::DestructorSafeContainer:
59885989
case KnownProtocolKind::SwiftNewtypeWrapper:

lib/SILGen/SILGenExpr.cpp

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

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

5761+
diagnoseImplicitRawConversion(lv.getSubstFormalType(),
5762+
pointerInfo.PointerType, loc, *this);
5763+
57105764
// The incoming lvalue should be at the abstraction level of T in
57115765
// Unsafe*Pointer<T>. Reabstract it if necessary.
57125766
auto opaqueTy = AbstractionPattern::getOpaque();
57135767
auto loweredTy = getLoweredType(opaqueTy, lv.getSubstFormalType());
57145768
if (lv.getTypeOfRValue().getASTType() != loweredTy.getASTType()) {
57155769
lv.addSubstToOrigComponent(opaqueTy, loweredTy);
57165770
}
5771+
57175772
switch (pointerInfo.PointerKind) {
57185773
case PTK_UnsafeMutablePointer:
57195774
case PTK_UnsafePointer:
@@ -5827,6 +5882,9 @@ SILGenFunction::emitArrayToPointer(SILLocation loc, ManagedValue array,
58275882
firstSubMap, secondSubMap, CombineSubstitutionMaps::AtIndex, 1, 0,
58285883
genericSig);
58295884

5885+
diagnoseImplicitRawConversion(accessInfo.ArrayType, accessInfo.PointerType,
5886+
loc, *this);
5887+
58305888
SmallVector<ManagedValue, 2> resultScalars;
58315889
emitApplyOfLibraryIntrinsic(loc, converter, subMap, array, SGFContext())
58325890
.getAll(resultScalars);

0 commit comments

Comments
 (0)