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

work around SR-11680 #145

Merged
merged 1 commit into from Jan 23, 2020
Merged

work around SR-11680 #145

merged 1 commit into from Jan 23, 2020

Conversation

mayoff
Copy link
Contributor

@mayoff mayoff commented Jan 23, 2020

The Swift bug report: https://bugs.swift.org/browse/SR-11680

Swift nightly toolchains are available here: https://swift.org/download/

The Swift nightly toolchains cannot build OpenCombine. Here's why:

  1. The COpenCombineHelpers target defines a non-static function
    (opencombine_stop_in_debugger) in a header file. This function is
    emitted in the target's IR, but not in the target's TBD.

  2. Swift nightly toolchains have assertions enabled, so they use the
    -validate-tbd-against-ir=missing build setting. This build setting
    makes the compiler fail if the TBD doesn't match the IR.

This commit turns off the build setting for the OpenCombine target.

@MaxDesiatov
Copy link
Collaborator

The linked SR-11680 has different environment specified, namely Xcode 11.0, swift-5.1-RELEASE. Does that mean that using nightlies is not required to reproduce it?

@broadwaylamb
Copy link
Member

Hi @mayoff, thanks for pointing out this issue!

However, we can't use the unsafeFlags in a public target because of this:

As some build flags can be exploited for unsupported or malicious behavior, a product can’t be used as a dependency in another package if one of its targets uses unsafe flags.

So, basically, clients won't be able to depend on OpenCombine.

What I would suggest to do is simply define opencombine_stop_in_debugger out of line in COpenCombineHelpers.cpp instead of the header file.

@codecov
Copy link

codecov bot commented Jan 23, 2020

Codecov Report

Merging #145 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #145      +/-   ##
==========================================
+ Coverage   98.24%   98.29%   +0.05%     
==========================================
  Files          84       82       -2     
  Lines        4730     4639      -91     
==========================================
- Hits         4647     4560      -87     
+ Misses         83       79       -4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79899f7...c44eda4. Read the comment docs.

@OpenCombineBot
Copy link

OpenCombineBot commented Jan 23, 2020

LGTM

Generated by 🚫 Danger Swift against c44eda4

@mayoff
Copy link
Contributor Author

mayoff commented Jan 23, 2020

@MaxDesiatov I don't know if the RELEASE builds at https://swift.org/download/ are built with assertions. You could (I assume) trigger the same error by manually setting-validate-tbd-against-ir=missing with any toolchain.

@broadwaylamb Oh, good point. I'll revise the patch to non-inline the function.

The Swift bug report: https://bugs.swift.org/browse/SR-11680

Swift nightly toolchains are available here: https://swift.org/download/

The Swift nightly toolchains cannot build OpenCombine. Here's why:

The COpenCombineHelpers target defines a non-static function
(`opencombine_stop_in_debugger`) in a header file. This function is
emitted in the target's IR, but not in the target's TBD.

Swift nightly toolchains have assertions enabled, so they use the
-validate-tbd-against-ir=missing build setting. This build setting
makes the compiler fail if the TBD doesn't match the IR.

This commit un-inlines `opencombine_stop_in_debugger`, so it
is not emitted in the IR. This stops the TBD validator from
complaining.
Copy link
Member

@broadwaylamb broadwaylamb left a comment

Choose a reason for hiding this comment

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

Nice!

@mayoff
Copy link
Contributor Author

mayoff commented Jan 23, 2020

I have rewritten the patch so that it no longer uses .unsafeFlags.

@broadwaylamb broadwaylamb merged commit 3b1437e into OpenCombine:master Jan 23, 2020
sfmntmzzn pushed a commit to sfmntmzzn/OpenCombine that referenced this pull request Jun 10, 2020
The Swift bug report: https://bugs.swift.org/browse/SR-11680

Swift nightly toolchains are available here: https://swift.org/download/

The Swift nightly toolchains cannot build OpenCombine. Here's why:

The COpenCombineHelpers target defines a non-static function
(`opencombine_stop_in_debugger`) in a header file. This function is
emitted in the target's IR, but not in the target's TBD.

Swift nightly toolchains have assertions enabled, so they use the
-validate-tbd-against-ir=missing build setting. This build setting
makes the compiler fail if the TBD doesn't match the IR.

This commit un-inlines `opencombine_stop_in_debugger`, so it
is not emitted in the IR. This stops the TBD validator from
complaining.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants