NagyDonat wrote: This is a clear example for a bug type where the simple AST matching that can be done in a diagnostic warning is insufficient and the path-sensitive analysis of the clang static analyzer provides much better reports.
The AST pattern matching does not follow the control flow and cannot deduce things like "when the value is negative, this never gets executed" -- while the symbolic execution process of static analyzer is designed exactly for these considerations. (We could introduce with hacks like "if there is an if between these two code fragments, then inspect its condition and ...", but these would just "reinvent the wheel" and quickly become impossible to manage.) Unfortunately the analyzer checkers are more costly (in terms of runtime) and less accessible than diagnostic warnings, but I still feel that it would be better to replace this diagnostic option with a static analyzer checker that can do the job properly. (In fact, the static analyzer already has the [`unix.Malloc`](https://clang.llvm.org/docs/analyzer/checkers.html#unix-malloc-c) checker which reports various bugs related to `Malloc`. If I recall correctly, zero-sized allocations are already modeled and reported when they are illegal; while negative-sized allocations are not currently covered but would be easy to handle.) ------- > > It also appears to have issues when the thing receiving the malloc is > > defined as a union, and the malloc allocates only the size of one of the > > smaller union options. > > I am not sure if that is defined behavior or not and I can't find any > information about that. To me, warning in that case makes sense because > dereferencing a larger field would access unallocated memory. Casts between pointer to union and pointer to a member of the union have well-defined behavior [(second paragraph in linked section)](https://en.cppreference.com/w/c/language/union.html#Explanation), so I'm sure that the following indirect approach doesn't trigger UB: ```c union u { short small; long big; }; void f() { union u *p = (union u*)(short *)malloc(sizeof(short)); p->small = 42; do_something_with(p); free(p); } ``` Omitting the `(short *)` cast would clearly work in practice and I would presume that it's well-defined according to the standard, but I'm not entirely sure about this. On the other hand, accessing a member of the union which is different from the one that was used to create it produces unspecified results. In your "dereferencing a larger field would access unallocated memory" example I would say that the problematic step is the one where the union is accessed through a member that is different from the first one. (It is fair to say that casting the small allocated memory to the union type is a _suspicious_ step. However I would guess that this would only happen rarely and only in situations when this suspicious solution is intentional and needed, so reporting it as a compiler warning isn't helpful for the developer.) https://github.com/llvm/llvm-project/pull/150028 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
