Skip to content
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

[SR-8622] Nonnull Objective-C property that falsely returns nil causes inconsistent Swift behavior #51137

Open
allevato opened this issue Aug 23, 2018 · 3 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@allevato
Copy link
Collaborator

Previous ID SR-8622
Radar rdar://43645642
Original Reporter @allevato
Type Bug
Environment

Swift 4.2, Xcode 10 beta 4, macOS 10.13.16. May be present in other versions.

Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: 1fd0b53e09d10b8697bba1cbc378a479

is duplicated by:

relates to:

  • SR-120 Objective-C non-null can be bypassed.
  • SR-7300 nil converted to NSNull when returned from a nonnull Objective-C method

Issue Description:

This looks like it might be related to SR-7300, but I don't see any explicit NSNull-related behavior so it could be different.

If an Objective-C API that's declared nonnull does in fact return nil, then the behavior when called from Swift is inconsistent:

  • Assigning such a value to a variable produces a non-optional value of that type whose underlying pointer value is 0x0 (when viewed in the debugger)

  • Calling a method or getting/setting a property with that variable as the receiver does not trap, but instead exhibits Objective-C-like functionality (silently succeeding, returning bit pattern 0 in non-void cases)

  • Trying to print the value itself, however, crashes the program/interpreter

This is of course a case where the API implementation is violating its own contract, but I would expect the first assignment above to be where Swift would trap, since there's no obvious way to guard against the unexpected nil result.

Repro case:

import Foundation

// Despite its contract, this returns nil if there is no bundle ID
// (e.g., in a command line app without an embedded Info.plist)
let center = NSUserNotificationCenter.default
print("got something for center")

let delegate = center.delegate
print("was able to call a getter, its value was (delegate)")

center.delegate = nil
print("was able to call a setter")

print("now we'll try to print the value itself")
print(center)
print("this won't print because it crashed")

When compiled and executed, the following output is produced:

got something for center
was able to call a getter, its value was nil
was able to call a setter
now we'll try to print the value itself
[1] 58342 segmentation fault ./trap

When executed under the interpreter, the backtrace gives a little more insight:

0 swift 0x0000000104ebbb8a PrintStackTraceSignalHandler(void*) + 42
1 swift 0x0000000104ebb32e SignalHandler(int) + 302
2 libsystem_platform.dylib 0x00007fff576fbf5a _sigtramp + 26
3 libsystem_platform.dylib 0x00007fe9b17a9238 _sigtramp + 1510658808
4 libswiftCore.dylib 0x000000010ae71f25 findDynamicValueAndType(swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::OpaqueValue*&, swift::TargetMetadata<swift::InProcess> const*&, bool&, bool, bool) + 245
5 libswiftCore.dylib 0x000000010ae76687 _dynamicCastToExistential(swift::OpaqueValue*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetExistentialTypeMetadata<swift::InProcess> const*, swift::DynamicCastFlags) + 135
6 libswiftCore.dylib 0x000000010ac41c4a $Ss15_print_unlockedyyx_q_zts16TextOutputStreamR_r0_lF + 394
7 libswiftCore.dylib 0x000000010ae1931a $Ss6_print_9separator10terminator2toySayypG_S2Sxzts16TextOutputStreamRzlFs7_StdoutV_Tg5Tf4nxxn_nTm + 250
8 libswiftCore.dylib 0x000000010ae19c1f $Ss5print_9separator10terminatoryypd_S2StFTf4nxx_nTm + 271
9 libswiftCore.dylib 0x000000010acedba0 $Ss5print_9separator10terminatoryypd_S2StF + 16
10 libswiftCore.dylib 0x000000010882856d $Ss5print_9separator10terminatoryypd_S2StF + 4256410077
11 swift 0x0000000101efa03d llvm::MCJIT::runFunction(llvm::Function*, llvm::ArrayRef<llvm::GenericValue>) + 365
12 swift 0x0000000101f00a3c llvm::ExecutionEngine::runFunctionAsMain(llvm::Function*, std::_1::vector<std::1::basic_string<char, std::1::char_traits<char>, std::1::allocator<char> >, std::1::allocator<std::1::basic_string<char, std::1::char_traits<char>, std::_1::allocator<char> > > > const&, char const* const*) + 1004
13 swift 0x000000010117c593 performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 52659
14 swift 0x000000010116c2e5 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 7733
15 swift 0x0000000101112155 main + 1349
16 libdyld.dylib 0x00007fff573ed015 start + 1

A radar has been filed separately for the NSUserNotificationCenter API's incorrect contract, although since it's deprecated it may not be a high priority fix: https://openradar.appspot.com/43645642

@belkadan
Copy link
Contributor

The cost of checking every nonnull return value was determined to be too high, but maybe we could do it in Debug builds.

@belkadan
Copy link
Contributor

Some good discussion in SR-11040.

@swift-ci
Copy link
Collaborator

Comment by Patrick Pijnappel (JIRA)

This sounds like it should be a compiler option.

I think for a lot of UI applications it might be worthwhile to check, esp. those with large codebases full of Obj-C/Swift interface (i.e. high probability of accidental contract violation).

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

3 participants