[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858 David Malcolm changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #5 from David Malcolm --- Thanks again for filing this. Should be fixed by the above commit.
[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858 --- Comment #4 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:2fc201382d3498778934f1262b57acd20f76f300 commit r11-2855-g2fc201382d3498778934f1262b57acd20f76f300 Author: David Malcolm Date: Fri Aug 21 18:34:16 2020 -0400 analyzer: fix leak false positive/widening on pointer iteration [PR94858] PR analyzer/94858 reports a false diagnostic from -Wanalyzer-malloc-leak, where the allocated pointer is pointed to by a field of a struct, and a loop writes to a buffer, writing through an iterating pointer value. There were several underlying problems, relating to clobbering of the struct holding the malloc-ed pointer; in each case the analyzer was conservatively assuming that a write could affect this region, clobbering it to "unknown", and this was detected as a leak. The initial write within the loop dereferences the initial value of a field, and the analyzer was assuming that that pointer could point to the result of the malloc call. The patch extends store::eval_alias_1 so that it "knows" that the initial value of a pointer at the beginning of a path can't point to a region that was allocated on the heap after the beginning of the path. On fixing that, the next issue is that within the loop the iterated pointer value becomes "unknown", and hence *ptr becomes a write to a symbolic region, and thus might clobber the struct (which it can't). This patch adds enough logic to svalue::can_merge_p to merge the iterating pointer value so that at the 2nd iteration analyzing the loop it becomes a widening_svalue from the initial svalue, so that this becomes a fixed point of the analysis, and is not an unknown_svalue. The patch further extends store::eval_alias_1 so that it "knows" that this widening_svalue can only point to the same base region as the initial value did; in particular, symbolic writes through this pointer can only clobber that base region, not the struct holding the malloc-ed pointer. gcc/analyzer/ChangeLog: PR analyzer/94858 * region-model-manager.cc (region_model_manager::get_or_create_widening_svalue): Assert that neither of the inputs are themselves widenings. * store.cc (store::eval_alias_1): The initial value of a pointer can't point to a region that was allocated on the heap after the beginning of the path. A widened pointer value can't alias anything that the initial pointer value can't alias. * svalue.cc (svalue::can_merge_p): Merge BINOP (X, OP, CST) with X to a widening svalue. Merge BINOP(WIDENING(BASE, BINOP(BASE, X)), X) and BINOP(BASE, X) to to the LHS of the first BINOP. gcc/testsuite/ChangeLog: PR analyzer/94858 * gcc.dg/analyzer/loop-start-up-to-end-by-1.c: Remove xfail. * gcc.dg/analyzer/pr94858-1.c: New test. * gcc.dg/analyzer/pr94858-2.c: New test. * gcc.dg/analyzer/torture/loop-inc-ptr-2.c: Update expected number of enodes. * gcc.dg/analyzer/torture/loop-inc-ptr-3.c: Likewise.
[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858
--- Comment #3 from David Malcolm ---
It looks like we lose the:
cluster for: (*INIT_VAL(td_14(D)))
ESCAPED
key: {kind: direct, start: 0, size: 64, next: 64}
value: ‘hashNx *’ {&HEAP_ALLOCATED_REGION(0)}
at the "*index.0_1 = -1;", where the whole cluster for *INIT_VAL(td_14(D))
becomes unknown.
I think the aliasing code conservatively assumes that this write could affect
the *INIT_VAL(td_14(D)).
But that was the initial value of a pointer param, whereas
HEAP_ALLOCATED_REGION is a freshly allocated heap region, so they can't alias.
Looks like I need to teach that to the pointer-aliasing logic.
[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858
David Malcolm changed:
What|Removed |Added
Last reconfirmed||2020-08-19
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
--- Comment #2 from David Malcolm ---
Thanks for filing this. Still affects trunk (my rewrite of
r11-2694-g808f4dfeb3a95f50f15e71148e5c1067f90a126d didn't fix it).
Seems to be an issue with state-merging; passing -fno-analyzer-state-merge
eliminates the false positive.
Immediately before the call to hashEmpty we have:
cluster for: (*INIT_VAL(td_14(D)))
ESCAPED
key: {kind: direct, start: 0, size: 64, next: 64}
value: ‘hashNx *’ {&HEAP_ALLOCATED_REGION(0)}
key: {kind: direct, start: 96, size: 32, next: 128}
value: ‘int’ {INIT_VAL(slots_15(D))}
malloc: &HEAP_ALLOCATED_REGION(0): nonnull (‘index_17’)
but afterwards (with state merging) we have:
cluster for: (*INIT_VAL(td_14(D)))
ESCAPED
key: {kind: direct, start: 0, size: 64, next: 64}
value: ‘hashNx *’ {UNKNOWN(hashNx *)}
key: {kind: direct, start: 64, size: 32, next: 96}
value: ‘int’ {(int)0}
key: {kind: direct, start: 96, size: 32, next: 128}
value: ‘int’ {UNKNOWN(int)}
key: {kind: default, start: 0, size: 128, next: 128}
value: ‘struct hashSt’ {UNKNOWN(struct hashSt)}
so the pointer (first 64 bits) has somehow become UNKNOWN.
[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858 Bug 94858 depends on bug 94839, which changed state. Bug 94839 Summary: False positive with -fanalyzer and direct field assignment from calloc https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94839 What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED
[Bug analyzer/94858] False report of memory leak
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94858 Andrew Pinski changed: What|Removed |Added Depends on||94839 --- Comment #1 from Andrew Pinski --- I think this is an almost exact dup of bug 94839 (which I filed yesterday). Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94839 [Bug 94839] False positive with -fanalyzer and direct field assignment from calloc
