The V8 code in the new engine uses implicit constructors with wild abandon, and it doesn't seem like a good use of time to try upstreaming a patch to remove them.
Depends on D71355
Differential Revision: https://phabricator.services.mozilla.com/D71356
The key here is to test the type of the variable declaration for being a
smartptr type, instead of testing the type of the variable _use_.
Differential Revision: https://phabricator.services.mozilla.com/D65581
--HG--
extra : moz-landing-system : lando
This is to fix build bustage until Thunderbird engineers are able to migrate
these threads to use background tasks.
Differential Revision: https://phabricator.services.mozilla.com/D64795
--HG--
extra : moz-landing-system : lando
Creates a NoNewThreadsChecker plugin that looks for the function and checks to see if it's allowed.
-Creates two allowlists - `ThreadAllows.txt` is for thread names, while `ThreadFileAllows.txt` checks for entire files where instances of `NS_NewNamedThread` should be ignored.
-If an instance of `NS_NewNamedThread` is not listed in either list, then the checker throws an error with an additional note containing a more descriptive explanation of the failure.
Differential Revision: https://phabricator.services.mozilla.com/D62635
--HG--
extra : moz-landing-system : lando
Creates a NoNewThreadsChecker plugin that looks for the function and checks to see if it's allowed.
-Creates two allowlists - `ThreadAllows.txt` is for thread names, while `ThreadFileAllows.txt` checks for entire files where instances of `NS_NewNamedThread` should be ignored.
-If an instance of `NS_NewNamedThread` is not listed in either list, then the checker throws an error with an additional note containing a more descriptive explanation of the failure.
Differential Revision: https://phabricator.services.mozilla.com/D62635
--HG--
extra : moz-landing-system : lando
GENERATED_FILES now defaults to python3 unless py2=True is specified as
an argument. All existing GENERATED_FILES scripts and GeneratedFile
templates have the py2=True attribute added, so this patch should
effectively be a no-op.
Going forward, individual scripts can be converted to python3 and their
corresponding py2=True attribute can be deleted. In effect, this patch
will be backed out in pieces until all scripts run in python3, at which
point the py2 attribute itself can be removed.
Differential Revision: https://phabricator.services.mozilla.com/D60919
--HG--
extra : moz-landing-system : lando
GENERATED_FILES now defaults to python3 unless py2=True is specified as
an argument. All existing GENERATED_FILES scripts and GeneratedFile
templates have the py2=True attribute added, so this patch should
effectively be a no-op.
Going forward, individual scripts can be converted to python3 and their
corresponding py2=True attribute can be deleted. In effect, this patch
will be backed out in pieces until all scripts run in python3, at which
point the py2 attribute itself can be removed.
Differential Revision: https://phabricator.services.mozilla.com/D60919
--HG--
extra : moz-landing-system : lando
GENERATED_FILES now defaults to python3 unless py2=True is specified as
an argument. All existing GENERATED_FILES scripts and GeneratedFile
templates have the py2=True attribute added, so this patch should
effectively be a no-op.
Going forward, individual scripts can be converted to python3 and their
corresponding py2=True attribute can be deleted. In effect, this patch
will be backed out in pieces until all scripts run in python3, at which
point the py2 attribute itself can be removed.
Differential Revision: https://phabricator.services.mozilla.com/D60919
--HG--
extra : moz-landing-system : lando
This ensures that we don't pass non-trivially-copiable types through the FFI
boundary.
Differential Revision: https://phabricator.services.mozilla.com/D61626
--HG--
extra : moz-landing-system : lando
This is a re-work of `https://mzl.la/31Bkl47`, and because time passed and not all of the
issues reported by this checker were fixed and we want to land it, we've decided to move
from errors diagnostic messages to warnings.
Differential Revision: https://phabricator.services.mozilla.com/D61753
--HG--
extra : moz-landing-system : lando
Starting with `Clang-10`, and in particular with `https://reviews.llvm.org/D66404`,
the `CFG` has been reflown in order to reduce discrepancies in destructor calls with
the actual codegen. This has also impacted the order of `CFGStmt` for `ReturnStmt`,
whereas now the AST children of the `ReturnStmt` have precedence other their parent
node.
The purpose of this patch is to fix this and add new supported cases for the `Expr`
that is generated from `ReturnStmt`.
Differential Revision: https://phabricator.services.mozilla.com/D61556
--HG--
extra : moz-landing-system : lando
With clang 10 and up `getTemporary` has been replaced with the more versatile `getSubExpr`.
The commit that added this behaviour is: b0561b3346
Differential Revision: https://phabricator.services.mozilla.com/D54101
--HG--
extra : moz-landing-system : lando
In the future we should re-write this checker but for now this solution is acceptable.
Differential Revision: https://phabricator.services.mozilla.com/D51242
--HG--
extra : moz-landing-system : lando
This is just drive-by, but it confused me. We require at least clang 4 to build
nowadays.
Differential Revision: https://phabricator.services.mozilla.com/D50646
--HG--
extra : moz-landing-system : lando
sccache-dist / icecream preprocess the file and then send it to a builder.
That means that the file contains #line directives or equivalents in order for
diagnostics and such to work correctly.
Unfortunately our clang-plugin build fails catastrophically if you include a
third-party header, as it doesn't account for them.
Use SourceManager::getPresumedLoc to get filenames, as it accounts for them,
unlike just getFilename.
Differential Revision: https://phabricator.services.mozilla.com/D50633
--HG--
extra : moz-landing-system : lando
In clang 9, the signature of `registerPPCallbacks` changed and it no longer provides us a `CompilerInstance` [0]. I couldn't find a way to reach the `CompilerInstance` from the new structures, so I've inserted a level into the class hierarchy that mimics the old functionality.
With this patch, clang-tidy compiles with either clang-8 or clang-9, so we can land this independently in preparation for bug 1573211.
[0] For reference: the new signature was introduced in bbc89dcb29, the old overload was marked deprecated in e9087fe75c and removed in ee737a84d7.
Differential Revision: https://phabricator.services.mozilla.com/D46702
--HG--
extra : moz-landing-system : lando
It doesn't really make sense to enforce this behavior for calls on stack
variables (which presumably we then want to do something with) and doesn't
necessarily make sense to enforce it for member variables either.
Differential Revision: https://phabricator.services.mozilla.com/D46105
--HG--
extra : moz-landing-system : lando
In some libstdc++ these appear in the inheritance hierarchy of __atomic_base, which is already whitelisted.
Differential Revision: https://phabricator.services.mozilla.com/D42348
--HG--
extra : moz-landing-system : lando
These tests are based on `moz_global_class` and `moz_trivial_ctor_dtor` tests,
respectively, but adapted for the semantics of the new attributes.
Differential Revision: https://phabricator.services.mozilla.com/D39718
--HG--
rename : build/clang-plugin/tests/TestGlobalClass.cpp => build/clang-plugin/tests/TestStaticLocalClass.cpp
rename : build/clang-plugin/tests/TestTrivialCtorDtor.cpp => build/clang-plugin/tests/TestTrivialDtor.cpp
extra : moz-landing-system : lando
This patch is in support of adding a variant of Static{Auto,Ref}Ptr for use as
static locals, taking advantage of C++11 "magic statics" such that we can lazily
initialize those variables in a thread-safe way.
In support of those classes, this patch adds two new attributes:
* `moz_static_local_class` to ensure that any instantiations of that class only
occur as static local variables;
* `moz_trivial_dtor` to ensure that these classes do not implicitly call `atexit`
and add a whole bunch of shutdown crap.
`moz_static_local_class` works similarly to `moz_global_class`, except that its
object must only instantiate as static locals.
`TrivialDtorChecker` is based on `TrivialCtorDtorChecker`, with the ctor-specific
bits removed.
Differential Revision: https://phabricator.services.mozilla.com/D39717
--HG--
rename : build/clang-plugin/TrivialCtorDtorChecker.cpp => build/clang-plugin/TrivialDtorChecker.cpp
rename : build/clang-plugin/TrivialCtorDtorChecker.h => build/clang-plugin/TrivialDtorChecker.h
extra : moz-landing-system : lando
These tests are based on `moz_global_class` and `moz_trivial_ctor_dtor` tests,
respectively, but adapted for the semantics of the new attributes.
Differential Revision: https://phabricator.services.mozilla.com/D39718
--HG--
rename : build/clang-plugin/tests/TestGlobalClass.cpp => build/clang-plugin/tests/TestStaticLocalClass.cpp
rename : build/clang-plugin/tests/TestTrivialCtorDtor.cpp => build/clang-plugin/tests/TestTrivialDtor.cpp
extra : moz-landing-system : lando
This patch is in support of adding a variant of Static{Auto,Ref}Ptr for use as
static locals, taking advantage of C++11 "magic statics" such that we can lazily
initialize those variables in a thread-safe way.
In support of those classes, this patch adds two new attributes:
* `moz_static_local_class` to ensure that any instantiations of that class only
occur as static local variables;
* `moz_trivial_dtor` to ensure that these classes do not implicitly call `atexit`
and add a whole bunch of shutdown crap.
`moz_static_local_class` works similarly to `moz_global_class`, except that its
object must only instantiate as static locals.
`TrivialDtorChecker` is based on `TrivialCtorDtorChecker`, with the ctor-specific
bits removed.
Differential Revision: https://phabricator.services.mozilla.com/D39717
--HG--
rename : build/clang-plugin/TrivialCtorDtorChecker.cpp => build/clang-plugin/TrivialDtorChecker.cpp
rename : build/clang-plugin/TrivialCtorDtorChecker.h => build/clang-plugin/TrivialDtorChecker.h
extra : moz-landing-system : lando
These tests are based on `moz_global_class` and `moz_trivial_ctor_dtor` tests,
respectively, but adapted for the semantics of the new attributes.
Differential Revision: https://phabricator.services.mozilla.com/D39718
--HG--
rename : build/clang-plugin/tests/TestGlobalClass.cpp => build/clang-plugin/tests/TestStaticLocalClass.cpp
rename : build/clang-plugin/tests/TestTrivialCtorDtor.cpp => build/clang-plugin/tests/TestTrivialDtor.cpp
extra : moz-landing-system : lando
This patch is in support of adding a variant of Static{Auto,Ref}Ptr for use as
static locals, taking advantage of C++11 "magic statics" such that we can lazily
initialize those variables in a thread-safe way.
In support of those classes, this patch adds two new attributes:
* `moz_static_local_class` to ensure that any instantiations of that class only
occur as static local variables;
* `moz_trivial_dtor` to ensure that these classes do not implicitly call `atexit`
and add a whole bunch of shutdown crap.
`moz_static_local_class` works similarly to `moz_global_class`, except that its
object must only instantiate as static locals.
`TrivialDtorChecker` is based on `TrivialCtorDtorChecker`, with the ctor-specific
bits removed.
Differential Revision: https://phabricator.services.mozilla.com/D39717
--HG--
rename : build/clang-plugin/TrivialCtorDtorChecker.cpp => build/clang-plugin/TrivialDtorChecker.cpp
rename : build/clang-plugin/TrivialCtorDtorChecker.h => build/clang-plugin/TrivialDtorChecker.h
extra : moz-landing-system : lando
LLVM r356397 added some new warnings to the compiler that in this file complain about `size argument is too large; destination buffer has size 100, but size argument is 101`.
The easiest path is to just remove those lines; doing so doesn't really weaken the test coverage of the SprintfLiteral checker.
Differential Revision: https://phabricator.services.mozilla.com/D36515
--HG--
extra : moz-landing-system : lando
LLVM r356397 added some new warnings to the compiler that in this file complain about `size argument is too large; destination buffer has size 100, but size argument is 101`.
The easiest path is to just remove those lines; doing so doesn't really weaken the test coverage of the SprintfLiteral checker.
Differential Revision: https://phabricator.services.mozilla.com/D36515
--HG--
extra : moz-landing-system : lando
https://reviews.llvm.org/rL360311 reworked the handling of `explicit` in `CXXConversionDecl`, and the `isExplicitSpecified()` method no longer exists. We can instead use `isExplicit()` which conveniently works on both old and new clangs. (Before 360311, `isExplicit()` just forwarded to `isExplicitSpecified()`.)
Differential Revision: https://phabricator.services.mozilla.com/D33958
--HG--
extra : moz-landing-system : lando
Inheriting constructors are implicitly introduced via a using-declaration.
Since the C++ grammar doesn't allow attributes on using-declarations, it
is currently impossible to add MOZ_IMPLICIT to implicit inheriting
constructors.
This commit changes the AST matcher such that it ignores inheriting
constructors altogether. If they are inheriting from an implicit inherited
constructor, that constructor's check should be enough to ensure that no
constructors are unintentionally implicit.
Differential Revision: https://phabricator.services.mozilla.com/D33281
--HG--
extra : moz-landing-system : lando
Some of our builds use --enable-warnings-as-errors and some don't, and I can't
figure out a way to write an expectation comment for that.
Differential Revision: https://phabricator.services.mozilla.com/D24469
--HG--
extra : moz-landing-system : lando
Since these are compile-time constants, they can't exactly go away on us due to
running script, right?
Differential Revision: https://phabricator.services.mozilla.com/D24195
--HG--
extra : moz-landing-system : lando
We need to typecheck the trivials too, not just the final thing after trivials
are stripped, because casts are trivials.
Differential Revision: https://phabricator.services.mozilla.com/D24186
--HG--
extra : moz-landing-system : lando
The old code for member method calls did the following:
1) Find the member method calls.
2) Look at their "this" expression.
3) If the "this" is an operator call, check for any of the arguments of the
operator call being invalid.
4) Otherwise (if not an operator call) check for the "this" value being
invalid.
This wasn't right, because the "is invalid" check checks the type and only
considers refcounted things. So if the code looked something like
"foo[i]->call_method()", we would look at the types of "foo" and "i" and
determine that none of those are refcounted types so there is nothing invalid
here (since "foo" is some sort of array type and "i" is an integer). The new
setup just checks whether the "this" value is invalid, which does the type
check on the "this" value itself; in the "foo[i]->call_method()" case on
"foo[i]". We then adjust the exclusions in InvalidArg to consider operator->
on known-live things valid, to allow the thing that we were really trying to
accomplish with the "check for an operator call" bits:
"stackRefPtr->some_method()".
The test coverage being added for the made-up TArray type is meant to catch
things like the geolocation issue that was being hidden by the buggy behavior.
I'm not using nsTArray itself because some header included by nsTArray.h
tries to define operator new/delete bits inline and that triggers warnings that
then cause a clang-plugin test failure, because they're unexpected.
Differential Revision: https://phabricator.services.mozilla.com/D24117
--HG--
extra : moz-landing-system : lando
"this" is guaranteed to stay alive as long as other MOZ_CAN_RUN_SCRIPT
conditions hold, and its const members can't change value and drop
their refs.
Differential Revision: https://phabricator.services.mozilla.com/D23997
--HG--
extra : moz-landing-system : lando
Since these are compile-time constants, they can't exactly go away on us due to
running script, right?
Differential Revision: https://phabricator.services.mozilla.com/D24195
--HG--
extra : moz-landing-system : lando
We need to typecheck the trivials too, not just the final thing after trivials
are stripped, because casts are trivials.
Differential Revision: https://phabricator.services.mozilla.com/D24186
--HG--
extra : moz-landing-system : lando
The old code for member method calls did the following:
1) Find the member method calls.
2) Look at their "this" expression.
3) If the "this" is an operator call, check for any of the arguments of the
operator call being invalid.
4) Otherwise (if not an operator call) check for the "this" value being
invalid.
This wasn't right, because the "is invalid" check checks the type and only
considers refcounted things. So if the code looked something like
"foo[i]->call_method()", we would look at the types of "foo" and "i" and
determine that none of those are refcounted types so there is nothing invalid
here (since "foo" is some sort of array type and "i" is an integer). The new
setup just checks whether the "this" value is invalid, which does the type
check on the "this" value itself; in the "foo[i]->call_method()" case on
"foo[i]". We then adjust the exclusions in InvalidArg to consider operator->
on known-live things valid, to allow the thing that we were really trying to
accomplish with the "check for an operator call" bits:
"stackRefPtr->some_method()".
The test coverage being added for the made-up TArray type is meant to catch
things like the geolocation issue that was being hidden by the buggy behavior.
I'm not using nsTArray itself because some header included by nsTArray.h
tries to define operator new/delete bits inline and that triggers warnings that
then cause a clang-plugin test failure, because they're unexpected.
Differential Revision: https://phabricator.services.mozilla.com/D24117
--HG--
extra : moz-landing-system : lando
"this" is guaranteed to stay alive as long as other MOZ_CAN_RUN_SCRIPT
conditions hold, and its const members can't change value and drop
their refs.
Differential Revision: https://phabricator.services.mozilla.com/D23997
--HG--
extra : moz-landing-system : lando
This allows calling a C++ MOZ_CAN_RUN_SCRIPT method that takes a callback argument.
The changes to TestCanRunScript.cpp are there to catch an incorrect change I was
going to make to the analysis to make this work, until I figured out that
RootedCallback should be MOZ_IS_SMARTPTR_TO_REFCOUNTED.
Differential Revision: https://phabricator.services.mozilla.com/D23519
--HG--
extra : moz-landing-system : lando
In at least some libstdc++ versions, std::pair (which we want to be
memmovable) inherits from __pair_base. We have a similar setup
already for __atomic_base.
Differential Revision: https://phabricator.services.mozilla.com/D23698
--HG--
extra : moz-landing-system : lando
This way if a caller calls a method that has a MOZ_CAN_RUN_SCRIPT override, it
can detect that it's possibly calling a MOZ_CAN_RUN_SCRIPT thing without having
to know about the override.
Differential Revision: https://phabricator.services.mozilla.com/D22839
--HG--
extra : moz-landing-system : lando
Clang 8 seems to generate destructor decls from different source
locations which breaks an assertion in the code. This patch updates the
code to remove the assertion and more robustly handle the new
declarations.
Differential Revision: https://phabricator.services.mozilla.com/D17902
--HG--
extra : moz-landing-system : lando
In the first part of bug 1501903, paths were canonicalized in
inThirdPartyPath. For some reason, this seems to add a non-negligible
overhead on Windows. OTOH, most include paths handed by the build system
are absolute, and most paths that contains .. are for system headers,
which don't matter for the inThirdPartyPath test.
Considering the inThirdPartyPath has been using non-canonicalized paths
for the longest, we'll assume it's fine to come back to that mode.
Differential Revision: https://phabricator.services.mozilla.com/D11725
--HG--
extra : moz-landing-system : lando
While looking at this code I found a couple of places where errors could
get ignored or silently discarded and result in corrupt data. This
checks for the errors and fails harder.
Depends on D10353
Differential Revision: https://phabricator.services.mozilla.com/D10354
--HG--
extra : moz-landing-system : lando
This is necessary because:
(a) the JSONFormatter emits a \n newline for each analysis line
(b) we truncate the file to the expected length after writing it
(c) on Windows writing the file in text mode replaces \n with \r\n
and invalidates our computed "expected length"
Differential Revision: https://phabricator.services.mozilla.com/D10353
--HG--
extra : moz-landing-system : lando
The function is called a lot for the same paths and is rather costly, so
cache the results for each path.
Depends on D9758
Differential Revision: https://phabricator.services.mozilla.com/D9759
--HG--
extra : moz-landing-system : lando
SourceLocation that are passed to inThirdPartyPath might be macro
expansion locations, for which SourceManager.getFilename returns the
path of the directory containing the source, rather than of the
expansion location.
Furthermore, the paths getFileName returns are not canonical, and can
contain e.g. `..`.
Differential Revision: https://phabricator.services.mozilla.com/D9758
--HG--
extra : moz-landing-system : lando
ParmVarDecl being a subclass of VarDecl, using two matchers then caused
ScopeChecker::check to be called twice for ParmVarDecl nodes, once for
each match. But the code in ScopeCheck::check is written with the
assumption that it's called only once for such nodes.
Somehow, this didn't cause problems with clang up to version 6, but
makes the plugin spuriously warn about already_AddRefed not being used
as temporaries when used as argument in function declarations, with
clang 7.
Differential Revision: https://phabricator.services.mozilla.com/D6360
--HG--
extra : moz-landing-system : lando
- We forcefully remove annotations from the AST so that they don't end
up impacting codegen.
- We change the API such that we use identifiers instead of strings,
reducing the chances of typo errors.
Differential Revision: https://phabricator.services.mozilla.com/D5493
--HG--
extra : moz-landing-system : lando
As per bug 1487622, annotations have side effects on codegen, one of
which changes a leak signature that the wpt harness doesn't match
against its whitelist anymore.
Interestingly, while looking at the issue, I found multiple moz_*
annotations that weren't actually looked for by the clang plugin,
making them useless. Even more interestingly, removing them changes
codegen in such a way that the leak signature mentioned above is not
altered anymore.
Differential Revision: https://phabricator.services.mozilla.com/D5000
Iain Ireland
Andrea Marchesini
Razvan Maries
Andi-Bogdan Postelnicu
Coroiu Cristina
Mihai Alexandru Michis
Cosmin Sabou
Simon Giesecke
Andreea Pavel
Boris Zbarsky
june wilde
Kershaw Chang
Csoregi Natalia
Sebastian Streich
Rob Lemley
Mike Hommey
kriswright
Dorel Luca
Mike Shal
Brindusan Cristian
Masatoshi Kimura
Emilio Cobos Álvarez
Aaron Klotz
shravanrn@gmail.com
Ricky Stewart
Daniel Varga
David Major
Nathan Froyd
Sylvestre Ledru
Andrew Sutherland
Tom Ritter
Bogdan Tara
Yaron Tausky
Noemi Erli
Ciure Andrei
Kartikaya Gupta
Myk Melez