Skip to content

[Runtime] Add a disabled workaround for protocol conformance checking to check conformances in reverse order. #35061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions include/swift/Runtime/Bincompat.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===--- Bincompat.h - Binary compatibility checks. -------------*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2020 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
//
//===----------------------------------------------------------------------===//
//
// Checks for enabling binary compatibility workarounds.
//
//===----------------------------------------------------------------------===//

namespace swift {

namespace runtime {

namespace bincompat {

/// Whether protocol conformance iteration should be reversed, to prefer
/// conformances from images that are later in the list over earlier ones.
bool workaroundProtocolConformanceReverseIteration();

} // namespace bincompat

} // namespace runtime

} // namespace swift
31 changes: 31 additions & 0 deletions stdlib/public/runtime/Bincompat.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//===--- Bincompat.cpp - Binary compatibility checks. -----------*- C++ -*-===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2020 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
//
//===----------------------------------------------------------------------===//
//
// Checks for enabling binary compatibility workarounds.
//
//===----------------------------------------------------------------------===//

#include "swift/Runtime/Bincompat.h"

namespace swift {

namespace runtime {

namespace bincompat {

bool workaroundProtocolConformanceReverseIteration() { return false; }

} // namespace bincompat

} // namespace runtime

} // namespace swift
1 change: 1 addition & 0 deletions stdlib/public/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ set(swift_runtime_sources
Array.cpp
AutoDiffSupport.cpp
BackDeployment.cpp
Bincompat.cpp
Casting.cpp
CompatibilityOverride.cpp
CygwinPort.cpp
Expand Down
18 changes: 15 additions & 3 deletions stdlib/public/runtime/ProtocolConformance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "swift/Basic/Lazy.h"
#include "swift/Demangling/Demangle.h"
#include "swift/Runtime/Bincompat.h"
#include "swift/Runtime/Casting.h"
#include "swift/Runtime/Concurrent.h"
#include "swift/Runtime/HeapObject.h"
Expand Down Expand Up @@ -238,8 +239,11 @@ namespace {
struct ConformanceState {
ConcurrentReadableHashMap<ConformanceCacheEntry> Cache;
ConcurrentReadableArray<ConformanceSection> SectionsToScan;

bool scanSectionsBackwards;

ConformanceState() {
scanSectionsBackwards =
runtime::bincompat::workaroundProtocolConformanceReverseIteration();
initializeProtocolConformanceLookup();
}

Expand Down Expand Up @@ -466,8 +470,7 @@ swift_conformsToProtocolImpl(const Metadata *const type,
return found.second;

// Scan conformance records.
auto snapshot = C.SectionsToScan.snapshot();
for (auto &section : snapshot) {
auto processSection = [&](const ConformanceSection &section) {
// Eagerly pull records for nondependent witnesses into our cache.
for (const auto &record : section) {
auto &descriptor = *record.get();
Expand All @@ -485,6 +488,15 @@ swift_conformsToProtocolImpl(const Metadata *const type,
C.cacheResult(matchingType, protocol, witness, /*always cache*/ 0);
}
}
};

auto snapshot = C.SectionsToScan.snapshot();
if (C.scanSectionsBackwards) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to do something like:

if (C.scanSectionsBackwards)
  std::reverse(std::begin(snapshot), std::end(snapshot))
for (auto &section : snapshot)
  processSection(section);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, we can't mutate in place, but we settled on:

  auto snapshot = C.SectionsToScan.snapshot();
  if (C.scanSectionsBackwards) {
    for (auto &section : llvm::reverse(snapshot))
      processSection(section);
  } else {
    for (auto &section : snapshot)
      processSection(section);
  }

for (auto &section : llvm::reverse(snapshot))
processSection(section);
} else {
for (auto &section : snapshot)
processSection(section);
}

// Try the search again to look for the most specific cached conformance.
Expand Down
76 changes: 76 additions & 0 deletions test/Runtime/protocol_conformance_collision.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift %s -o %t/newSDK %target-link-sdk-future-version
// RUN: %target-codesign %t/newSDK
// RUN: %target-run %t/newSDK newSDK
// RUN: %target-build-swift %s -o %t/oldSDK %target-link-sdk-2020-version
// RUN: %target-codesign %t/oldSDK
// RUN: %target-run %t/oldSDK oldSDK

// REQUIRES: VENDOR=apple

// Simulators refuse to run binaries built with an SDK newer than the simulator.
// UNSUPPORTED: DARWIN_SIMULATOR=ios
// UNSUPPORTED: DARWIN_SIMULATOR=tvos
// UNSUPPORTED: DARWIN_SIMULATOR=watchos

import Accelerate
import Foundation
import StdlibUnittest
import SwiftShims


extension CFString: Hashable {
static var localHashableCallCount = 0
public var hashValue: Int {
Self.localHashableCallCount += 1
return (self as String).hashValue
}
}

protocol P {
func firstHashValue() -> Int
}

extension Set: P {
func firstHashValue() -> Int {
return first!.hashValue
}
}

@_optimize(none)
func firstHashValue(_ x: P) -> Int {
x.firstHashValue()
}

let osHasWorkaround: Bool
// These are deliberately NOT version 9999, as we don't want to hit the special
// case where development runtimes always return true for 9999. This check needs
// to be false until real version numbers are put in.
if #available(macOS 99990, iOS 99990, tvOS 99990, watchOS 99990, *) {
osHasWorkaround = true
} else {
osHasWorkaround = false
}

let testingOldSDK = CommandLine.arguments.last == "oldSDK"

var tests: TestSuite

if testingOldSDK {
tests = TestSuite("old SDK protocol conformance collision")
tests.test("CFString: Hashable conformance") {
_ = firstHashValue(NSSet(object: "Whatever") as! Set<CFString>)

let expectedCallCount = osHasWorkaround ? 1 : 0
expectEqual(expectedCallCount, CFString.localHashableCallCount)
}
} else {
tests = TestSuite("new SDK protocol conformance collision")
tests.test("CFString: Hashable conformance") {
_ = firstHashValue(NSSet(object: "Whatever") as! Set<CFString>)

expectEqual(0, CFString.localHashableCallCount)
}
}

runAllTests()
32 changes: 32 additions & 0 deletions test/lit.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,31 @@ if run_vendor == 'apple':

config.otool_classic = ("%s otool-classic" % (xcrun_prefix))

SDK_2020_VERSION = {
'macosx': '11.0',
'ios': '14.0',
'maccatalyst': '14.0',
'tvos': '14.0',
'watchos': '7.0'
}
sdk_2020_version = SDK_2020_VERSION.get(run_os, '')
linker_os = {
'iphoneos': 'ios',
'appletvos': 'tvos',
'watchos': 'watchos',
'iphonesimulator': 'ios-simulator',
'watchsimulator': 'watchos-simulator',
'appletvsimulator': 'tvos-simulator',
'macosx': 'macos'
}.get(config.target_sdk_name, run_os)

config.target_link_sdk_2020_version = (
"-Xlinker -platform_version -Xlinker %s -Xlinker %s -Xlinker %s" %
(linker_os, sdk_2020_version, sdk_2020_version))
config.target_link_sdk_future_version = (
"-Xlinker -platform_version -Xlinker %s -Xlinker %s -Xlinker %s" %
(linker_os, target_future_version, target_future_version))

elif run_os in ['windows-msvc']:
lit_config.note('Testing Windows ' + config.variant_triple)
config.environment['NUMBER_OF_PROCESSORS'] = os.environ['NUMBER_OF_PROCESSORS']
Expand Down Expand Up @@ -2021,6 +2046,13 @@ config.substitutions.append(('%llvm-cov', config.llvm_cov))
if hasattr(config, 'otool_classic'):
config.substitutions.append(('%otool-classic', config.otool_classic))

if hasattr(config, 'target_link_sdk_2020_version'):
config.substitutions.append(('%target-link-sdk-2020-version',
config.target_link_sdk_2020_version))
if hasattr(config, 'target_link_sdk_future_version'):
config.substitutions.append(('%target-link-sdk-future-version',
config.target_link_sdk_future_version))

run_filecheck = '%s %s --sanitize BUILD_DIR=%s --sanitize SOURCE_DIR=%s --use-filecheck %s %s' % (
shell_quote(sys.executable),
shell_quote(config.PathSanitizingFileCheck),
Expand Down