[PATCH] D48831: alpha.unix.cstring.OutOfBounds checker enable/disable fix
Balogh, Ádám via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 5 23:21:14 PDT 2018
baloghadamsoftware accepted this revision.
baloghadamsoftware added a comment.
This revision is now accepted and ready to land.
LGTM! But wait for Artem's acceptance before submitting.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:311
+ if (!Filter.CheckCStringOutOfBounds)
+ return StOutBound;
> NoQ wrote:
> > Could we preserve the other portion of the assertion on this branch? I.e., `assert(Filter.CheckCStringNullArg)`.
> > Additionally, do you really want to continue analysis on this path? Maybe `return nullptr` to sink?
> I was unsure whether to return nullptr or StOutBound. I thought that alpha.unix.cstring.OutOfBounds is in alpha because it may falsely detect buffer overflows and then we would cut the path unnecessarily.
> But OK, it is safer this way.
> I could not put back the assertion, because if only unix.Malloc checker is enabled (and CStringOutOfBounds and CStringNullArg are not) the assertion is not true.
I think the assertion could be preserved after the early exit, but it has no point to repeat the same condition in subsequent lines.
More information about the cfe-commits