[4.1] IRGen: Fix witness-table accessors for conditional conformances

[aschwaighofer]

Explanation: After adding conditional conformances witness table
accessors are no longer readnone because the witnesses are passed as an
in memory argument. In optimized mode LLVM will optimize away the store
of the witnesses because it believes the accessor function is readnone
leading to crashes

Scope: Introduced with conditional conformances, potentially affects any project
that emit witness-table accessors on based types with conditional
conformances

Risk: Very low since the fix is to not emit LLVM's readnone attribute
(which should do no harm)

Testing: The project on which this was reported was tested and a CI test
added

SR-7228
rdar://38624842

[aschwaighofer]

@swift-ci Please test

[aschwaighofer]

@huonw Can you review this change?

[aschwaighofer]

@swift-ci please nominate

[aschwaighofer]

This is a cherry-pick of #15397

[aschwaighofer]

@shahmishal In case this is interesting to you and not just temporary fallout: https://ci.swift.org/job/swift-PR-osx/3839/
10:56:36 FATAL: command execution failed
10:56:36 java.io.IOException

Restarting the test.

[aschwaighofer]

@swift-ci Please test

[DougGregor]

LGTM, thanks for catching this.

I fixed a similar problem not too long ago with a rogue "readnone" that caused obscure runtime failures. Should we perform a holistic audit of the use of these attributes in IRGen?

[aschwaighofer]

I went over all instances of the following in IRGen on master:

ReadNone
ReadOnly
setOnlyReadsMemory
setDoesNotAccessMemory

[huonw]

This is unfortunate. Thank you!

I wonder if it'd be worth having a verifier that validates that that there's no static readnone calls to non-readnone functions to protect against bugs like this in future, since it seems like a trivial thing to check and very problematic in non-obvious ways when wrong.