1192015 - Refactor clang-plugin's allocation analysis logic

Status: RESOLVED FIXED

(Firefox Build System :: Source Code Analysis, defect)

Comment 1

[:Nika Layzell (on leave)]

Attachment: Refactor allocation analysis in clang plugin to better allow adding new analyses, and support detecting temporary value allocations

try in progress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58f9985abac

I haven't run the new analylsis on the whole tree yet, so the try may fail and need extra changes.

Comment 2

[:ehsan akhgari]

Comment on attachment 8644568 [details] [diff] [review]
Refactor allocation analysis in clang plugin to better allow adding new analyses, and support detecting temporary value allocations

Review of attachment 8644568 [details] [diff] [review]:
-----------------------------------------------------------------

Very nice!

::: build/clang-plugin/clang-plugin.cpp
@@ +930,3 @@
>      return false;
> +  if (const FunctionDecl *Decl = Expr->getOperatorNew()) {
> +    if (MozChecker::hasCustomAnnotation(Decl, "moz_heap_allocator"))

Nit: please use && instead of nested ifs.

@@ +1034,5 @@
> +  astMatcher.addMatcher(varDecl().bind("node"), &scopeChecker);
> +  astMatcher.addMatcher(newExpr().bind("node"), &scopeChecker);
> +  astMatcher.addMatcher(materializeTemporaryExpr().bind("node"), &scopeChecker);
> +  astMatcher.addMatcher(callExpr(callee(functionDecl(heapAllocator()))).bind("node"),
> +                        &scopeChecker);

Do you mind moving this back up to the beginning of DiagnosticsMatcher::DiagnosticsMatcher, please?

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d6871d66dc0

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0d6871d66dc0