Skip to content

Use execve() to replace system() #4223

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use execve() to replace system() #4223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions core/iwasm/compilation/aot_compiler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4195,13 +4195,13 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
bh_print_time("Begin to emit object file");

if (comp_ctx->external_llc_compiler || comp_ctx->external_asm_compiler) {
char cmd[1024];
int ret;

if (comp_ctx->external_llc_compiler) {
const char *stack_usage_flag = "";
char bc_file_name[64];
char su_file_name[65]; /* See the comment below */
char *stack_usage_flag = "";
char bc_file_name[64] = { 0 };
char su_file_name[65] = { 0 };
char *argv[10] = { 0 };

if (comp_ctx->stack_usage_file != NULL) {
/*
Expand Down Expand Up @@ -4229,14 +4229,16 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
return false;
}

snprintf(cmd, sizeof(cmd), "%s%s %s -o %s %s",
comp_ctx->external_llc_compiler, stack_usage_flag,
comp_ctx->llc_compiler_flags ? comp_ctx->llc_compiler_flags
: "-O3 -c",
file_name, bc_file_name);
LOG_VERBOSE("invoking external LLC compiler:\n\t%s", cmd);
argv[0] = stack_usage_flag;
argv[1] = comp_ctx->llc_compiler_flags
? (char *)comp_ctx->llc_compiler_flags
: "-O3 -c";
argv[2] = "-o";
argv[3] = file_name;
argv[4] = bc_file_name;
argv[5] = NULL;

ret = bh_system(cmd);
ret = bh_execve(comp_ctx->external_llc_compiler, argv, 6);
/* remove temp bitcode file */
unlink(bc_file_name);

Expand All @@ -4263,7 +4265,8 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
}
}
else if (comp_ctx->external_asm_compiler) {
char asm_file_name[64];
char asm_file_name[64] = { 0 };
char *argv[10] = { 0 };

if (!aot_generate_tempfile_name("wamrc-asm", "s", asm_file_name,
sizeof(asm_file_name))) {
Expand All @@ -4282,14 +4285,15 @@ aot_emit_object_file(AOTCompContext *comp_ctx, char *file_name)
return false;
}

snprintf(cmd, sizeof(cmd), "%s %s -o %s %s",
comp_ctx->external_asm_compiler,
comp_ctx->asm_compiler_flags ? comp_ctx->asm_compiler_flags
: "-O3 -c",
file_name, asm_file_name);
LOG_VERBOSE("invoking external ASM compiler:\n\t%s", cmd);
argv[0] = comp_ctx->asm_compiler_flags
? (char *)comp_ctx->asm_compiler_flags
: "-O3 -c";
argv[1] = "-o";
argv[2] = file_name;
argv[3] = asm_file_name;
argv[4] = NULL;

ret = bh_system(cmd);
ret = bh_execve(comp_ctx->external_asm_compiler, argv, 5);
/* remove temp assembly file */
unlink(asm_file_name);

Expand Down
33 changes: 18 additions & 15 deletions core/iwasm/compilation/aot_emit_aot_file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4339,18 +4339,23 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
else if (!strncmp(LLVMGetTargetName(target), "arc", 3)) {
/* Emit to assembly file instead for arc target
as it cannot emit to object file */
char file_name[] = "wasm-XXXXXX", buf[128];
char file_name[] = "wasm-XXXXXX";
char assembly_file_name[64] = { 0 };
char object_file_name[64] = { 0 };
int ret;
char *argv[] = { "-mcpu=arcem", "-o", object_file_name, "-c",
assembly_file_name, NULL };

if (!bh_mkstemp(file_name, sizeof(file_name))) {
aot_set_last_error("make temp file failed.");
goto fail;
}

snprintf(buf, sizeof(buf), "%s%s", file_name, ".s");
snprintf(assembly_file_name, sizeof(assembly_file_name) - 1, "%s.s",
file_name);
if (LLVMTargetMachineEmitToFile(comp_ctx->target_machine,
comp_ctx->module, buf, LLVMAssemblyFile,
&err)
comp_ctx->module, assembly_file_name,
LLVMAssemblyFile, &err)
!= 0) {
if (err) {
LLVMDisposeMessage(err);
Expand All @@ -4363,14 +4368,14 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
/* call arc gcc to compile assembly file to object file */
/* TODO: get arc gcc from environment variable firstly
and check whether the toolchain exists actually */
snprintf(buf, sizeof(buf), "%s%s%s%s%s%s",
"/opt/zephyr-sdk/arc-zephyr-elf/bin/arc-zephyr-elf-gcc ",
"-mcpu=arcem -o ", file_name, ".o -c ", file_name, ".s");
snprintf(object_file_name, sizeof(object_file_name) - 1, "%s.o",
file_name);
/* TODO: use try..catch to handle possible exceptions */
ret = bh_system(buf);
/* TODO: use ZEPHYR_SDK_INSTALL_DIR to construct the path */
ret = bh_execve("/opt/zephyr-sdk/arc-zephyr-elf/bin/arc-zephyr-elf-gcc",
argv, 6);
/* remove temp assembly file */
snprintf(buf, sizeof(buf), "%s%s", file_name, ".s");
unlink(buf);
unlink(assembly_file_name);

if (ret != 0) {
aot_set_last_error("failed to compile asm file to obj file "
Expand All @@ -4379,12 +4384,10 @@ aot_obj_data_create(AOTCompContext *comp_ctx)
}

/* create memory buffer from object file */
snprintf(buf, sizeof(buf), "%s%s", file_name, ".o");
ret = LLVMCreateMemoryBufferWithContentsOfFile(buf, &obj_data->mem_buf,
&err);
ret = LLVMCreateMemoryBufferWithContentsOfFile(
object_file_name, &obj_data->mem_buf, &err);
/* remove temp object file */
snprintf(buf, sizeof(buf), "%s%s", file_name, ".o");
unlink(buf);
unlink(object_file_name);

if (ret != 0) {
if (err) {
Expand Down
30 changes: 27 additions & 3 deletions core/shared/utils/bh_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,39 @@ wa_strdup(const char *s)
}

#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
/* need to make sure that The `argv[]` must be terminated by a NULL pointer. */
int
bh_system(const char *cmd)
bh_execve(const char *pathname, char *const argv[], int argc)
{
int ret;
/* no environment variables */
char *const envp[] = { NULL };

if (pathname == NULL) {
return -1;
}

if (argc > 0) {
if (argv == NULL) {
return -1;
}

/* The `argv[]` must be terminated by a NULL pointer. */
if (argv[argc - 1] != NULL) {
return -1;
}
}

#if !(defined(_WIN32) || defined(_WIN32_))
ret = system(cmd);
ret = execve(pathname, argv, envp);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work at all? i suppose you need fork+execve+wait.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

execve() executes the program referred to by pathname. This causes the program that is currently being run by the calling process to be replaced with a new program, with newly initialized stack, heap, and (initialized and uninitialized) data segments.

IMU, in our scenarios, there is no need to fork a new process and run commands. The default behavior is sufficient

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think so. wamrc has other things to do after invoking these external commands.
have you tested?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I used commands like WMRC_LLC_COMPILER=/SDF/051_llvm_releases/clang+llvm-15.0.6-x86_64-linux-gnu-ubuntu-18.04/bin/clang WAMRC_LLC_FLAGS="-mcmodel=medium" /workspaces/wasm-micro-runtime/wamr-compiler/build/wamrc -o xyz.aot ./xyz.wasm.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I‘m also curious about how this works. The man page says:

execve() does not return on success, and the text, initialized data, uninitialized data (bss), and stack of
the calling process are overwritten according to the contents of the newly loaded program.

That means the later instructions after execve will not be executed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reminder. Now I see both of your points.

#ifndef NDEBUG
if (ret == -1) {
LOG_WARNING("execute \"%s\" failed because of \"%s\"", pathname,
strerror(errno));
}
#endif
#else
ret = _spawnlp(_P_WAIT, "cmd.exe", "/c", cmd, NULL);
ret = _execve(pathname, argv, envp);
#endif

return ret;
Expand Down
14 changes: 12 additions & 2 deletions core/shared/utils/bh_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,19 @@ char *
wa_strdup(const char *s);

#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
/* Executes a system command in bash/cmd.exe */
/*
* Executes a program referred to by cmd in bash/cmd.exe
* Always be sure that argv[argc-1] == NULL
*
* @param pathname The program to execute. need to be absolute path.
* @param argv The command line arguments.
* @param argc The number of command line arguments.
*
* like to execute "ls -l /tmp":
* bh_execve("/bin/ls", (char *const []){ "-l", "/tmp", NULL }, 3);
*/
int
bh_system(const char *cmd);
bh_execve(const char *pathname, char *const argv[], int argc);

/* Tests whether can create a temporary file with the given name */
bool
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/shared-utils/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ set (unit_test_sources

add_executable (shared_utils_test ${unit_test_sources})

target_link_libraries (shared_utils_test gtest_main)
target_link_libraries (shared_utils_test gtest_main ${LLVM_AVAILABLE_LIBS})

gtest_discover_tests(shared_utils_test)
10 changes: 10 additions & 0 deletions tests/unit/shared-utils/bh_common_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,13 @@ TEST_F(bh_common_test_suite, b_memcpy_s)
EXPECT_EQ(-1, b_memcpy_s(dest, sizeof(dest), nullptr, sizeof(STR_TEST)));
EXPECT_EQ(-1, b_memcpy_s(dest, sizeof(dest), STR_TEST, sizeof(dest) + 1));
}

TEST_F(bh_common_test_suite, bh_execve)
{
#if WASM_ENABLE_WAMR_COMPILER != 0 || WASM_ENABLE_JIT != 0
char *const argv[] = { "-n", "\"hello\"", "world", "\n", nullptr };
EXPECT_EQ(0, bh_execve("/usr/bin/echo", argv, 5));
#else
GTEST_SKIP() << "bh_execve() is not supported";
#endif
}
Loading