This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Introduce range-based reasoning for subtraction operator
Needs ReviewPublic

Authored by manas on Jul 1 2021, 4:15 AM.

Details

Summary

Add logic for computing rangesets for symbolic subtraction operator and
add test cases.

Diff Detail

Event Timeline

manas created this revision.Jul 1 2021, 4:15 AM
manas requested review of this revision.Jul 1 2021, 4:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2021, 4:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Hey Manas! Great job, you put this together real quick!

clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
1481

Maybe you can include this as yet another condition in the next if statement? Their bodies are identical.

clang/test/Analysis/constant-folding.c
399

Maybe you can check (a - b) > 5 && (a - b) < UINT_MAX - 9 to cover the whole range?

405

This is also Min and Max overflowing on the positive side.

manas marked 3 inline comments as done.Jul 2 2021, 11:45 AM

Thanks Valeriy.

clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
1481

True. I wanted to optimize the following if condition as it looks quite ugly right now! Although, I will concatenate these conditions for now and work on the optimization later.

clang/test/Analysis/constant-folding.c
399

That's true! I never thought of putting them together. Makes more sense.

405

I missed it! I will add another case for only Max overflowing on positive side and will keep this test as well.

manas updated this revision to Diff 356235.Jul 2 2021, 11:47 AM
manas marked 3 inline comments as done.

Merge conditionals with similar block and add test for one overflow on Tmax-side

manas added inline comments.Jul 2 2021, 11:49 AM
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
1485

@vsavchenko one thing crossed my mind is that, shouldn't I compare From and To values with llvm::APSInt Zero = ValueFactory.getAPSIntType(T).getZeroValue() instead of literal 0?

vsavchenko added inline comments.Jul 2 2021, 11:56 AM
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
1485

Yes! Thanks for noticing this (the same goes to the other patch as well)

manas updated this revision to Diff 356245.Jul 2 2021, 12:22 PM

Replace literal-value 0 with APSInt Zero

manas marked an inline comment as done.Jul 2 2021, 12:22 PM
manas updated this revision to Diff 356249.Jul 2 2021, 12:38 PM

Remove redundant getAPSIntType calls

manas added a comment.Aug 3 2021, 7:54 AM

I have updated the proof for Sub.