Bug 1814295 - Handle references in the analysis, in particular AutoCheckCannotGC&&. r=jonco

The hazard analysis needs to be able to pass a parameter of type AutoCheckCannotGC&& that can be considered to be "consumed" either explicitly by calling reset() on it, or given over to a calle with std::move().

Differential Revision: https://phabricator.services.mozilla.com/D170142
This commit is contained in:
Steve Fink 2023-03-15 17:43:35 +00:00
parent b469c5a495
commit 231aa8be0a
13 changed files with 320 additions and 72 deletions

View File

@ -1131,7 +1131,7 @@ class JS_PUBLIC_API AutoCheckCannotGC : public AutoRequireNoGC {
explicit AutoCheckCannotGC(JSContext* cx = nullptr) {}
AutoCheckCannotGC(const AutoCheckCannotGC& other) : AutoCheckCannotGC() {}
#endif
} JS_HAZ_GC_INVALIDATED;
} JS_HAZ_GC_INVALIDATED JS_HAZ_GC_REF;
extern JS_PUBLIC_API void SetLowMemoryState(JSContext* cx, bool newState);

View File

@ -21,6 +21,11 @@
// MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS.
# define JS_HAZ_GC_POINTER __attribute__((annotate("GC Pointer")))
// Same as JS_HAZ_GC_POINTER, except additionally treat pointers to these
// as GC pointers themselves in order to check references to them, since
// the analysis cannot distinguish between pointers and references.
# define JS_HAZ_GC_REF __attribute__((annotate("GC Pointer or Reference")))
// Mark a type as a rooted pointer, suitable for use on the stack (eg all
// Rooted<T> instantiations should have this.) "Inherited" by templatized types
// with MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS.
@ -78,6 +83,7 @@
# define JS_EXPECT_HAZARDS
# define JS_HAZ_GC_THING
# define JS_HAZ_GC_POINTER
# define JS_HAZ_GC_REF
# define JS_HAZ_ROOTED
# define JS_HAZ_GC_INVALIDATED
# define JS_HAZ_ROOTED_BASE

View File

@ -10,6 +10,11 @@
var TRACING = false;
// When edge.Kind == "Pointer", these are the meanings of the edge.Reference field.
var PTR_POINTER = 0;
var PTR_REFERENCE = 1;
var PTR_RVALUE_REF = 2;
// Find all points (positions within the code) of the body given by the list of
// bodies and the blockId to match (which will specify an outer function or a
// loop within it), recursing into loops if needed.
@ -335,6 +340,11 @@ function isImmobileValue(exp) {
return false;
}
// Returns whether decl is a body.DefineVariable[] entry for a non-temporary reference.
function isReferenceDecl(decl) {
return decl.Type.Kind == "Pointer" && decl.Type.Reference != PTR_POINTER && decl.Variable.Kind != "Temp";
}
function expressionIsVariableAddress(exp, variable)
{
while (exp.Kind == "Fld")
@ -452,22 +462,29 @@ function isReturningImmobileValue(edge, variable)
// start looking at the final point in the function, not one point back from
// that, since that would skip over the GCing call.
//
// Note that this returns true only if the variable's incoming value is used.
// So this would return false for 'obj':
// Certain references may be annotated to be live to the end of the function
// as well (eg AutoCheckCannotGC&& parameters).
//
// Note that this returns a nonzero value only if the variable's incoming value is used.
// So this would return 0 for 'obj':
//
// obj = someFunction();
//
// but these would return true:
// but these would return a positive value:
//
// obj = someFunction(obj);
// obj->foo = someFunction();
//
function edgeUsesVariable(edge, variable, body)
function edgeUsesVariable(edge, variable, body, liveToEnd=false)
{
if (ignoreEdgeUse(edge, variable, body))
return 0;
if (variable.Kind == "Return" && body.Index[1] == edge.Index[1] && body.BlockId.Kind == "Function") {
if (variable.Kind == "Return") {
liveToEnd = true;
}
if (liveToEnd && body.Index[1] == edge.Index[1] && body.BlockId.Kind == "Function") {
// The last point in the function body is treated as using the return
// value. This is the only time the destination point is returned
// rather than the source point.
@ -543,19 +560,39 @@ function edgeUsesVariable(edge, variable, body)
}
}
// If `decl` is the body.DefineVariable[] declaration of a reference type, then
// return the expression without the outer dereference. Otherwise, return the
// original expression.
function maybeDereference(exp, decl) {
if (exp.Kind == "Drf" && exp.Exp[0].Kind == "Var") {
if (isReferenceDecl(decl)) {
return exp.Exp[0];
}
}
return exp;
}
function expressionIsVariable(exp, variable)
{
return exp.Kind == "Var" && sameVariable(exp.Variable, variable);
}
function expressionIsMethodOnVariable(exp, variable)
// Similar to the above, except treat uses of a reference as if they were uses
// of the dereferenced contents. This requires knowing the type of the
// variable, and so takes its declaration rather than the variable itself.
function expressionIsDeclaredVariable(exp, decl)
{
exp = maybeDereference(exp, decl);
return expressionIsVariable(exp, decl.Variable);
}
function expressionIsMethodOnVariableDecl(exp, decl)
{
// This might be calling a method on a base class, in which case exp will
// be an unnamed field of the variable instead of the variable itself.
while (exp.Kind == "Fld" && exp.Field.Name[0].startsWith("field:"))
exp = exp.Exp[0];
return exp.Kind == "Var" && sameVariable(exp.Variable, variable);
return expressionIsDeclaredVariable(exp, decl);
}
// Return whether the edge starts the live range of a variable's value, by setting
@ -626,6 +663,21 @@ function edgeStartsValueLiveRange(edge, variable)
return false;
}
// Match an optional <namespace>:: followed by the class name,
// and then an optional template parameter marker.
//
// Example: mozilla::dom::UniquePtr<...
//
function parseTypeName(typeName) {
const m = typeName.match(/^(((?:\w|::)+::)?(\w+))\b(\<)?/);
if (!m) {
return undefined;
}
const [, type, raw_namespace, classname, is_specialized] = m;
const namespace = raw_namespace === null ? "" : raw_namespace;
return { type, namespace, classname, is_specialized }
}
// Return whether an edge "clears out" a variable's value. A simple example
// would be
//
@ -673,12 +725,14 @@ function edgeEndsValueLiveRange(edge, variable, body)
return true;
}
const decl = lookupVariable(body, variable);
if (edge.Type.Kind == 'Function' &&
edge.Exp[0].Kind == 'Var' &&
edge.Exp[0].Variable.Kind == 'Func' &&
edge.Exp[0].Variable.Name[1] == 'move' &&
edge.Exp[0].Variable.Name[0].includes('std::move(') &&
expressionIsVariable(edge.PEdgeCallArguments.Exp[0], variable) &&
expressionIsDeclaredVariable(edge.PEdgeCallArguments.Exp[0], decl) &&
edge.Exp[1].Kind == 'Var' &&
edge.Exp[1].Variable.Kind == 'Temp')
{
@ -721,16 +775,25 @@ function edgeEndsValueLiveRange(edge, variable, body)
if (edge.Type.Kind == 'Function' &&
edge.Type.TypeFunctionCSU &&
edge.PEdgeCallInstance &&
expressionIsMethodOnVariable(edge.PEdgeCallInstance.Exp, variable))
expressionIsMethodOnVariableDecl(edge.PEdgeCallInstance.Exp, decl))
{
const typeName = edge.Type.TypeFunctionCSU.Type.Name;
const m = typeName.match(/^(((\w|::)+?)(\w+))</);
if (m) {
const [, type, namespace,, classname] = m;
// Synthesize a zero-arg constructor name like
// mozilla::dom::UniquePtr<T>::UniquePtr(). Note that the `<T>` is
// literal -- the pretty name from sixgill will render the actual
// constructor name as something like
//
// UniquePtr<T>::UniquePtr() [where T = int]
//
const parsed = parseTypeName(typeName);
if (parsed) {
const { type, namespace, classname, is_specialized } = parsed;
// special-case: the initial constructor that doesn't provide a value.
// Useful for things like Maybe<T>.
const ctorName = `${namespace}${classname}<T>::${classname}()`;
const template = is_specialized ? '<T>' : '';
const ctorName = `${namespace}${classname}${template}::${classname}()`;
if (callee.Kind == 'Var' &&
typesWithSafeConstructors.has(type) &&
callee.Variable.Name[0].includes(ctorName))
@ -769,7 +832,17 @@ function edgeEndsValueLiveRange(edge, variable, body)
return false;
}
function edgeMovesVariable(edge, variable)
// Look up a variable in the list of declarations for this body.
function lookupVariable(body, variable) {
for (const decl of (body.DefineVariable || [])) {
if (sameVariable(decl.Variable, variable)) {
return decl;
}
}
return undefined;
}
function edgeMovesVariable(edge, variable, body)
{
if (edge.Kind != 'Call')
return false;
@ -778,10 +851,25 @@ function edgeMovesVariable(edge, variable)
callee.Variable.Kind == 'Func')
{
const { Variable: { Name: [ fullname, shortname ] } } = callee;
const [ mangled, unmangled ] = splitFunction(fullname);
// Match a UniquePtr move constructor.
if (unmangled.match(/::UniquePtr<[^>]*>::UniquePtr\((\w+::)*UniquePtr<[^>]*>&&/))
return true;
// Match an rvalue parameter.
if (!edge || !edge.PEdgeCallArguments || !edge.PEdgeCallArguments.Exp) {
return false;
}
for (const arg of edge.PEdgeCallArguments.Exp) {
if (arg.Kind != 'Drf') continue;
const val = arg.Exp[0];
if (val.Kind == 'Var' && sameVariable(val.Variable, variable)) {
// This argument is the variable we're looking for. Return true
// if it is passed as an rvalue reference.
const type = lookupVariable(body, variable).Type;
if (type.Kind == "Pointer" && type.Reference == PTR_RVALUE_REF) {
return true;
}
}
}
}
return false;
@ -802,7 +890,7 @@ function basicBlockEatsVariable(variable, body, startpoint)
}
const edge = edges[0];
if (edgeMovesVariable(edge, variable)) {
if (edgeMovesVariable(edge, variable, body)) {
return true;
}

View File

@ -81,42 +81,53 @@ var typeInfo = loadTypeInfo(options.typeInfo);
var gcEdges = JSON.parse(os.file.readFile(options.gcEdges));
var match;
var gcThings = {};
var gcPointers = {};
var gcThings = new Set();
var gcPointers = new Set();
var gcRefs = new Set(typeInfo.GCRefs);
text = snarf(options.gcTypes).split("\n");
for (var line of text) {
if (match = /^GCThing: (.*)/.exec(line))
gcThings[match[1]] = true;
gcThings.add(match[1]);
if (match = /^GCPointer: (.*)/.exec(line))
gcPointers[match[1]] = true;
gcPointers.add(match[1]);
}
text = null;
function isGCRef(type)
{
if (type.Kind == "CSU")
return gcRefs.has(type.Name);
return false;
}
function isGCType(type)
{
if (type.Kind == "CSU")
return type.Name in gcThings;
return gcThings.has(type.Name);
else if (type.Kind == "Array")
return isGCType(type.Type);
return false;
}
function isUnrootedType(type)
function isUnrootedPointerDeclType(decl)
{
if (type.Kind == "Pointer")
// Treat non-temporary T& references as if they were the underlying type T.
// For now, restrict this to only the types specifically annotated with JS_HAZ_GC_REF
// to avoid lots of false positives with other types.
let type = isReferenceDecl(decl) && isGCRef(decl.Type.Type) ? decl.Type.Type : decl.Type;
while (type.Kind == "Array") {
type = type.Type;
}
if (type.Kind == "Pointer") {
return isGCType(type.Type);
else if (type.Kind == "Array") {
if (!type.Type) {
printErr("Received Array Kind with no Type");
printErr(JSON.stringify(type));
printErr(getBacktrace({args: true, locals: true}));
}
return isUnrootedType(type.Type);
} else if (type.Kind == "CSU")
return type.Name in gcPointers;
else
} else if (type.Kind == "CSU") {
return gcPointers.has(type.Name);
} else {
return false;
}
}
function edgeCanGC(edge)
@ -500,7 +511,7 @@ function findGCBeforeValueUse(start_body, start_point, suppressed_bits, variable
return BFS_upwards(start_body, start_point, functionBodies, visitor, new Path()) || bestPathWithAnyUse;
}
function variableLiveAcrossGC(suppressed, variable)
function variableLiveAcrossGC(suppressed, variable, liveToEnd=false)
{
// A variable is live across a GC if (1) it is used by an edge (as in, it
// was at least initialized), and (2) it is used after a GC in a successor
@ -541,7 +552,7 @@ function variableLiveAcrossGC(suppressed, variable)
if (edgeEndsValueLiveRange(edge, variable, body))
continue;
var usePoint = edgeUsesVariable(edge, variable, body);
var usePoint = edgeUsesVariable(edge, variable, body, liveToEnd);
if (usePoint) {
var call = findGCBeforeValueUse(body, usePoint, suppressed, variable);
if (!call)
@ -690,8 +701,10 @@ function printEntryTrace(functionName, entry)
}
}
function isRootedType(type)
function isRootedDeclType(decl)
{
// Treat non-temporary T& references as if they were the underlying type T.
const type = isReferenceDecl(decl) ? decl.Type.Type : decl.Type;
return type.Kind == "CSU" && ((type.Name in typeInfo.RootedPointers) ||
(type.Name in typeInfo.RootedGCThings));
}
@ -775,20 +788,31 @@ function processBodies(functionName, wholeBodyAttrs)
}
}
for (const variable of functionBodies[0].DefineVariable) {
for (let decl of functionBodies[0].DefineVariable) {
var name;
if (variable.Variable.Kind == "This")
if (decl.Variable.Kind == "This")
name = "this";
else if (variable.Variable.Kind == "Return")
else if (decl.Variable.Kind == "Return")
name = "<returnvalue>";
else
name = variable.Variable.Name[0];
name = decl.Variable.Name[0];
if (ignoreVars.has(name))
continue;
if (isRootedType(variable.Type)) {
if (!variableLiveAcrossGC(suppressed, variable.Variable)) {
let liveToEnd = false;
if (decl.Variable.Kind == "Arg" && isReferenceDecl(decl) && decl.Type.Reference == 2) {
// References won't run destructors, so they would normally not be
// considered live at the end of the function. In order to handle
// the pattern of moving a GC-unsafe value into a function (eg an
// AutoCheckCannotGC&&), assume all argument rvalue references live to the
// end of the function unless their liveness is terminated by
// calling reset() or moving them into another function call.
liveToEnd = true;
}
if (isRootedDeclType(decl)) {
if (!variableLiveAcrossGC(suppressed, decl.Variable)) {
// The earliest use of the variable should be its constructor.
var lineText;
for (var body of functionBodies) {
@ -801,28 +825,28 @@ function processBodies(functionName, wholeBodyAttrs)
print("\nFunction '" + functionName + "'" +
" has unnecessary root '" + name + "' at " + lineText);
}
} else if (isUnrootedType(variable.Type)) {
var result = variableLiveAcrossGC(suppressed, variable.Variable);
} else if (isUnrootedPointerDeclType(decl)) {
var result = variableLiveAcrossGC(suppressed, decl.Variable, liveToEnd);
if (result) {
assert(result.gcInfo);
var lineText = findLocation(result.gcInfo.body, result.gcInfo.ppoint);
if (annotations.has('Expect Hazards')) {
print("\nThis is expected, but '" + functionName + "'" +
" has unrooted '" + name + "'" +
" of type '" + typeDesc(variable.Type) + "'" +
" of type '" + typeDesc(decl.Type) + "'" +
" live across GC call " + result.gcInfo.name +
" at " + lineText);
missingExpectedHazard = false;
} else {
print("\nFunction '" + functionName + "'" +
" has unrooted '" + name + "'" +
" of type '" + typeDesc(variable.Type) + "'" +
" of type '" + typeDesc(decl.Type) + "'" +
" live across GC call " + result.gcInfo.name +
" at " + lineText);
}
printEntryTrace(functionName, result);
}
result = unsafeVariableAddressTaken(suppressed, variable.Variable);
result = unsafeVariableAddressTaken(suppressed, decl.Variable);
if (result) {
var lineText = findLocation(result.body, result.ppoint);
print("\nFunction '" + functionName + "'" +

View File

@ -34,6 +34,7 @@ var resetterMethods = {
'mozilla::dom::TypedArray_base': new Set(["Reset"]),
'RefPtr': new Set(["forget"]),
'nsCOMPtr': new Set(["forget"]),
'JS::AutoAssertNoGC': new Set(["reset"]),
};
function isRefcountedDtor(name) {

View File

@ -18,6 +18,7 @@ var typeInfo = {
'GCPointers': [],
'GCThings': [],
'GCInvalidated': [],
'GCRefs': [],
'NonGCTypes': {}, // unused
'NonGCPointers': {},
'RootedGCThings': {},
@ -48,8 +49,7 @@ var rootedPointers = {};
// Accumulate the base GC types before propagating info through the type graph,
// so that we can include edges from types processed later
// (eg MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS).
var pendingGCTypes = [];
var pendingGCPointers = [];
var pendingGCTypes = []; // array of [name, reason, ptrdness]
function processCSU(csu, body)
{
@ -61,6 +61,8 @@ function processCSU(csu, body)
typeInfo.GCPointers.push(csu);
else if (tag == 'Invalidated by GC')
typeInfo.GCInvalidated.push(csu);
else if (tag == 'GC Pointer or Reference')
typeInfo.GCRefs.push(csu);
else if (tag == 'GC Thing')
typeInfo.GCThings.push(csu);
else if (tag == 'Suppressed GC Pointer')
@ -411,15 +413,12 @@ function addGCType(typeName, child, why, depth, fieldPtrLevel)
function addGCPointer(typeName)
{
pendingGCPointers.push([typeName, '<pointer-annotation>', '(annotation)', 1, 0]);
pendingGCTypes.push([typeName, '<pointer-annotation>', '(annotation)', 1, 0]);
}
for (const pending of pendingGCTypes) {
markGCType(...pending);
}
for (const pending of pendingGCPointers) {
markGCType(...pending);
}
// Call a function for a type and every type that contains the type in a field
// or as a base class (which internally is pretty much the same thing --

View File

@ -62,13 +62,13 @@ function str_Type(type) {
try {
const {Kind, Type, Name, TypeFunctionArguments} = type;
if (Kind == 'Pointer')
return str_Type(Type) + "*";
return str_Type(Type) + ["*", "&", "&&"][type.Reference];
else if (Kind == 'CSU')
return Name;
else if (Kind == 'Array')
return str_Type(Type) + "[]";
return Kind;
else
return Kind;
} catch(e) {
badFormat("type", type);
}

View File

@ -45,17 +45,19 @@ parser.add_argument(
action="store_true",
help="Display verbose output, including commands executed",
)
ALL_TESTS = [
"sixgill-tree",
"suppression",
"hazards",
"exceptions",
"virtual",
"graph",
"types",
]
parser.add_argument(
"tests",
nargs="*",
default=[
"sixgill-tree",
"suppression",
"hazards",
"exceptions",
"virtual",
"graph",
],
default=ALL_TESTS,
help="tests to run",
)
@ -104,9 +106,19 @@ make_dir(outroot)
os.environ["HAZARD_RUN_INTERNAL_TESTS"] = "1"
exclude = []
tests = []
for t in cfg.tests:
if t.startswith("!"):
exclude.append(t[1:])
else:
tests.append(t)
if len(tests) == 0:
tests = filter(lambda t: t not in exclude, ALL_TESTS)
failed = set()
passed = set()
for name in cfg.tests:
for name in tests:
name = os.path.basename(name)
indir = os.path.join(testdir, name)
outdir = os.path.join(outroot, name)

View File

@ -85,7 +85,7 @@ class Test(object):
)
return json.loads(output)
def run_analysis_script(self, startPhase, upto=None):
def run_analysis_script(self, startPhase="gcTypes", upto=None):
open("defaults.py", "w").write(
"""\
analysis_scriptdir = '{scriptdir}'

View File

@ -0,0 +1,99 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include <utility>
#define ANNOTATE(property) __attribute__((annotate(property)))
struct Cell {
int f;
} ANNOTATE("GC Thing");
namespace World {
namespace NS {
struct Unsafe {
int g;
~Unsafe() { asm(""); }
} ANNOTATE("Invalidated by GC") ANNOTATE("GC Pointer or Reference");
} // namespace NS
} // namespace World
extern void GC() ANNOTATE("GC Call");
extern void invisible();
void GC() {
// If the implementation is too trivial, the function body won't be emitted at
// all.
asm("");
invisible();
}
extern void usecell(Cell*);
Cell* cell() {
static Cell c;
return &c;
}
template <typename T, typename U>
struct SimpleTemplate {
int member;
};
template <typename T, typename U>
class ANNOTATE("moz_inherit_type_annotations_from_template_args") Container {
public:
template <typename V, typename W>
void foo(V& v, W& w) {
class InnerClass {};
InnerClass xxx;
return;
}
};
Cell* f() {
Container<int, double> c1;
Container<SimpleTemplate<int, int>, SimpleTemplate<double, double>> c2;
Container<Container<int, double>, Container<void, void>> c3;
Container<Container<SimpleTemplate<int, int>, void>,
Container<void, SimpleTemplate<char, char>>>
c4;
return nullptr;
};
void rvalue_ref(World::NS::Unsafe&& arg1) { GC(); }
void ref(const World::NS::Unsafe& arg2) {
GC();
static int use = arg2.g;
}
// A function that consumes a parameter, but only if passed by rvalue reference.
extern void eat(World::NS::Unsafe&&);
extern void eat(World::NS::Unsafe&);
void rvalue_ref_ok() {
World::NS::Unsafe unsafe1;
eat(std::move(unsafe1));
GC();
}
void rvalue_ref_not_ok() {
World::NS::Unsafe unsafe2;
eat(unsafe2);
GC();
}
void rvalue_ref_arg_ok(World::NS::Unsafe&& unsafe3) {
eat(std::move(unsafe3));
GC();
}
void rvalue_ref_arg_not_ok(World::NS::Unsafe&& unsafe4) {
eat(unsafe4);
GC();
}

View File

@ -0,0 +1,14 @@
# flake8: noqa: F821
from collections import defaultdict
test.compile("source.cpp")
test.run_analysis_script()
hazards = test.load_hazards()
hazmap = {haz.variable: haz for haz in hazards}
assert "arg1" in hazmap
assert "arg2" in hazmap
assert "unsafe1" not in hazmap
assert "unsafe2" in hazmap
assert "unsafe3" not in hazmap
assert "unsafe4" in hazmap

View File

@ -2974,6 +2974,11 @@ bool XPCJSRuntime::DescribeCustomObjects(JSObject* obj, const JSClass* clasp,
}
XPCWrappedNativeProto* p = XPCWrappedNativeProto::Get(obj);
// Nothing here can GC. The analysis would otherwise think that ~nsCOMPtr
// could GC, but that's only possible if nsIXPCScriptable::GetJSClass()
// somehow released a reference to the nsIXPCScriptable, which isn't going to
// happen.
JS::AutoSuppressGCAnalysis nogc;
nsCOMPtr<nsIXPCScriptable> scr = p->GetScriptable();
if (!scr) {
return false;

View File

@ -11,7 +11,7 @@ root_dir=$MOZ_FETCHES_DIR
build_dir=$GECKO_PATH/build
data_dir=$GECKO_PATH/build/unix/build-gcc
sixgill_rev=9b7f4a7bf67a
sixgill_rev=a642a811d6ee
sixgill_repo=https://hg.mozilla.org/users/sfink_mozilla.com/sixgill
. $data_dir/build-gcc.sh