Skip to content
New issue

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

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

Already on GitHub? # to your account

Null pointer is accessed when calling JS_EvalFunction #913

Closed
Please-just-dont opened this issue Feb 14, 2025 · 3 comments · Fixed by #942
Closed

Null pointer is accessed when calling JS_EvalFunction #913

Please-just-dont opened this issue Feb 14, 2025 · 3 comments · Fixed by #942

Comments

@Please-just-dont
Copy link

Please-just-dont commented Feb 14, 2025

I have made a minimal example of the problem I'm having. When I call JS_EvalFunction the first module is built, and then it iterates to build the array of dependent modules (I think). It iterates through req_module_entries, and req_module_entries[0].module is null in js_create_module_function().

#include "quickjs.h"
#include <stdio.h>

const char main_file[] = R"( 

import { func } from 'other.js';

func(5)
	
)";

const char other_file[] = R"(

export function func(number) { }

)";


JSModuleDef* my_js_module_loader(JSContext* ctx, const char* module_name, void* opaque) {


	JSValue func_val = JS_Eval(ctx, other_file, sizeof(other_file) - 1, "anything", JS_EVAL_TYPE_MODULE | JS_EVAL_FLAG_COMPILE_ONLY);

	if (JS_IsException(func_val)) {


		JSValue exception = JS_GetException(ctx);
		const char* error_str = JS_ToCString(ctx, exception);
		printf("Error: %s\n", error_str);
		JS_FreeCString(ctx, error_str);
		JS_FreeValue(ctx, func_val);
		return nullptr;
	}




	// Get the module definition
	JSModuleDef* module = (JSModuleDef*)JS_VALUE_GET_PTR(func_val);
	if (!module)
		throw;
	JS_FreeValue(ctx, func_val);  // Free the function value, module is still valid


	return module;

}



int main()
{
	JSRuntime* rt;
	JSContext* ctx;
	rt = JS_NewRuntime();
	ctx = JS_NewContext(rt);


	JS_SetModuleLoaderFunc(rt, nullptr, my_js_module_loader, nullptr);








	//JSValue js_value = JS_Eval(ctx, quickjs_js_code, strlen(quickjs_js_code), "first_file.js", JS_EVAL_TYPE_MODULE | JS_EVAL_FLAG_COMPILE_ONLY);
	JSValue js_value = JS_Eval(ctx, main_file, sizeof(main_file) - 1, "anything", JS_EVAL_TYPE_MODULE | JS_EVAL_FLAG_COMPILE_ONLY);

	if (JS_IsException(js_value))
	{
		JSValue exception = JS_GetException(ctx);
		const char* error_str = JS_ToCString(ctx, exception);
		printf("Error: %s\n", error_str);
		JS_FreeCString(ctx, error_str);
	}

	size_t bytecode_size;
	uint8_t* bytecode_buf = JS_WriteObject(ctx, &bytecode_size, js_value, JS_WRITE_OBJ_BYTECODE);

	if (!bytecode_buf) throw;

	JS_FreeValue(ctx, js_value);






	JSValue func_val = JS_ReadObject(ctx, bytecode_buf, bytecode_size, JS_READ_OBJ_BYTECODE);

	if (JS_IsException(func_val)) {
		JSValue exception = JS_GetException(ctx);
		const char* error_str = JS_ToCString(ctx, exception);
		printf("Error: %s\n", error_str);
		
		throw;
		//return false;
	}


	JSValue result = JS_EvalFunction(ctx, func_val); // NULL PTR IS ACCESSED
	// req_module_entries[0].module is null in js_create_module_function() 
	

	if (JS_IsException(func_val)) {
		JSValue exception = JS_GetException(ctx);
		const char* error_str = JS_ToCString(ctx, exception);
		printf("Error: %s\n", error_str);

	}


return 0;

}
@bnoordhuis
Copy link
Contributor

Can you submit your test case as a pull request that updates (freshly added) api-test.c? Please adhere to the code style.

@bnoordhuis
Copy link
Contributor

I looked into this and the problem is serializing and deserializing an unlinked module, then trying to link, evaluate and instantiate it. Context is lost during (de)serialization and looks like it's hard to preserve or recompute that.

The easiest fix is rejecting unlinked modules in the serializer but that's not very convenient. Let me think if there's a better way.

bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Mar 2, 2025
A module's imports are not serialized along with the module itself and
that left the deserialized module with dangling references. Fix that by
checking the module cache first, the module loader second.

A glaring problem with cache checking is that the cached module doesn't
have to be the module it was at the time of serialization.

Why not call out to the module loader right away? Because then a module
can get loaded twice and that's arguably even worse.

The alternative of serializing modules transitively doesn't work for
C modules and is also prone to loading the same module twice.

Fixes: quickjs-ng#913
bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Mar 2, 2025
A module's imports are not serialized along with the module itself and
that left the deserialized module with dangling references. Fix that by
checking the module cache first, the module loader second.

A glaring problem with cache checking is that the cached module doesn't
have to be the module it was at the time of serialization.

Why not call out to the module loader right away? Because then a module
can get loaded twice and that's arguably even worse.

The alternative of serializing modules transitively doesn't work for
C modules and is also prone to loading the same module twice.

Fixes: quickjs-ng#913
@bnoordhuis
Copy link
Contributor

Can you check if #942 works for you?

bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Mar 6, 2025
A module's imports are not serialized along with the module itself and
that left the deserialized module with dangling references. Fix that by
checking the module cache first, the module loader second.

A glaring problem with cache checking is that the cached module doesn't
have to be the module it was at the time of serialization.

Why not call out to the module loader right away? Because then a module
can get loaded twice and that's arguably even worse.

The alternative of serializing modules transitively doesn't work for
C modules and is also prone to loading the same module twice.

Fixes: quickjs-ng#913
bnoordhuis added a commit that referenced this issue Mar 6, 2025
A module's imports are not serialized along with the module itself and
that left the deserialized module with dangling references. Fix that by
checking the module cache first, the module loader second.

A glaring problem with cache checking is that the cached module doesn't
have to be the module it was at the time of serialization.

Why not call out to the module loader right away? Because then a module
can get loaded twice and that's arguably even worse.

The alternative of serializing modules transitively doesn't work for
C modules and is also prone to loading the same module twice.

Fixes: #913
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants