Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Make changes according to dap's code review:
  - Do not warn if JSTypedArray type is missing.
  - Clarify/added comments.
  - Formatting/indentation nits.
  - Print <Typed array of length N> when ::jsprinting a typed array.
  - get_map_constructor fails if V8_OFF_MAP_CONSTRUCTOR is missing.
  - Update tests accordingly.

Also, use a separate V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER constant to
store the constructor_or_backpointer offset, and set
V8_OFF_MAP_CONSTRUCTOR with that value if V8_OFF_MAP_CONSTRUCTOR is -1.
Before that change, the same variable was used for both offsets, and
for node versions < 4.x, it would result in V8_OFF_MAP_CONSTRUCTOR
always being -1, which would break, among other things,
get_map_constructor and thus ::findjsobjects.
  • Loading branch information
Julien Gilli committed Sep 22, 2015
1 parent af31011 commit 249d5c5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 46 deletions.
65 changes: 21 additions & 44 deletions src/mdb_v8.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ ssize_t V8_OFF_JSFUNCTION_SHARED;
ssize_t V8_OFF_JSOBJECT_ELEMENTS;
ssize_t V8_OFF_JSOBJECT_PROPERTIES;
ssize_t V8_OFF_MAP_CONSTRUCTOR;
ssize_t V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER;
ssize_t V8_OFF_MAP_INOBJECT_PROPERTIES;
ssize_t V8_OFF_MAP_INSTANCE_ATTRIBUTES;
ssize_t V8_OFF_MAP_INSTANCE_DESCRIPTORS;
Expand Down Expand Up @@ -419,7 +420,7 @@ static v8_offset_t v8_offsets[] = {
{ &V8_OFF_MAP_CONSTRUCTOR,
"Map", "constructor",
B_FALSE, V8_CONSTANT_REMOVED_SINCE(4, 3)},
{ &V8_OFF_MAP_CONSTRUCTOR,
{ &V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER,
"Map", "constructor_or_backpointer",
B_FALSE, V8_CONSTANT_ADDED_SINCE(4, 3)},
{ &V8_OFF_MAP_INOBJECT_PROPERTIES,
Expand Down Expand Up @@ -721,11 +722,6 @@ autoconfigure(v8_cfg_t *cfgp)
failed++;
}

if (V8_TYPE_JSTYPEDARRAY == -1) {
mdb_warn("couldn't find JSTypedArray type\n");
failed++;
}

/*
* It's non-fatal if we can't find HeapNumber, JSDate, JSRegExp, or
* Oddball because they're only used for heuristics. It's not even a
Expand Down Expand Up @@ -863,6 +859,18 @@ autoconfigure(v8_cfg_t *cfgp)
}
}

/*
* With V8 version 4.3, a new "constructor_or_backpointer" field
* replaces the original "constructor" field. Both of them can't
* exist at the same time, and the data they point have similar
* semantics (at least similar enough so that the current code can
* handle either of them transparently). So if the new field exists,
* copy its value into the variable used to hold the old field to
* allow the code to be more concise.
*/
if (V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER != -1)
V8_OFF_MAP_CONSTRUCTOR = V8_OFF_MAP_CONSTRUCTOR_OR_BACKPOINTER;

return (failed ? -1 : 0);
}

Expand Down Expand Up @@ -1552,7 +1560,7 @@ read_heap_dict(uintptr_t addr,
}

/*
* Given a JavaScript object's Map "map", stores its constructor
* Given a JavaScript object's Map "map", stores the object's constructor
* in valp. Returns 0 if it succeeded, -1 otherwise.
*/
static int
Expand All @@ -1561,6 +1569,9 @@ get_map_constructor(uintptr_t *valp, uintptr_t map) {
int constructor_found = 0;
uint8_t type;

if (V8_OFF_MAP_CONSTRUCTOR == -1)
return (-1);

/*
* https://codereview.chromium.org/950283002, which landed in V8 4.3.x,
* makes the "constructor" and "backpointer to transitions" field
Expand All @@ -1571,7 +1582,7 @@ get_map_constructor(uintptr_t *valp, uintptr_t map) {
*/
while (constructor_found == 0) {
if (read_heap_ptr(&constructor_candidate,
map, V8_OFF_MAP_CONSTRUCTOR) != 0)
map, V8_OFF_MAP_CONSTRUCTOR) != 0)
return (-1);

if (read_typebyte(&type, constructor_candidate) != 0)
Expand Down Expand Up @@ -3448,7 +3459,6 @@ jsobj_print_jstyped_array(uintptr_t addr, jsobj_print_t *jsop)
{
char **bufp = jsop->jsop_bufp;
size_t *lenp = jsop->jsop_lenp;
int indent = jsop->jsop_indent;
uintptr_t length;

if (V8_OFF_JSTYPEDARRAY_LENGTH == -1 ||
Expand All @@ -3458,41 +3468,8 @@ jsobj_print_jstyped_array(uintptr_t addr, jsobj_print_t *jsop)
return (-1);
}

if (jsop->jsop_member != NULL) {
if (strcmp(jsop->jsop_member, "length") == 0) {

if (jsop->jsop_baseaddr != NULL)
(void) bsnprintf(bufp, lenp, "%p: ",
jsop->jsop_baseaddr);

(void) bsnprintf(bufp, lenp, "%d", (int)length);
jsop->jsop_found = B_TRUE;
jsop->jsop_member = NULL;
return (0);
}

return (jsobj_properties(addr, jsobj_print_prop_member,
jsop, &jsop->jsop_propinfo));
}

(void) bsnprintf(bufp, lenp, "{\n");
(void) bsnprintf(bufp, lenp, "%*s", indent + 4, "");

(void) bsnprintf(bufp, lenp, "\"length\": ");

if (jsop->jsop_baseaddr != NULL && jsop->jsop_member == NULL)
(void) bsnprintf(bufp, lenp, "%p: ", jsop->jsop_baseaddr);

(void) bsnprintf(bufp, lenp, "%d,", (int)length);

++jsop->jsop_nprops;

jsobj_properties(addr, jsobj_print_prop,
jsop, &jsop->jsop_propinfo);
(void) bsnprintf(bufp, lenp, "\n");

(void) bsnprintf(bufp, lenp, "%*s", indent, "");
(void) bsnprintf(bufp, lenp, "}");
(void) bsnprintf(bufp, lenp, "<Typed array of length ");
(void) bsnprintf(bufp, lenp, "%d>", (int)length);

return (0);
}
Expand Down
14 changes: 12 additions & 2 deletions test/standalone/tst.postmortem_details.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,19 @@ gcore.on('exit', function (code) {
var mod = util.format('::load %s\n', common.dmodpath());
mdb.stdin.write(mod);
mdb.stdin.write('!echo test: jsconstructor\n');
mdb.stdin.write('::findjsobjects -p my_buffer | ::findjsobjects | ' +
'::jsprint -b length ! awk -F: \'$2 == ' + bufstr.length +
// Starting from node v4.0, buffers are actually Uint8Array instances,
// and they don't have a "length" property
if (process.versions.node < '4.0') {
mdb.stdin.write('::findjsobjects -p my_buffer | ' +
'::findjsobjects | ' + '::jsprint -b length ! ' +
'awk -F: \'$2 == ' + bufstr.length +
'{ print $1 }\'' + '| head -1 > ' + tmpfile + '\n');
} else {
mdb.stdin.write('::findjsobjects -p my_buffer | ' +
'::findjsobjects | ' + '::jsprint -b ! ' +
'awk -F: \'index($2, "length ' + bufstr.length + '>") > 0 ' +
'{ print $1 }\'' + '| head -1 > ' + tmpfile + '\n');
}
mdb.stdin.write('::cat ' + tmpfile + ' | ::jsconstructor\n');
mdb.stdin.write('!echo test: nodebuffer\n');
mdb.stdin.write('::cat ' + tmpfile + ' | ::nodebuffer\n');
Expand Down

0 comments on commit 249d5c5

Please sign in to comment.