Merge pull request #71429 from gottesmm/fixes-for-stdlib

[region-isolation] Fixes that enable the stdlib to build with region based isolation

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -894,8 +894,20 @@ NOTE(sil_referencebinding_inout_binding_here, none,
       "inout binding here",
       ())
 
-// Warnings arising from the flow-sensitive checking of Sendability of
-// non-Sendable values
+//===----------------------------------------------------------------------===//
+//                  MARK: Region Based Isolation Diagnostics
+//===----------------------------------------------------------------------===//
+
+NOTE(regionbasedisolation_maybe_race, none,
+     "access here could race", ())
+ERROR(regionbasedisolation_unknown_pattern, none,
+      "pattern that the region based isolation checker does not understand how to check. Please file a bug",
+      ())
+
+//===---
+// Old Transfer Non Sendable Diagnostics
+//
+
 ERROR(regionbasedisolation_selforargtransferred, none,
       "call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
 ERROR(regionbasedisolation_transfer_yields_race_no_isolation, none,
@@ -919,14 +931,28 @@ ERROR(regionbasedisolation_arg_transferred, none,
 ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
       "task isolated value of type %0 passed as a strongly transferred parameter; later accesses could race",
       (Type))
-NOTE(regionbasedisolation_maybe_race, none,
-     "access here could race", ())
 NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
      "value is %0 since it is in the same region as %1",
      (StringRef, DeclBaseName))
-ERROR(regionbasedisolation_unknown_pattern, none,
-      "pattern that the region based isolation checker does not understand how to check. Please file a bug",
-      ())
+
+//===---
+// New Transfer Non Sendable Diagnostics
+//
+
+ERROR(regionbasedisolation_named_transfer_yields_race, none,
+      "transferring non-Sendable value %0 could yield races with later accesses",
+      (Identifier))
+NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
+     "%0 is transferred from %1 caller to %2 callee. Later uses in caller could race with potential uses in callee",
+     (Identifier, ActorIsolation, ActorIsolation))
+NOTE(regionbasedisolation_named_info_isolated_capture, none,
+     "%1 value %0 is captured by %2 closure. Later local uses could race",
+     (Identifier, ActorIsolation, ActorIsolation))
+NOTE(regionbasedisolation_named_arg_info, none,
+     "Transferring task isolated function argument %0 could yield races with caller uses",
+     (Identifier))
+NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
+     "Cannot access a transferring parameter after the parameter has been transferred", ())
 
 // TODO: print the name of the nominal type
 ERROR(deinit_not_visible, none,

include/swift/SIL/DebugUtils.h

@@ -531,6 +531,19 @@ struct DebugVarCarryingInst : VarDeclCarryingInst {
     return varName;
   }
 
+  std::optional<StringRef> maybeGetName() const {
+    assert(getKind() != Kind::Invalid);
+    if (auto varInfo = getVarInfo()) {
+      return varInfo->Name;
+    }
+
+    if (auto *decl = getDecl()) {
+      return decl->getBaseName().userFacingName();
+    }
+
+    return {};
+  }
+
   /// Take in \p inst, a potentially invalid DebugVarCarryingInst, and returns a
   /// name for it. If we have an invalid value or don't find var info or a decl,
   /// return "unknown".

include/swift/SILOptimizer/Utils/VariableNameUtils.h

@@ -0,0 +1,116 @@
+//===--- VariableNameUtils.h ----------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+///
+/// Utilities for inferring the name of a value.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H
+#define SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H
+
+#include "swift/SIL/ApplySite.h"
+#include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/MemAccessUtils.h"
+#include "swift/SIL/SILInstruction.h"
+#include "swift/SIL/StackList.h"
+
+namespace swift {
+
+class VariableNameInferrer {
+  /// The stacklist that we use to process from use->
+  StackList<PointerUnion<SILInstruction *, SILValue>> variableNamePath;
+
+  /// The root value of our string.
+  ///
+  /// If set, a diagnostic should do a 'root' is defined here error.
+  SILValue rootValue;
+
+  /// The final string we computed.
+  SmallString<64> &resultingString;
+
+public:
+  VariableNameInferrer(SILFunction *fn, SmallString<64> &resultingString)
+      : variableNamePath(fn), resultingString(resultingString) {}
+
+  /// Attempts to infer a name from just uses of \p searchValue.
+  ///
+  /// Returns true if we found a name.
+  bool tryInferNameFromUses(SILValue searchValue) {
+    auto *use = getAnyDebugUse(searchValue);
+    if (!use)
+      return false;
+
+    auto debugVar = DebugVarCarryingInst(use->getUser());
+    if (!debugVar)
+      return false;
+
+    assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
+    resultingString += debugVar.getName();
+    return true;
+  }
+
+  /// See if \p searchValue or one of its defs (walking use->def) has a name
+  /// that we can use.
+  ///
+  /// \p failInsteadOfEmittingUnknown set to true if we should return false
+  /// rather than emitting unknown (and always succeeding).
+  ///
+  /// \returns true if we inferred anything. Returns false otherwise.
+  bool inferByWalkingUsesToDefs(SILValue searchValue) {
+    // Look up our root value while adding to the variable name path list.
+    auto rootValue = findDebugInfoProvidingValue(searchValue);
+    if (!rootValue) {
+      // If we do not pattern match successfully, just set resulting string to
+      // unknown and return early.
+      resultingString += "unknown";
+      return true;
+    }
+
+    drainVariableNamePath();
+    return true;
+  }
+
+  /// Infers the value that provides the debug info. This can be something like
+  /// an alloc_stack that provides the information directly or a value that has
+  /// a debug_value as a user.
+  ///
+  /// \returns SILValue() if we did not find anything.
+  SILValue inferByWalkingUsesToDefsReturningRoot(SILValue searchValue) {
+    // Look up our root value while adding to the variable name path list.
+    auto rootValue = findDebugInfoProvidingValue(searchValue);
+    if (!rootValue) {
+      // If we do not pattern match successfully, return SILValue() early.
+      return SILValue();
+    }
+
+    drainVariableNamePath();
+    return rootValue;
+  }
+
+  StringRef getName() const { return resultingString; }
+
+private:
+  void drainVariableNamePath();
+
+  /// Finds the SILValue that either provides the direct debug information or
+  /// that has a debug_value user that provides the name of the value.
+  SILValue findDebugInfoProvidingValue(SILValue searchValue);
+
+  /// Given an initialized once allocation inst without a ValueDecl or a
+  /// DebugVariable provided name, attempt to find a root value from its
+  /// initialization.
+  SILValue getRootValueForTemporaryAllocation(AllocationInst *allocInst);
+};
+
+} // namespace swift
+
+#endif

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -126,13 +126,17 @@ struct UseDefChainVisitor
     // do not want to treat this as a merge.
     if (auto p = Projection(inst)) {
       switch (p.getKind()) {
+      // Currently if we load and then project_box from a memory location,
+      // we treat that as a projection. This follows the semantics/notes in
+      // getAccessProjectionOperand.
+      case ProjectionKind::Box:
+        return cast<ProjectBoxInst>(inst)->getOperand();
       case ProjectionKind::Upcast:
       case ProjectionKind::RefCast:
       case ProjectionKind::BlockStorageCast:
       case ProjectionKind::BitwiseCast:
-      case ProjectionKind::TailElems:
-      case ProjectionKind::Box:
       case ProjectionKind::Class:
+      case ProjectionKind::TailElems:
         llvm_unreachable("Shouldn't see this here");
       case ProjectionKind::Index:
         // Index is always a merge.
@@ -1289,8 +1293,7 @@ class PartitionOpTranslator {
               continue;
             }
             if (auto *pbi = dyn_cast<ProjectBoxInst>(val)) {
-              if (isNonSendableType(
-                      pbi->getType().getSILBoxFieldType(function))) {
+              if (isNonSendableType(pbi->getType())) {
                 auto trackVal = getTrackableValue(val, true);
                 (void)trackVal;
                 continue;
@@ -1431,8 +1434,12 @@ class PartitionOpTranslator {
 
     if (auto tryApplyInst = dyn_cast<TryApplyInst>(inst)) {
       foundResults.emplace_back(tryApplyInst->getNormalBB()->getArgument(0));
-      if (tryApplyInst->getErrorBB()->getNumArguments() > 0)
+      if (tryApplyInst->getErrorBB()->getNumArguments() > 0) {
         foundResults.emplace_back(tryApplyInst->getErrorBB()->getArgument(0));
+      }
+      for (auto indirectResults : tryApplyInst->getIndirectSILResults()) {
+        foundResults.emplace_back(indirectResults);
+      }
       return;
     }
 
@@ -1797,10 +1804,10 @@ class PartitionOpTranslator {
     };
 
     if (applySite.hasSelfArgument()) {
-      handleSILOperands(applySite.getOperandsWithoutSelf());
+      handleSILOperands(applySite.getOperandsWithoutIndirectResultsOrSelf());
       handleSILSelf(&applySite.getSelfArgumentOperand());
     } else {
-      handleSILOperands(applySite->getAllOperands());
+      handleSILOperands(applySite.getOperandsWithoutIndirectResults());
     }
 
     // non-sendable results can't be returned from cross-isolation calls without
@@ -2109,6 +2116,7 @@ class PartitionOpTranslator {
     }
 
     case TranslationSemantics::Asserting:
+      llvm::errs() << "BannedInst: " << *inst;
       llvm::report_fatal_error(
           "transfer-non-sendable: Found banned instruction?!");
       return;
@@ -2119,6 +2127,7 @@ class PartitionOpTranslator {
             return ::isNonSendableType(value->getType(), inst->getFunction());
           }))
         return;
+      llvm::errs() << "BadInst: " << *inst;
       llvm::report_fatal_error(
           "transfer-non-sendable: Found instruction that is not allowed to "
           "have non-Sendable parameters with such parameters?!");
@@ -2304,7 +2313,6 @@ CONSTANT_TRANSLATION(TupleInst, Assign)
 CONSTANT_TRANSLATION(BeginAccessInst, LookThrough)
 CONSTANT_TRANSLATION(BeginBorrowInst, LookThrough)
 CONSTANT_TRANSLATION(BeginDeallocRefInst, LookThrough)
-CONSTANT_TRANSLATION(RefToBridgeObjectInst, LookThrough)
 CONSTANT_TRANSLATION(BridgeObjectToRefInst, LookThrough)
 CONSTANT_TRANSLATION(CopyValueInst, LookThrough)
 CONSTANT_TRANSLATION(ExplicitCopyValueInst, LookThrough)
@@ -2414,6 +2422,8 @@ CONSTANT_TRANSLATION(BeginUnpairedAccessInst, Require)
 CONSTANT_TRANSLATION(ValueMetatypeInst, Require)
 // Require of the value we extract the metatype from.
 CONSTANT_TRANSLATION(ExistentialMetatypeInst, Require)
+// These can take a parameter. If it is non-Sendable, use a require.
+CONSTANT_TRANSLATION(GetAsyncContinuationAddrInst, Require)
 
 //===---
 // Asserting If Non Sendable Parameter
@@ -2454,7 +2464,6 @@ CONSTANT_TRANSLATION(DynamicMethodBranchInst, TerminatorPhi)
 // (UnsafeContinuation and UnsafeThrowingContinuation).
 CONSTANT_TRANSLATION(AwaitAsyncContinuationInst, AssertingIfNonSendable)
 CONSTANT_TRANSLATION(GetAsyncContinuationInst, AssertingIfNonSendable)
-CONSTANT_TRANSLATION(GetAsyncContinuationAddrInst, AssertingIfNonSendable)
 CONSTANT_TRANSLATION(ExtractExecutorInst, AssertingIfNonSendable)
 
 //===---
@@ -2614,6 +2623,13 @@ CAST_WITH_MAYBE_SENDABLE_NONSENDABLE_OP_AND_RESULT(UncheckedValueCastInst)
 // Custom Handling
 //
 
+TranslationSemantics
+PartitionOpTranslator::visitRefToBridgeObjectInst(RefToBridgeObjectInst *r) {
+  translateSILLookThrough(
+      SILValue(r), r->getOperand(RefToBridgeObjectInst::ConvertedOperand));
+  return TranslationSemantics::Special;
+}
+
 TranslationSemantics
 PartitionOpTranslator::visitPackElementGetInst(PackElementGetInst *r) {
   if (!isNonSendableType(r->getType()))