In order to make sure we don't regress we should also run our tests in bulk mode, all tests passed to clang-tidy in one execution pipe.

Unfortunately I've discovered an issue that is present in clang-tidy 5.0 till 6.0.1, running: clang-tidy -checks=-*,bugprone-suspicious-memset-usage,clang-analyzer-cplusplus.NewDelete,clang-analyzer-cplusplus.NewDeleteLeaks,clang-analyzer-deadcode.DeadStores,clang-analyzer-security.FloatLoopCounter,clang-analyzer-security.insecureAPI.getpw,clang-analyzer-security.insecureAPI.mkstemp,clang-analyzer-security.insecureAPI.mktemp,clang-analyzer-security.insecureAPI.UncheckedReturn,clang-analyzer-security.insecureAPI.vfork,clang-analyzer-unix.Malloc,clang-analyzer-unix.cstring.BadSizeArg,clang-analyzer-unix.cstring.NullArg,misc-argument-comment,misc-assert-side-effect,misc-bool-pointer-implicit-conversion,misc-forward-declaration-namespace,misc-macro-repeated-side-effects,misc-string-constructor,misc-string-integer-assignment,misc-suspicious-missing-comma,misc-suspicious-semicolon,misc-swapped-arguments,misc-unused-alias-decls,misc-unused-raii,misc-unused-using-decls,modernize-avoid-bind,modernize-loop-convert,modernize-raw-string-literal,modernize-shrink-to-fit,modernize-use-bool-literals,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,performance-faster-string-find,performance-for-range-copy,performance-inefficient-string-concatenation,performance-inefficient-vector-operation,performance-type-promotion-in-math-fn,performance-unnecessary-copy-initialization,performance-unnecessary-value-param,readability-container-size-empty,readability-else-after-return,readability-misleading-indentation,readability-redundant-control-flow,readability-redundant-string-cstr,readability-redundant-string-init,readability-uniqueptr-delete-release -extra-arg=-DMOZ_CLANG_PLUGIN -p=/var/folders/21/5g_w1s552c3c60dtq5_74djw0000gn/T/ccSK4Ktr /Users/abpostelnicu/Projects/mozilla/mozilla-unified/tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp we get: clang-analyzer-unix.cstring.NullArg.cpp:7:10: warning: Null pointer argument in call to string length function [clang-analyzer-cplusplus.NewDeleteLeaks] return strlen(s); // warning ^ clang-analyzer-unix.cstring.NullArg.cpp:12:3: note: 's' initialized to a null pointer value const char* s = nullptr; ^ clang-analyzer-unix.cstring.NullArg.cpp:13:20: note: Passing null pointer value via 1st parameter 's' return my_strlen(s); ^ clang-analyzer-unix.cstring.NullArg.cpp:13:10: note: Calling 'my_strlen' return my_strlen(s); ^ clang-analyzer-unix.cstring.NullArg.cpp:7:10: note: Null pointer argument in call to string length function return strlen(s); // warning ^ Suppressed 3 warnings (2 in non-user code, 1 with check filters). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. abpostelnicu@Mozilla-MacBook-Pro:~/Desktop$ code /Users/abpostelnicu/Projects/mozilla/mozilla-unified/tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp The category of the deffect is wrong, it should be: clang-analyzer-unix.cstring.NullArg This issue has been fixed in 7.0, but i didn't look to see what patch has fixed it. Jan do you think it's possible to skip 6.0 and move directly to 7.0, even though 7.0 I don't think it's released yet?

(In reply to Andi-Bogdan Postelnicu [:andi] from comment #2) > Unfortunately I've discovered an issue that is present in clang-tidy 5.0 > till 6.0.1 I was able to reproduce this on Ubuntu 16.04 with clang-tidy 6.0.1. The bug happens because both checks "clang-analyzer-cplusplus.NewDeleteLeaks" and "clang-analyzer-unix.cstring.NullArg" seem to find the same defect, and for some reason when both checks are active, "NewDeleteLeaks" has the priority to report it: 1) Running with "NullArg" only: $ clang-tidy-6.0 -checks=-*,clang-analyzer-unix.cstring.NullArg tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp [...] /home/user/firefox/tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp:7:10: warning: Null pointer argument in call to string length function [clang-analyzer-unix.cstring.NullArg] 2) Running with "NewDeleteLeaks" only: $ clang-tidy-6.0 -checks=-*,clang-analyzer-cplusplus.NewDeleteLeaks tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp [...] /home/user/firefox/tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp:7:10: warning: Null pointer argument in call to string length function [clang-analyzer-cplusplus.NewDeleteLeaks] 3) Running with both "NullArg" and "NewDeleteLeaks" (check order doesn't matter): $ clang-tidy-6.0 -checks=-*,clang-analyzer-unix.cstring.NullArg,clang-analyzer-cplusplus.NewDeleteLeaks tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp [...] /home/user/firefox/tools/clang-tidy/test/clang-analyzer-unix.cstring.NullArg.cpp:7:10: warning: Null pointer argument in call to string length function [clang-analyzer-cplusplus.NewDeleteLeaks] > This issue has been fixed in 7.0, but i didn't look to see what patch has > fixed it. Could you please find the patch? It would be good to back-port the fix into our tree. > Jan do you think it's possible to skip 6.0 and move directly to 7.0, even > though 7.0 I don't think it's released yet? 7.0 is indeed not yet released: https://llvm.org/svn/llvm-project/llvm/tags/ I'm not sure whether we can do such a thing. Maybe it's better for now to back-port the fix? (Note that bug 1466427 is upgrading 5.0.1 to 6.0.1, but that can happen independently of back-porting this fix.)

> WIP WIP - Bug 1480089 - pass all of the test files to to our static-analysis pipeline. Nits: - Remove one "WIP" - Start commit message with a capital letter - Improve commit message clarity, e.g. like so: "Run individual static-analysis tests all at once in order to cover analysis of multiple files." (or simply "Test './mach static-analysis check' on multiple files.")

(In reply to Jan Keromnes [:janx] from comment #4) > (In reply to Andi-Bogdan Postelnicu [:andi] from comment #2) > > Jan do you think it's possible to skip 6.0 and move directly to 7.0, even > > though 7.0 I don't think it's released yet? > > 7.0 is indeed not yet released: https://llvm.org/svn/llvm-project/llvm/tags/ > > I'm not sure whether we can do such a thing. Maybe it's better for now to > back-port the fix? (Note that bug 1466427 is upgrading 5.0.1 to 6.0.1, but > that can happen independently of back-porting this fix.) Actually, maybe we could use this: https://llvm.org/svn/llvm-project/llvm/branches/release_70/ ? The file structure looks similar to what we use currently: https://llvm.org/svn/llvm-project/llvm/tags/RELEASE_501/final/ However, using a "branch" instead of a "tag" probably has major implications like "the code might change every now and then", which would probably mess up our hashes and cached binaries.

Since clang 7 rc1 has been released we should try with that version, and for this there is a tag: https://llvm.org/svn/llvm-project/llvm/tags/RELEASE_700/rc1/

One more thing that i want to mention, in release_70 we have also this checker: https://github.com/llvm-mirror/clang/blob/release_70/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp that would be nice to activate it for our analysis.

Since we've updated to clang 7.0 rc2 i've finished this patch, let's see how it works on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c0ea0ff137af4046d6e6da75a67da3a6865884f5

Windows passed, let's see linux: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b24d212fd85a9ac467ab684d41cd03caa812239

Comment on attachment 9005158 [details] Bug 1480089 - pass all of the test files to to our static-analysis pipeline. r=janx Jan Keromnes [:janx] has approved the revision.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=17fc2af9fcbb4c94a05e7aca55a6df2d1611a801 https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5bfb3a9a2e9da74b532f95ec8bc3ee76edea75f

Everything works just fine, pushing it with lando!

Pushed by bpostelnicu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7082bd230c3c pass all of the test files to to our static-analysis pipeline. r=janx

bravo, much nicer!