Skip to content

Merge r347981 into the 7.0 branch #39194

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

Closed
llvmbot opened this issue Nov 30, 2018 · 7 comments
Closed

Merge r347981 into the 7.0 branch #39194

llvmbot opened this issue Nov 30, 2018 · 7 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang:static analyzer

Comments

@llvmbot
Copy link
Member

llvmbot commented Nov 30, 2018

Bugzilla Link 39847
Resolution FIXED
Resolved on Dec 05, 2018 11:33
Version 7.0
OS All
Blocks #38454
Reporter LLVM Bugzilla Contributor
CC @dkrupp,@devincoughlin,@tstellar
Fixed by commit(s) r348362 r348362

Extended Description

[Analyzer] [HOTFIX!] SValBuilder crash when aggressive-binary-operation-simplification enabled

During the review of D41938 a condition check with an early exit accidentally slipped into a branch, leaving the other branch unprotected. This may result in an assertion later on. This hotfix moves this contition check outside of the branch.

Commit: https://reviews.llvm.org/rC347981

@llvmbot
Copy link
Member Author

llvmbot commented Nov 30, 2018

assigned to @tstellar

@tstellar
Copy link
Collaborator

Artem, is this OK to merge to the release_70 branch?

@tstellar
Copy link
Collaborator

Also Devin, since you are the code owner, what do you think?

@haoNoQ
Copy link
Collaborator

haoNoQ commented Nov 30, 2018

I approve merging. It looks like a very safe patch to me: it merely disables a new feature in certain scenarios, reverting to the old behavior.

@tstellar
Copy link
Collaborator

tstellar commented Dec 5, 2018

Ádám are you able to port this test case to the release_70 branch. It looks like it uses some helper functions that aren't in the branch.

@llvmbot
Copy link
Member Author

llvmbot commented Dec 5, 2018

Oh yes, Artem updated the tests recently using a new feature. I changed it to the old format and committed it to the release_70 branch. However I can only see an LLVM commit (https://reviews.llvm.org/rL348362) but no Clang commit (no https://reviews.llvm.org/rC348362). Also the automatic mail to llvm-branch-commits is waiting for the moderator. I keep this bug open until you confirm that my commit was successful.

@tstellar
Copy link
Collaborator

tstellar commented Dec 5, 2018

I can confirm this was merged: r348362.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:static analyzer
Projects
None yet
Development

No branches or pull requests

3 participants