Bug 1816165 - [hazards] Replace shared_ptr<T>::~shared_ptr() calls with T::~T() r=jonco

Differential Revision: https://phabricator.services.mozilla.com/D171513
This commit is contained in:
Steve Fink 2023-04-01 19:41:33 +00:00
parent ee7bccbcdc
commit 3a672e3b71
7 changed files with 83 additions and 8 deletions

View file

@ -908,22 +908,39 @@ function basicBlockEatsVariable(variable, body, startpoint)
return false; return false;
} }
var PROP_REFCNT = 1 << 0; var PROP_REFCNT = 1 << 0;
var PROP_SHARED_PTR_DTOR = 1 << 1;
function getCalleeProperties(calleeName) { function getCalleeProperties(calleeName) {
let props = 0; let props = 0;
if (isRefcountedDtor(calleeName)) { if (isRefcountedDtor(calleeName)) {
props = props | PROP_REFCNT; props |= PROP_REFCNT;
}
if (calleeName.includes("~shared_ptr()")) {
props |= PROP_SHARED_PTR_DTOR;
} }
return props; return props;
} }
// Basic C++ ABI mangling: prefix an identifier with its length, in decimal.
function mangle(name) {
return name.length + name;
}
function synthesizeDestructorName(className) {
const parts = className.split("::");
const mangled_dtor = "_ZN" + parts.map(p => mangle(p)).join("") + "D2Ev";
const pretty_dtor = `void ${className}::~${parts.at(-1)}()`;
return mangled_dtor + "$" + pretty_dtor;
}
function getCallEdgeProperties(body, edge, calleeName, functionBodies) { function getCallEdgeProperties(body, edge, calleeName, functionBodies) {
let attrs = 0; let attrs = 0;
let extraCalls = [];
if (edge.Kind !== "Call") { if (edge.Kind !== "Call") {
return { attrs }; return { attrs, extraCalls };
} }
const props = getCalleeProperties(calleeName); const props = getCalleeProperties(calleeName);
@ -938,8 +955,19 @@ function getCallEdgeProperties(body, edge, calleeName, functionBodies) {
} }
} }
if (props & PROP_SHARED_PTR_DTOR) {
const m = calleeName.match(/shared_ptr<(.*?)>::~shared_ptr()/);
assert(m);
const className = m[1];
attrs |= ATTR_REPLACED;
extraCalls.push({
attrs: ATTR_SYNTHETIC,
name: synthesizeDestructorName(className),
});
}
if ((props & PROP_REFCNT) == 0) { if ((props & PROP_REFCNT) == 0) {
return { attrs }; return { attrs, extraCalls };
} }
let callee = edge.Exp[0]; let callee = edge.Exp[0];
@ -950,7 +978,7 @@ function getCallEdgeProperties(body, edge, calleeName, functionBodies) {
const instance = edge.PEdgeCallInstance.Exp; const instance = edge.PEdgeCallInstance.Exp;
if (instance.Kind !== "Var") { if (instance.Kind !== "Var") {
// TODO: handle field destructors // TODO: handle field destructors
return { attrs }; return { attrs, extraCalls };
} }
// Test whether the dtor call is dominated by operations on the variable // Test whether the dtor call is dominated by operations on the variable
@ -1006,7 +1034,7 @@ function getCallEdgeProperties(body, edge, calleeName, functionBodies) {
if (edgeIsNonReleasingDtor) { if (edgeIsNonReleasingDtor) {
attrs |= ATTR_GC_SUPPRESSED | ATTR_NONRELEASING; attrs |= ATTR_GC_SUPPRESSED | ATTR_NONRELEASING;
} }
return { attrs }; return { attrs, extraCalls };
} }
// gcc uses something like "__dt_del " for virtual destructors that it // gcc uses something like "__dt_del " for virtual destructors that it

View file

@ -167,6 +167,9 @@ function getCallees(body, edge, scopeAttrs, functionBodies) {
calls.push({ callee, attrs: scopeAttrs }); calls.push({ callee, attrs: scopeAttrs });
} else { } else {
const edgeInfo = getCallEdgeProperties(body, edge, callee.name, functionBodies); const edgeInfo = getCallEdgeProperties(body, edge, callee.name, functionBodies);
for (const extra of (edgeInfo.extraCalls || [])) {
calls.push({ attrs: scopeAttrs | extra.attrs, callee: { name: extra.name, 'kind': "direct", } });
}
calls.push({ callee, attrs: scopeAttrs | edgeInfo.attrs}); calls.push({ callee, attrs: scopeAttrs | edgeInfo.attrs});
} }
} }

View file

@ -154,6 +154,14 @@ function processBody(functionName, body, functionBodies)
const scopeAttrs = body.attrs[edge.Index[0]] | 0; const scopeAttrs = body.attrs[edge.Index[0]] | 0;
for (const { callee, attrs } of getCallees(body, edge, scopeAttrs, functionBodies)) { for (const { callee, attrs } of getCallees(body, edge, scopeAttrs, functionBodies)) {
// Some function names will be synthesized by manually constructing
// their names. Verify that we managed to synthesize an existing function.
// This cannot be done later with either the callees or callers tables,
// because the function may be an otherwise uncalled leaf.
if (attrs & ATTR_SYNTHETIC) {
assertFunctionExists(callee.name);
}
// Individual callees may have additional attrs. The only such // Individual callees may have additional attrs. The only such
// bit currently is that nsISupports.{AddRef,Release} are assumed // bit currently is that nsISupports.{AddRef,Release} are assumed
// to never GC. // to never GC.
@ -272,6 +280,11 @@ if (options.function) {
minStream = maxStream = index; minStream = maxStream = index;
} }
function assertFunctionExists(name) {
var data = xdb.read_entry(name);
assert(data.contents != 0, `synthetic function '${name}' not found!`);
}
function process(functionName, functionBodies) function process(functionName, functionBodies)
{ {
for (var body of functionBodies) for (var body of functionBodies)
@ -283,8 +296,12 @@ function process(functionName, functionBodies)
} }
} }
for (var body of functionBodies) if (options.function) {
debugger;
}
for (var body of functionBodies) {
processBody(functionName, body, functionBodies); processBody(functionName, body, functionBodies);
}
// Not strictly necessary, but add an edge from the synthetic "(js-code)" // Not strictly necessary, but add an edge from the synthetic "(js-code)"
// to RunScript to allow better stacks than just randomly selecting a // to RunScript to allow better stacks than just randomly selecting a

View file

@ -54,6 +54,8 @@ class Test(object):
def compile(self, source, options=""): def compile(self, source, options=""):
env = os.environ env = os.environ
env["CCACHE_DISABLE"] = "1" env["CCACHE_DISABLE"] = "1"
if "-fexceptions" not in options and "-fno-exceptions" not in options:
options += " -fno-exceptions"
cmd = "{CXX} -c {source} -O3 -std=c++17 -fplugin={sixgill} -fplugin-arg-xgill-mangle=1 {options}".format( # NOQA: E501 cmd = "{CXX} -c {source} -O3 -std=c++17 -fplugin={sixgill} -fplugin-arg-xgill-mangle=1 {options}".format( # NOQA: E501
source=self.infile(source), source=self.infile(source),
CXX=self.cfg.cxx, CXX=self.cfg.cxx,

View file

@ -4,6 +4,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * 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/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include <memory>
#include <utility> #include <utility>
#define ANNOTATE(property) __attribute__((annotate(property))) #define ANNOTATE(property) __attribute__((annotate(property)))
@ -31,6 +32,14 @@ void GC() {
invisible(); invisible();
} }
struct GCOnDestruction {
~GCOnDestruction() { GC(); }
};
struct NoGCOnDestruction {
~NoGCOnDestruction() { asm(""); }
};
extern void usecell(Cell*); extern void usecell(Cell*);
Cell* cell() { Cell* cell() {
@ -97,3 +106,15 @@ void rvalue_ref_arg_not_ok(World::NS::Unsafe&& unsafe4) {
eat(unsafe4); eat(unsafe4);
GC(); GC();
} }
void shared_ptr_hazard() {
Cell* unsafe5 = f();
{ auto p = std::make_shared<GCOnDestruction>(); }
usecell(unsafe5);
}
void shared_ptr_no_hazard() {
Cell* safe6 = f();
{ auto p = std::make_shared<NoGCOnDestruction>(); }
usecell(safe6);
}

View file

@ -12,3 +12,5 @@ assert "unsafe1" not in hazmap
assert "unsafe2" in hazmap assert "unsafe2" in hazmap
assert "unsafe3" not in hazmap assert "unsafe3" not in hazmap
assert "unsafe4" in hazmap assert "unsafe4" in hazmap
assert "unsafe5" in hazmap
assert "safe6" not in hazmap

View file

@ -16,9 +16,11 @@ var ATTR_CANSCRIPT_BOUNDED = 1 << 1; // Unimplemented
var ATTR_DOM_ITERATING = 1 << 2; // Unimplemented var ATTR_DOM_ITERATING = 1 << 2; // Unimplemented
var ATTR_NONRELEASING = 1 << 3; // ~RefPtr of value whose refcount will not go to zero var ATTR_NONRELEASING = 1 << 3; // ~RefPtr of value whose refcount will not go to zero
var ATTR_REPLACED = 1 << 4; // Ignore edge, it was replaced by zero or more better edges. var ATTR_REPLACED = 1 << 4; // Ignore edge, it was replaced by zero or more better edges.
var ATTR_SYNTHETIC = 1 << 5; // Call was manufactured in some way.
var ATTR_LAST = 1 << 5;
var ATTRS_NONE = 0; var ATTRS_NONE = 0;
var ATTRS_ALL = (ATTR_REPLACED << 1) - 1; // All possible bits set var ATTRS_ALL = (ATTR_LAST << 1) - 1; // All possible bits set
// The traversal algorithms we run will recurse into children if you change any // The traversal algorithms we run will recurse into children if you change any
// attrs bit to zero. Use all bits set to maximally attributed, including // attrs bit to zero. Use all bits set to maximally attributed, including