From 22979b9a7a8e16f9b42ffc8edf692d6a5b87acbc Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 9 Oct 2023 17:32:51 +0200 Subject: [PATCH 1/2] fix an uninitialized pointer in BridgedSourceLoc It's so easy to run into undefined behavior in C++ --- SwiftCompilerSources/Sources/Basic/SourceLoc.swift | 4 ++-- include/swift/Basic/BasicBridging.h | 14 ++++++++++++-- lib/AST/ASTBridging.cpp | 4 ++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/SwiftCompilerSources/Sources/Basic/SourceLoc.swift b/SwiftCompilerSources/Sources/Basic/SourceLoc.swift index 9f831ec3e0f0f..a82baf8d2fc35 100644 --- a/SwiftCompilerSources/Sources/Basic/SourceLoc.swift +++ b/SwiftCompilerSources/Sources/Basic/SourceLoc.swift @@ -27,11 +27,11 @@ public struct SourceLoc { guard bridged.isValid() else { return nil } - self.locationInFile = bridged.opaquePointer! + self.locationInFile = bridged.uint8Pointer()! } public var bridged: BridgedSourceLoc { - .init(opaquePointer: locationInFile) + .init(locationInFile) } } diff --git a/include/swift/Basic/BasicBridging.h b/include/swift/Basic/BasicBridging.h index 3ef3ac83913b2..8988bb75eb329 100644 --- a/include/swift/Basic/BasicBridging.h +++ b/include/swift/Basic/BasicBridging.h @@ -88,9 +88,19 @@ class BridgedOwnedString { void destroy() const; }; -struct BridgedSourceLoc { - const uint8_t * _Nullable opaquePointer; +class BridgedSourceLoc { + const void * _Nullable opaquePointer; +public: + BridgedSourceLoc() : opaquePointer(nullptr) {} + BridgedSourceLoc(const void * _Nullable loc) : opaquePointer(loc) {} + bool isValid() const { return opaquePointer != nullptr; } + SWIFT_IMPORT_UNSAFE const uint8_t * _Nullable uint8Pointer() const { + return (const uint8_t * _Nullable)opaquePointer; + } + const char * _Nullable getLoc() const { + return (const char * _Nullable)opaquePointer; + } }; struct BridgedArrayRef { diff --git a/lib/AST/ASTBridging.cpp b/lib/AST/ASTBridging.cpp index 946cd335d2b6d..a6d8a42ab3d2e 100644 --- a/lib/AST/ASTBridging.cpp +++ b/lib/AST/ASTBridging.cpp @@ -38,7 +38,7 @@ static_assert(sizeof(BridgedDiagnosticFixIt) >= sizeof(DiagnosticInfo::FixIt), "BridgedDiagnosticArgument has wrong size"); static SourceLoc getSourceLoc(BridgedSourceLoc bridgedLoc) { - return SourceLoc(llvm::SMLoc::getFromPointer((const char *)bridgedLoc.opaquePointer)); + return SourceLoc(llvm::SMLoc::getFromPointer(bridgedLoc.getLoc())); } BridgedDiagnosticFixIt::BridgedDiagnosticFixIt(BridgedSourceLoc start, uint32_t length, BridgedStringRef text) @@ -60,7 +60,7 @@ void DiagnosticEngine_diagnose( for (auto arg : getArrayRef(bridgedArguments)) { arguments.push_back(arg.get()); } - auto inflight = D->diagnose(SourceLoc(llvm::SMLoc::getFromPointer((const char *)loc.opaquePointer)), diagID, arguments); + auto inflight = D->diagnose(SourceLoc(llvm::SMLoc::getFromPointer(loc.getLoc())), diagID, arguments); // Add highlight. if (highlightStart.isValid()) { From f57bc648521b4fecfc76e9a4957f281bbdf40cd3 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 9 Oct 2023 17:43:22 +0200 Subject: [PATCH 2/2] Builder Bridging: some refactoring to simplify the code NFC --- include/swift/SIL/SILBridging.h | 18 +++++ include/swift/SIL/SILBridgingImpl.h | 111 ++++++++++++---------------- 2 files changed, 66 insertions(+), 63 deletions(-) diff --git a/include/swift/SIL/SILBridging.h b/include/swift/SIL/SILBridging.h index 86d9100d115a4..fae3dd666023c 100644 --- a/include/swift/SIL/SILBridging.h +++ b/include/swift/SIL/SILBridging.h @@ -21,6 +21,7 @@ #ifdef USED_IN_CPP_SOURCE #include "llvm/ADT/ArrayRef.h" +#include "swift/SIL/SILBuilder.h" #include "swift/SIL/SILInstruction.h" #include "swift/SIL/SILWitnessTable.h" #endif @@ -769,6 +770,23 @@ struct BridgedBuilder{ SwiftObject insertionObj; BridgedLocation loc; + +#ifdef USED_IN_CPP_SOURCE + swift::SILBuilder get() const { + switch (insertAt) { + case BridgedBuilder::InsertAt::beforeInst: + return swift::SILBuilder(BridgedInstruction(insertionObj).get(), loc.getLoc().getScope()); + case BridgedBuilder::InsertAt::endOfBlock: + return swift::SILBuilder(BridgedBasicBlock(insertionObj).get(), loc.getLoc().getScope()); + case BridgedBuilder::InsertAt::intoGlobal: + return swift::SILBuilder(BridgedGlobalVar(insertionObj).getGlobal()); + } + } + swift::SILLocation regularLoc() const { + return swift::RegularLocation(loc.getLoc().getLocation()); + } +#endif + SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedInstruction createBuiltinBinaryFunction(BridgedStringRef name, BridgedType operandType, BridgedType resultType, BridgedValueArray arguments) const; diff --git a/include/swift/SIL/SILBridgingImpl.h b/include/swift/SIL/SILBridgingImpl.h index 19e72e1cfd72f..d195ffb9bf1ab 100644 --- a/include/swift/SIL/SILBridgingImpl.h +++ b/include/swift/SIL/SILBridgingImpl.h @@ -1124,127 +1124,112 @@ BridgedWitnessTableEntryArray BridgedDefaultWitnessTable::getEntries() const { // BridgedBuilder //===----------------------------------------------------------------------===// -static swift::SILBuilder builder(const BridgedBuilder * _Nonnull b) { - switch (b->insertAt) { - case BridgedBuilder::InsertAt::beforeInst: - return swift::SILBuilder(BridgedInstruction(b->insertionObj).get(), b->loc.getLoc().getScope()); - case BridgedBuilder::InsertAt::endOfBlock: - return swift::SILBuilder(BridgedBasicBlock(b->insertionObj).get(), b->loc.getLoc().getScope()); - case BridgedBuilder::InsertAt::intoGlobal: - return swift::SILBuilder(BridgedGlobalVar(b->insertionObj).getGlobal()); - } -} - -static swift::SILLocation regularLoc(const BridgedBuilder * _Nonnull b) { - return swift::RegularLocation(b->loc.getLoc().getLocation()); -} - BridgedInstruction BridgedBuilder::createBuiltinBinaryFunction(BridgedStringRef name, BridgedType operandType, BridgedType resultType, BridgedValueArray arguments) const { llvm::SmallVector argValues; - return {builder(this).createBuiltinBinaryFunction(regularLoc(this), + return {get().createBuiltinBinaryFunction(regularLoc(), name.get(), operandType.get(), resultType.get(), arguments.getValues(argValues))}; } BridgedInstruction BridgedBuilder::createCondFail(BridgedValue condition, BridgedStringRef message) const { - return {builder(this).createCondFail(regularLoc(this), condition.getSILValue(), message.get())}; + return {get().createCondFail(regularLoc(), condition.getSILValue(), message.get())}; } BridgedInstruction BridgedBuilder::createIntegerLiteral(BridgedType type, SwiftInt value) const { - return {builder(this).createIntegerLiteral(regularLoc(this), type.get(), value)}; + return {get().createIntegerLiteral(regularLoc(), type.get(), value)}; } BridgedInstruction BridgedBuilder::createAllocStack(BridgedType type, bool hasDynamicLifetime, bool isLexical, bool wasMoved) const { - return {builder(this).createAllocStack(regularLoc(this), type.get(), llvm::None, + return {get().createAllocStack(regularLoc(), type.get(), llvm::None, hasDynamicLifetime, isLexical, wasMoved)}; } BridgedInstruction BridgedBuilder::createDeallocStack(BridgedValue operand) const { - return {builder(this).createDeallocStack(regularLoc(this), operand.getSILValue())}; + return {get().createDeallocStack(regularLoc(), operand.getSILValue())}; } BridgedInstruction BridgedBuilder::createDeallocStackRef(BridgedValue operand) const { - return {builder(this).createDeallocStackRef(regularLoc(this), operand.getSILValue())}; + return {get().createDeallocStackRef(regularLoc(), operand.getSILValue())}; } BridgedInstruction BridgedBuilder::createUncheckedRefCast(BridgedValue op, BridgedType type) const { - return {builder(this).createUncheckedRefCast(regularLoc(this), op.getSILValue(), type.get())}; + return {get().createUncheckedRefCast(regularLoc(), op.getSILValue(), type.get())}; } BridgedInstruction BridgedBuilder::createUpcast(BridgedValue op, BridgedType type) const { - return {builder(this).createUpcast(regularLoc(this), op.getSILValue(), type.get())}; + return {get().createUpcast(regularLoc(), op.getSILValue(), type.get())}; } BridgedInstruction BridgedBuilder::createLoad(BridgedValue op, SwiftInt ownership) const { - return {builder(this).createLoad(regularLoc(this), op.getSILValue(), (swift::LoadOwnershipQualifier)ownership)}; + return {get().createLoad(regularLoc(), op.getSILValue(), (swift::LoadOwnershipQualifier)ownership)}; } BridgedInstruction BridgedBuilder::createBeginDeallocRef(BridgedValue reference, BridgedValue allocation) const { - return {builder(this).createBeginDeallocRef(regularLoc(this), reference.getSILValue(), allocation.getSILValue())}; + return {get().createBeginDeallocRef(regularLoc(), reference.getSILValue(), allocation.getSILValue())}; } BridgedInstruction BridgedBuilder::createEndInitLetRef(BridgedValue op) const { - return {builder(this).createEndInitLetRef(regularLoc(this), op.getSILValue())}; + return {get().createEndInitLetRef(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createStrongRetain(BridgedValue op) const { - auto b = builder(this); - return {b.createStrongRetain(regularLoc(this), op.getSILValue(), b.getDefaultAtomicity())}; + auto b = get(); + return {b.createStrongRetain(regularLoc(), op.getSILValue(), b.getDefaultAtomicity())}; } BridgedInstruction BridgedBuilder::createStrongRelease(BridgedValue op) const { - auto b = builder(this); - return {b.createStrongRelease(regularLoc(this), op.getSILValue(), b.getDefaultAtomicity())}; + auto b = get(); + return {b.createStrongRelease(regularLoc(), op.getSILValue(), b.getDefaultAtomicity())}; } BridgedInstruction BridgedBuilder::createUnownedRetain(BridgedValue op) const { - auto b = builder(this); - return {b.createUnownedRetain(regularLoc(this), op.getSILValue(), b.getDefaultAtomicity())}; + auto b = get(); + return {b.createUnownedRetain(regularLoc(), op.getSILValue(), b.getDefaultAtomicity())}; } BridgedInstruction BridgedBuilder::createUnownedRelease(BridgedValue op) const { - auto b = builder(this); - return {b.createUnownedRelease(regularLoc(this), op.getSILValue(), b.getDefaultAtomicity())}; + auto b = get(); + return {b.createUnownedRelease(regularLoc(), op.getSILValue(), b.getDefaultAtomicity())}; } BridgedInstruction BridgedBuilder::createFunctionRef(BridgedFunction function) const { - return {builder(this).createFunctionRef(regularLoc(this), function.getFunction())}; + return {get().createFunctionRef(regularLoc(), function.getFunction())}; } BridgedInstruction BridgedBuilder::createCopyValue(BridgedValue op) const { - return {builder(this).createCopyValue(regularLoc(this), op.getSILValue())}; + return {get().createCopyValue(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createBeginBorrow(BridgedValue op) const { - return {builder(this).createBeginBorrow(regularLoc(this), op.getSILValue())}; + return {get().createBeginBorrow(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createEndBorrow(BridgedValue op) const { - return {builder(this).createEndBorrow(regularLoc(this), op.getSILValue())}; + return {get().createEndBorrow(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createCopyAddr(BridgedValue from, BridgedValue to, bool takeSource, bool initializeDest) const { - return {builder(this).createCopyAddr(regularLoc(this), + return {get().createCopyAddr(regularLoc(), from.getSILValue(), to.getSILValue(), swift::IsTake_t(takeSource), swift::IsInitialization_t(initializeDest))}; } BridgedInstruction BridgedBuilder::createDestroyValue(BridgedValue op) const { - return {builder(this).createDestroyValue(regularLoc(this), op.getSILValue())}; + return {get().createDestroyValue(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createDestroyAddr(BridgedValue op) const { - return {builder(this).createDestroyAddr(regularLoc(this), op.getSILValue())}; + return {get().createDestroyAddr(regularLoc(), op.getSILValue())}; } BridgedInstruction BridgedBuilder::createDebugStep() const { - return {builder(this).createDebugStep(regularLoc(this))}; + return {get().createDebugStep(regularLoc())}; } BridgedInstruction BridgedBuilder::createApply(BridgedValue function, BridgedSubstitutionMap subMap, @@ -1255,7 +1240,7 @@ BridgedInstruction BridgedBuilder::createApply(BridgedValue function, BridgedSub if (isNonThrowing) { applyOpts |= swift::ApplyFlags::DoesNotThrow; } if (isNonAsync) { applyOpts |= swift::ApplyFlags::DoesNotAwait; } - return {builder(this).createApply(regularLoc(this), + return {get().createApply(regularLoc(), function.getSILValue(), subMap.get(), arguments.getValues(argValues), applyOpts, specInfo.data)}; @@ -1277,7 +1262,7 @@ BridgedInstruction BridgedBuilder::createSwitchEnumInst(BridgedValue enumVal, Op assert(mappedElements.count(c.first) && "wrong enum element index"); convertedCases.push_back({mappedElements[c.first], c.second.get()}); } - return {builder(this).createSwitchEnum(regularLoc(this), + return {get().createSwitchEnum(regularLoc(), enumVal.getSILValue(), defaultBlock.get(), convertedCases)}; } @@ -1285,7 +1270,7 @@ BridgedInstruction BridgedBuilder::createSwitchEnumInst(BridgedValue enumVal, Op BridgedInstruction BridgedBuilder::createUncheckedEnumData(BridgedValue enumVal, SwiftInt caseIdx, BridgedType resultType) const { swift::SILValue en = enumVal.getSILValue(); - return {builder(this).createUncheckedEnumData(regularLoc(this), enumVal.getSILValue(), + return {get().createUncheckedEnumData(regularLoc(), enumVal.getSILValue(), en->getType().getEnumElement(caseIdx), resultType.get())}; } @@ -1293,75 +1278,75 @@ BridgedInstruction BridgedBuilder::createEnum(SwiftInt caseIdx, OptionalBridgedV BridgedType resultType) const { swift::EnumElementDecl *caseDecl = resultType.get().getEnumElement(caseIdx); swift::SILValue pl = payload.getSILValue(); - return {builder(this).createEnum(regularLoc(this), pl, caseDecl, resultType.get())}; + return {get().createEnum(regularLoc(), pl, caseDecl, resultType.get())}; } BridgedInstruction BridgedBuilder::createBranch(BridgedBasicBlock destBlock, BridgedValueArray arguments) const { llvm::SmallVector argValues; - return {builder(this).createBranch(regularLoc(this), destBlock.get(), arguments.getValues(argValues))}; + return {get().createBranch(regularLoc(), destBlock.get(), arguments.getValues(argValues))}; } BridgedInstruction BridgedBuilder::createUnreachable() const { - return {builder(this).createUnreachable(regularLoc(this))}; + return {get().createUnreachable(regularLoc())}; } BridgedInstruction BridgedBuilder::createObject(BridgedType type, BridgedValueArray arguments, SwiftInt numBaseElements) const { llvm::SmallVector argValues; - return {builder(this).createObject(swift::ArtificialUnreachableLocation(), + return {get().createObject(swift::ArtificialUnreachableLocation(), type.get(), arguments.getValues(argValues), numBaseElements)}; } BridgedInstruction BridgedBuilder::createGlobalAddr(BridgedGlobalVar global) const { - return {builder(this).createGlobalAddr(regularLoc(this), global.getGlobal())}; + return {get().createGlobalAddr(regularLoc(), global.getGlobal())}; } BridgedInstruction BridgedBuilder::createGlobalValue(BridgedGlobalVar global, bool isBare) const { - return {builder(this).createGlobalValue(regularLoc(this), global.getGlobal(), isBare)}; + return {get().createGlobalValue(regularLoc(), global.getGlobal(), isBare)}; } BridgedInstruction BridgedBuilder::createStruct(BridgedType type, BridgedValueArray elements) const { llvm::SmallVector elementValues; - return {builder(this).createStruct(regularLoc(this), type.get(), elements.getValues(elementValues))}; + return {get().createStruct(regularLoc(), type.get(), elements.getValues(elementValues))}; } BridgedInstruction BridgedBuilder::createStructExtract(BridgedValue str, SwiftInt fieldIndex) const { swift::SILValue v = str.getSILValue(); - return {builder(this).createStructExtract(regularLoc(this), v, v->getType().getFieldDecl(fieldIndex))}; + return {get().createStructExtract(regularLoc(), v, v->getType().getFieldDecl(fieldIndex))}; } BridgedInstruction BridgedBuilder::createStructElementAddr(BridgedValue addr, SwiftInt fieldIndex) const { swift::SILValue v = addr.getSILValue(); - return {builder(this).createStructElementAddr(regularLoc(this), v, v->getType().getFieldDecl(fieldIndex))}; + return {get().createStructElementAddr(regularLoc(), v, v->getType().getFieldDecl(fieldIndex))}; } BridgedInstruction BridgedBuilder::createDestructureStruct(BridgedValue str) const { - return {builder(this).createDestructureStruct(regularLoc(this), str.getSILValue())}; + return {get().createDestructureStruct(regularLoc(), str.getSILValue())}; } BridgedInstruction BridgedBuilder::createTuple(BridgedType type, BridgedValueArray elements) const { llvm::SmallVector elementValues; - return {builder(this).createTuple(regularLoc(this), type.get(), elements.getValues(elementValues))}; + return {get().createTuple(regularLoc(), type.get(), elements.getValues(elementValues))}; } BridgedInstruction BridgedBuilder::createTupleExtract(BridgedValue str, SwiftInt elementIndex) const { swift::SILValue v = str.getSILValue(); - return {builder(this).createTupleExtract(regularLoc(this), v, elementIndex)}; + return {get().createTupleExtract(regularLoc(), v, elementIndex)}; } BridgedInstruction BridgedBuilder::createTupleElementAddr(BridgedValue addr, SwiftInt elementIndex) const { swift::SILValue v = addr.getSILValue(); - return {builder(this).createTupleElementAddr(regularLoc(this), v, elementIndex)}; + return {get().createTupleElementAddr(regularLoc(), v, elementIndex)}; } BridgedInstruction BridgedBuilder::createDestructureTuple(BridgedValue str) const { - return {builder(this).createDestructureTuple(regularLoc(this), str.getSILValue())}; + return {get().createDestructureTuple(regularLoc(), str.getSILValue())}; } BridgedInstruction BridgedBuilder::createStore(BridgedValue src, BridgedValue dst, SwiftInt ownership) const { - return {builder(this).createStore(regularLoc(this), src.getSILValue(), dst.getSILValue(), + return {get().createStore(regularLoc(), src.getSILValue(), dst.getSILValue(), (swift::StoreOwnershipQualifier)ownership)}; } @@ -1369,7 +1354,7 @@ BridgedInstruction BridgedBuilder::createInitExistentialRef(BridgedValue instanc BridgedType type, BridgedInstruction useConformancesOf) const { auto *src = useConformancesOf.getAs(); - return {builder(this).createInitExistentialRef(regularLoc(this), type.get(), + return {get().createInitExistentialRef(regularLoc(), type.get(), src->getFormalConcreteType(), instance.getSILValue(), src->getConformances())}; @@ -1379,11 +1364,11 @@ BridgedInstruction BridgedBuilder::createMetatype(BridgedType type, BridgedType::MetatypeRepresentation representation) const { auto *mt = swift::MetatypeType::get(type.get().getASTType(), (swift::MetatypeRepresentation)representation); auto t = swift::SILType::getPrimitiveObjectType(swift::CanType(mt)); - return {builder(this).createMetatype(regularLoc(this), t)}; + return {get().createMetatype(regularLoc(), t)}; } BridgedInstruction BridgedBuilder::createEndCOWMutation(BridgedValue instance, bool keepUnique) const { - return {builder(this).createEndCOWMutation(regularLoc(this), instance.getSILValue(), keepUnique)}; + return {get().createEndCOWMutation(regularLoc(), instance.getSILValue(), keepUnique)}; } //===----------------------------------------------------------------------===//