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

Fix passes that introduce new branches on undef/poison #45489

Closed
fhahn opened this issue May 31, 2020 · 5 comments
Closed

Fix passes that introduce new branches on undef/poison #45489

fhahn opened this issue May 31, 2020 · 5 comments
Labels

Comments

@fhahn
Copy link
Contributor

fhahn commented May 31, 2020

Bugzilla Link 46144
Version trunk
OS All
Depends On #45301
CC @efriedma-quic,@eugenis,@LebedevRI,@nunoplopes,@regehr

Extended Description

Currently there are some passes that may introduce new branches on undef/poison, which h potentially introduces UB where there was none before.

While the LangRef was clear about branch on poison being UB, recently it was also clarified that this applies to branches on undef as well (https://reviews.llvm.org/rG05f0e598ab265a80fedb23225cde4176f11774ac).

There has been some discussion about problematic passes (https://reviews.llvm.org/D76973,
https://reviews.llvm.org/D80875.

Nuno mentioned "The passes we are aware that introduce branch on poison are: IndVarSimplify, LoopUnswitch, SimpleLoopUnswitch, and SimpifyCFG.
(https://web.ist.utl.pt/nuno.lopes/alive2/index.php?hash=4beb2b117e2fdd2c)"

Eli mentioned that some passes condition certain transforms on theAttribute::SanitizeMemoryattribute, because of MemSAN does not like new branches on undef/poison values. See also #28428

@eugenis
Copy link
Contributor

eugenis commented Jun 1, 2020

It great to see that LLVM IR semantics is converging with MSan.

@fhahn
Copy link
Contributor Author

fhahn commented Jun 19, 2020

For now, we decided to delay a few changes in LLVM until there's some progress on this. In particular https://reviews.llvm.org/D80875 and marking ranges from branch conditions as not containing undef in SCCP (was part of https://reviews.llvm.org/D81756)

@fhahn
Copy link
Contributor Author

fhahn commented May 10, 2022

SimpleLoopUnswitch has been fixed, LoopUnswitch has been removed.

@nikic
Copy link
Contributor

nikic commented May 10, 2022

From https://web.ist.utl.pt/nuno.lopes/alive2/index.php?hash=7fc0899d6af2b9f6 remaining issues are:

Sorry, something went wrong.

@nikic
Copy link
Contributor

nikic commented May 30, 2022

I believe all known issues are fixed now. Let's open new issues if something more turns up.

There's one more followup for CGP (https://reviews.llvm.org/D126638), but it's not a branch on poison issue anymore.

@nikic nikic closed this as completed May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants