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-2394] Swift shouldn't import "returns twice" functions like setjmp #45001

Closed
belkadan opened this issue Aug 18, 2016 · 4 comments
Closed

Comments

@belkadan
Copy link
Contributor

belkadan commented Aug 18, 2016

Previous ID SR-2394
Radar rdar://problem/27912174
Original Reporter @belkadan
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, ClangImporter, StarterBug
Assignee Lovems (JIRA)
Priority Medium

md5: f54f1e669749026cf48985777f591afe

Issue Description:

Clang has a notion of “returns twice” functions, used for things like fork/setjmp and other icky things. The compiler doesn’t reason about these function and the control flow that they entail, so DI and other properties are not correct.

When importing one of them, we should slap an ‘unavailable’ attribute on them, so swift programmers don’t use them.

@modocache
Copy link
Mannequin

modocache mannequin commented Dec 30, 2016

I'd like to try my hand at this, assuming no one else minds. I expect to make progress on it by the end of January at the latest.

Some notes of mine:

fork and vfork are explicitly marked as unavailable on most Darwin platforms. But setjmp and other functions are not. Here's my test program, which compiles just fine:

// swift -frontend -c -primary-file 2394.swift -sdk `xcrun -sdk macosx10.12 --show-sdk-path` -module-name main -o 2394.o

import Darwin

var x: Int32 = 0
setjmp(&x)

I believe Clang contains some code that attributes functions like fork and setjmp with __attribute__ ((returns_twice)). By placing a breakpoint within a ClangImporter method, I can see that setjmp has that attribute set. So, as far as I understand, all I have to do is find a good spot in lib/ClangImporter to check for that attribute, then mark the symbol as unavailable if it's set. At that point, I think I can remove the explicit exclusion of fork and vfork, since they'll be unavailable anyway.

I wonder, though: this is a source breaking change, isn't it? The Swift code above used to compile, but as of this change, it will no longer compile. Is that acceptable, @belkadan?

@modocache
Copy link
Mannequin

modocache mannequin commented Dec 30, 2016

I think #6513 resolves this issue, but this seems like an invasive, source-breaking change. Particularly disappointing for me is the fact that the Swift test suite runs on Android in part thanks to fork. Marking it unavailable will break those tests. I'll have to re-implement that portion in order to get them running again.

@modocache
Copy link
Mannequin

modocache mannequin commented Dec 31, 2016

Oops, my bad, fork isn't a 'returns_twice' function, I don't think. Sorry about the confusion!

@modocache
Copy link
Mannequin

modocache mannequin commented Jan 9, 2017

#6513 was merged, so I think this is done!

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant