TritonDataCenter / mdb_v8

postmortem debugging for Node.js and other V8-based programs
Mozilla Public License 2.0
240 stars 18 forks source link

v8o_optional serves no purpose #120

Open mgerdts opened 5 years ago

mgerdts commented 5 years ago

mdb_v8.c has:

typedef struct v8_offset {
    ssize_t     *v8o_valp;
    const char  *v8o_class;
    const char  *v8o_member;
    boolean_t   v8o_optional;
    uint32_t    v8o_flags;
    intptr_t    v8o_fallback;
} v8_offset_t;

v8o_flags has:

#define V8_CONSTANT_OPTIONAL        1
#define V8_CONSTANT_HASFALLBACK     2
#define V8_CONSTANT_REMOVED     4
#define V8_CONSTANT_ADDED       8

V8_CONSTANT_FALLBACK() is commonly used to indicate that a value is optional and that it has a fallback value by setting V8_CONSTANT_OPTIONAL, among other things. However, v8o_optional is set to non-zero and V8_CONSTANT_FALLBACK() is used, the fallback value will never be used. This is confusing.

Worse, there are some offsets that are improperly initialized such that v8o_optional is set to a non-zero value intended for v8o_flags. A proper v8_offset_t initializer is:

    { &V8_OFF_MAP_INOBJECT_PROPERTIES_OR_CTOR_FUN_INDEX,
        "Map", "inobject_properties_or_constructor_function_index",
        B_FALSE, V8_CONSTANT_FALLBACK(4, 6), 4 },

Examples of improper initializers that omit the (required) v8o_optional field are:

    { &V8_OFF_SHAREDFUNCTIONINFO_INFERRED_NAME,
        "SharedFunctionInfo", "inferred_name",
        V8_CONSTANT_REMOVED_SINCE(5, 1) },
#ifdef _LP64
    { &V8_OFF_SHAREDFUNCTIONINFO_IDENTIFIER,
        "SharedFunctionInfo", "function_identifier",
        V8_CONSTANT_FALLBACK(5, 1), 79},
#else
    { &V8_OFF_SHAREDFUNCTIONINFO_IDENTIFIER,
        "SharedFunctionInfo", "function_identifier",
        V8_CONSTANT_FALLBACK(5, 1), 39},
#endif

It is understandable that someone made this mistake. v8_constants[] uses very similar initializers but does not have an equivalent of the v8o_optional field.

Code inspection and testing suggests that v8o_optional should go away.