[PATCH] D158707: [analyzer] Fix a few size-type signedness inconsistency related to DynamicExtent

Balázs Benics via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 24 08:48:23 PDT 2023


steakhal added a comment.

As a general comment on requiring all extents to be of unsigned APSInts.
Checkers, like the `MallocChecker`, binds the result of arbitrary expression's values (the argument of malloc, for example) as extents of some regions.
Usually, that argument is of an `unsigned` static type, thus even if some other expression is there, and implicit cast will turn it into `unsigned`.
However, in CSA, we don't respect such casts, thus if it was signed, we will bind a signed value as an extent.

Here is an example <https://godbolt.org/z/jv5e8EoaM> demonstrating this:

  int n = conjure();
  clang_analyzer_dump(n);  // conj
  clang_analyzer_value(n); // i32
  
  char *p = (char*)malloc(n);
  clang_analyzer_dump(clang_analyzer_getExtent(p));  // conj
  clang_analyzer_value(clang_analyzer_getExtent(p)); // i32

Consequently, I'm not sure it's possible to turn extents "unsigned", without fixing casts.
I was sort of expecting this, but I didn't know real world issues materialized by this in the domain of extents.
Usually, they just happen to work, so I didn't really bother with it.
This was actually what I intended to deliver with my comment at D158499#4609046 <https://reviews.llvm.org/D158499#4609046>.

By this comment I'm not suggesting that this is a lost cause, or we shouldn't move in this direction. I think something like this would make sense, but I'd need to delve into the change to make sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158707



More information about the cfe-commits mailing list