[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

Mon Sep 26 09:50:17 PDT 2022

ASDenysPetrov reopened this revision.
ASDenysPetrov added a comment.
This revision is now accepted and ready to land.

I've just investigated this patch deeply. I think we have wrong logic here.

Assume we have `(int)(short)(int x)`. `VisitSymbolCast` will try to get the constant recursively in the next order:

- `(short)(int x)`
- `(int x)`

And here is my concern. Whether it is correct to get the range for `int x` and consider it as a correct simplification of `(int)(short)(int x)`. IMO it can't be simplified like that. It should go through the set of conventions and intersections.
Working on an integral cast feature I've encountered in the situation when I can get the range for `(short)(int x)` in the chain above. And it logically matches with the original symbol `(int)(short)(int x)`. After that, your solution (this patch) stops and returns this range, missing the range associated with `int x`.  That borns the problem:

| **before my patch**          | **after my patch**                                        |
| (int)(short)(int x) = 0      | (int)(short)(int x) = 0                                   |
| (short)(int x) = ? / go next | (short)(int x) = 0 / match                                |
| (int x) = 1 / match          | (int x) = 1 / doesn't reach because of the previous match |
| **result**                   | **result**                                                |
| 0 and 1 infeasible           | 0 and 0 feasible                                          |
|

I think we have to improve this behavior.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126481/new/

https://reviews.llvm.org/D126481