Skip to content

Commit

Permalink
fix sysimage-native-code=no option with pkgimages (#53373)
Browse files Browse the repository at this point in the history
Loading pkgimages would try to access the sysimage native code, which
will fail. Ensure that no code tries to load if the sysimage native code
is not available, as it may try to link against it.

Fixes #53147
  • Loading branch information
vtjnash authored Feb 19, 2024
1 parent 9c0f1dc commit 02699bb
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -607,15 +607,8 @@ extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_
extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_system_image_size;
static void jl_load_sysimg_so(void)
{
int imaging_mode = jl_generating_output() && !jl_options.incremental;
// in --build mode only use sysimg data, not precompiled native code
if (!imaging_mode && jl_options.use_sysimage_native_code==JL_OPTIONS_USE_SYSIMAGE_NATIVE_CODE_YES) {
assert(sysimage.fptrs.ptrs);
}
else {
memset(&sysimage.fptrs, 0, sizeof(sysimage.fptrs));
}
const char *sysimg_data;
assert(sysimage.fptrs.ptrs); // jl_init_processor_sysimg should already be run
if (jl_sysimg_handle == jl_exe_handle &&
&jl_system_image_data != JL_WEAK_SYMBOL_DEFAULT(system_image_data_unavailable))
sysimg_data = (const char*)&jl_system_image_data;
Expand Down Expand Up @@ -3097,6 +3090,17 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl
htable_new(&new_dt_objs, 0);
arraylist_new(&deser_sym, 0);

// in --build mode only use sysimg data, not precompiled native code
int imaging_mode = jl_generating_output() && !jl_options.incremental;
if (!imaging_mode && jl_options.use_sysimage_native_code == JL_OPTIONS_USE_SYSIMAGE_NATIVE_CODE_YES) {
if (image->gvars_base)
assert(image->fptrs.ptrs);
}
else {
memset(&image->fptrs, 0, sizeof(image->fptrs));
image->gvars_base = NULL;
}

// step 1: read section map
assert(ios_pos(f) == 0 && f->bm == bm_mem);
size_t sizeof_sysdata = read_uint(f);
Expand Down

7 comments on commit 02699bb

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

A few scattered improvements, but also some extreme regressions, particularly in inference but also in arrays:

["array", "growth", ("push_single!", 2048)] 2.75 (5%) ❌ 1.00 (1%)
["array", "growth", ("push_single!", 256)] 2.69 (5%) ❌ 1.00 (1%)
["array", "growth", ("push_single!", 8)] 2.69 (5%) ❌ 1.00

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks("inference", vs="@41a6e7b6c839f4828f1dbda66725aabc78b75d50")

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

Please sign in to comment.