Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: js atom null #333

Merged
merged 1 commit into from
Jul 19, 2024
Merged

fix: js atom null #333

merged 1 commit into from
Jul 19, 2024

Conversation

stefan-gorules
Copy link
Contributor

Hi, I've ran into an issue on Linux where calling Module::import resulted in an error with UTF8. After closer inspection I've noticed that Ctx::script_or_module_name generated what appeared to be pseudo-random values. I've dug into JS_GetScriptOrModuleName in c:

/* return JS_ATOM_NULL if the name cannot be found. Only works with
   not striped bytecode functions. */
JSAtom JS_GetScriptOrModuleName(JSContext *ctx, int n_stack_levels)
{
    JSStackFrame *sf;
    JSFunctionBytecode *b;
    JSObject *p;
    /* XXX: currently we just use the filename of the englobing
       function from the debug info. May need to add a ScriptOrModule
       info in JSFunctionBytecode. */
    sf = ctx->rt->current_stack_frame;
    if (!sf)
        return JS_ATOM_NULL;
    while (n_stack_levels-- > 0) {
        sf = sf->prev_frame;
        if (!sf)
            return JS_ATOM_NULL;
    }
    for(;;) {
        if (JS_VALUE_GET_TAG(sf->cur_func) != JS_TAG_OBJECT)
            return JS_ATOM_NULL;
        p = JS_VALUE_GET_OBJ(sf->cur_func);
        if (!js_class_has_bytecode(p->class_id))
            return JS_ATOM_NULL;
        b = p->u.func.function_bytecode;
        if (!b->is_direct_or_indirect_eval) {
            if (!b->has_debug)
                return JS_ATOM_NULL;
            return JS_DupAtom(ctx, b->debug.filename);
        } else {
            sf = sf->prev_frame;
            if (!sf)
                return JS_ATOM_NULL;
        }
    }
}

We can see that JS_ATOM_NULL is returned in case script name cannot be resolved. This is not the same value as the one currently being checked against (JS_ATOM_null = 1, __JS_ATOM_NULL = 0). As is defined in .h.

#define JS_ATOM_NULL 0

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.83%. Comparing base (304db5d) to head (63f8168).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #333       +/-   ##
===========================================
- Coverage   68.31%   55.83%   -12.48%     
===========================================
  Files          83       83               
  Lines       12237    12237               
===========================================
- Hits         8360     6833     -1527     
- Misses       3877     5404     +1527     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DelSkayn
Copy link
Owner

Good catch! Thanks for the PR!

@DelSkayn DelSkayn merged commit b58c9dd into DelSkayn:master Jul 19, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants