Page MenuHomePhabricator

[Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184)
ClosedPublic

Authored by MaggieYi on Aug 19 2015, 10:45 AM.

Details

Summary

Dear All,

I would like to propose a patch to solve an assertion failure reported by Dmitry in https://llvm.org/bugs/show_bug.cgi?id=24184.

The assertion is caused by reusing a “filler” ExplodedNode as an error node. The “filler” nodes are only used for intermediate processing and are not essential for analyzer history, so they can be reclaimed when the ExplodedGraph is trimmed by the “collectNode” function. When a checker finds a bug, they generate a new transition in the ExplodedGraph. The analyzer will try to reuse the existing predecessor node. If it cannot, it creates a new ExplodedNode, which always has a tag to uniquely identify the creation site. The assertion is caused when the analyzer reuses a “filler” node.

In the test case, some “filler” nodes were reused and then reclaimed later when the ExplodedGraph was trimmed. This caused an assertion because the node was needed to generate the report. The “filler” nodes should not be reused as error nodes. The patch adds a constraint to prevent this happening, which solves the problem and makes the test cases pass.

Please let me know if this is an acceptable patch.

Regards,

Ying Yi
SN Systems Ltd - Sony Computer Entertainment Group.

Diff Detail

Event Timeline

MaggieYi updated this revision to Diff 32574.Aug 19 2015, 10:45 AM
MaggieYi retitled this revision from to [Patch] [Analyzer] BugReporter.cpp:2869: Assertion failed: !RemainingNodes.empty() && "No error node found in the trimmed graph" (PR 24184).
MaggieYi updated this object.
MaggieYi added a reviewer: krememek.
MaggieYi added a subscriber: cfe-commits.
zaks.anna edited edge metadata.Aug 19 2015, 7:00 PM

I have some minor nits but looks good otherwise. Thanks for fixing this!

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
290–293

Please, remove the reference to the method name, the comment might get stale if the function name changes. Also, please, remove the PR reference from here.

test/Analysis/malloc.c
1389–1390

Some wordsmithing: Object "a->p" is returned after being freed by calling "realloc".

MaggieYi updated this revision to Diff 32671.Aug 20 2015, 2:22 AM
MaggieYi edited edge metadata.

Hi Anna,

Many thanks for your comments. I have modified the patch to address your comments. Please let me know what you think.

PS: If the updated patch looks good to you, could you please commit it for me (as I do not have commit access) ?

Many thanks,
Ying Yi

zaks.anna accepted this revision.Aug 27 2015, 11:40 AM
zaks.anna edited edge metadata.
This revision is now accepted and ready to land.Aug 27 2015, 11:40 AM
xazax.hun closed this revision.Aug 27 2015, 11:59 AM
xazax.hun added a subscriber: xazax.hun.

Committed in http://reviews.llvm.org/rL246188.

Thanks for the patch.

Thanks for helping me review and submit the patch.