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

Wrong MC/DC code generation with ternary operator #78453

Closed
chapuni opened this issue Jan 17, 2024 · 4 comments
Closed

Wrong MC/DC code generation with ternary operator #78453

chapuni opened this issue Jan 17, 2024 · 4 comments

Comments

@chapuni
Copy link
Contributor

chapuni commented Jan 17, 2024

(https://godbolt.org/z/qYKKnvbva)

bool foo(bool a, bool b, bool c, bool d, bool e) {
    return (a || b ? c && d : e);
}

This is not rejected (as unsupported) but wrong code is generated.
CoverageGen emits a couple of decisions but CodeGen emits weird code.
Finally, llvm-cov crashes with unexpected test vector.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 17, 2024

@llvm/issue-subscribers-clang-codegen

Author: NAKAMURA Takumi (chapuni)

(https://godbolt.org/z/qYKKnvbva) ``` bool foo(bool a, bool b, bool c, bool d, bool e) { return (a || b ? c && d : e); } ```

This is not rejected (as unsupported) but wrong code is generated.
CoverageGen emits a couple of decisions but CodeGen emits weird code.
Finally, llvm-cov crashes with unexpected test vector.

@chapuni
Copy link
Contributor Author

chapuni commented Jan 17, 2024

Seems this is generic to split decisions, like func(a && b, c && d).
https://godbolt.org/z/5h1dqc4be

Can I retitle?

@evodius96
Copy link
Contributor

The temporary bitmap value for the ternary operator's condition was being saved after its LHS and RHS was evaluated, which means that if the LHS or RHS has a boolean expression, the bitmap value will become corrupted, and the bitmaps for all of the boolean expressions will be wrong. The fix is to ensure that the temporary bitmap value for the ternary operator's condition is saved prior to visiting the LHS and RHS. It's a straightforward fix, but I'll put up a PR soon.

evodius96 added a commit to evodius96/llvm-project that referenced this issue Jan 20, 2024
…and RHS

This is a fix for MC/DC issue llvm#78453 in
which a ConditionalOperator that evaluates a complex condition was incorrectly
updating its global bitmap after visiting its LHS and RHS children.  This was
wrong because if the LHS or RHS also evaluate a complex condition, the MCDC
temporary bitmap value will get corrupted.  The fix is to ensure that the
bitmap is updated prior to visiting the LHS and RHS.
evodius96 added a commit that referenced this issue Jan 22, 2024
…re visiting children (#78814)

This is a fix for MC/DC issue #78453 in which a ConditionalOperator that evaluates a complex condition was incorrectly updating its global bitmap after visiting its LHS and RHS children. This
was wrong because if the LHS or RHS also evaluate a complex condition, the MCDC temporary bitmap value will get corrupted. The fix is to ensure that the bitmap is updated prior to visiting the LHS and RHS.
@chapuni
Copy link
Contributor Author

chapuni commented Jan 23, 2024

Thanks!

@chapuni chapuni closed this as completed Jan 23, 2024
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

4 participants