This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Extend SuppressInlineDefensiveChecksVisitor to all macros, including non-function-like ones
ClosedPublic

Authored by george.karpenkov on Jan 23 2018, 3:01 PM.

Details

Summary

It seems there is no particular reason to treat function-like macros differently here.
The change is currently being evaluated on the analyzer benchmark suite, let's see whether removed issues would be true or false positives.

Tracked in rdar://29907377

Diff Detail

Event Timeline

So evaluation shows on a whole corpus we have removed two warnings on postgresql:

REMOVED: "lwlock.c:429:19, Logic error: Access to field 'lwWaiting' results in a dereference of a null pointer (loaded from variable 'proc')"
REMOVED: "list.c:194:6, Logic error: Access to field 'tail' results in a dereference of a null pointer (loaded from variable 'list')"

Unfortunately, at least one of those is definitely a true positive.
However, the underlying root cause seems to be a more general case: we suppress nullability reports from macros EVEN if the bug is there even without a macro.

Actually, I think it should go in.
The true positive is removed due to an unrelated bug, which could even happen with function-like macros:

#define NOPS(X) if (X == 0) {}

int foo(int *x) {
    NOPS(x);
    if (x == 0) {}
    return *x;
}

The second removed issue is a false positive, and it is removed due to the fact that isFunctionMacroExpansion check is buggy, as demonstrated by:

#define F(X) if (!X) {}
#define C(X) (X == 0 || *X > 10)

int foo(int *x) {
  C(x);
  return *x;
}
NoQ accepted this revision.Jan 26 2018, 3:56 PM

Yeah, makes sense to me. Hopefully we're not missing any common valid patterns.

This revision is now accepted and ready to land.Jan 26 2018, 3:56 PM
This revision was automatically updated to reflect the committed changes.