Skip to content

Map user-specified system libraries to correct variants #14355

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

Merged
merged 1 commit into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 35 additions & 28 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,12 @@ def __init__(self, args):
self.has_dash_c = False
self.has_dash_E = False
self.has_dash_S = False
self.libs = []
self.link_flags = []
self.lib_dirs = []
self.forced_stdlibs = []


def add_link_flag(state, i, f):
if f.startswith('-l'):
state.libs.append((i, f[2:]))
if f.startswith('-L'):
state.lib_dirs.append(f[2:])

Expand Down Expand Up @@ -638,7 +635,7 @@ def check(input_file):
else:
return True

return [f for f in inputs if check(f[1])]
return [f for f in inputs if check(f)]


def filter_out_duplicate_dynamic_libs(inputs):
Expand All @@ -654,7 +651,7 @@ def check(input_file):
seen.add(abspath)
return True

return [f for f in inputs if check(f[1])]
return [f for f in inputs if check(f)]


def process_dynamic_libs(dylibs, lib_dirs):
Expand Down Expand Up @@ -1053,7 +1050,7 @@ def run(args):
if state.mode == Mode.POST_LINK_ONLY:
settings.limit_settings(None)
target, wasm_target = phase_linker_setup(options, state, newargs, settings_map)
process_libraries(state.libs, state.lib_dirs, [])
process_libraries(state.link_flags, state.lib_dirs, [])
if len(input_files) != 1:
exit_with_error('--post-link requires a single input file')
phase_post_link(options, state, input_files[0][1], wasm_target, target)
Expand Down Expand Up @@ -1116,24 +1113,23 @@ def phase_calculate_linker_inputs(options, state, linker_inputs):
state.link_flags = filter_link_flags(state.link_flags, using_lld)

# Decide what we will link
consumed = process_libraries(state.libs, state.lib_dirs, linker_inputs)
# Filter out libraries that are actually JS libs
state.link_flags = [l for l in state.link_flags if l[0] not in consumed]
state.link_flags = process_libraries(state.link_flags, state.lib_dirs, linker_inputs)

linker_args = [val for _, val in sorted(linker_inputs + state.link_flags)]

# If we are linking to an intermediate object then ignore other
# "fake" dynamic libraries, since otherwise we will end up with
# multiple copies in the final executable.
if options.oformat == OFormat.OBJECT or options.ignore_dynamic_linking:
linker_inputs = filter_out_dynamic_libs(options, linker_inputs)
linker_args = filter_out_dynamic_libs(options, linker_args)
else:
linker_inputs = filter_out_duplicate_dynamic_libs(linker_inputs)
linker_args = filter_out_duplicate_dynamic_libs(linker_args)

if settings.MAIN_MODULE:
dylibs = [i[1] for i in linker_inputs if building.is_wasm_dylib(i[1])]
dylibs = [a for a in linker_args if building.is_wasm_dylib(a)]
process_dynamic_libs(dylibs, state.lib_dirs)

linker_arguments = [val for _, val in sorted(linker_inputs + state.link_flags)]
return linker_arguments
return linker_args


@ToolchainProfiler.profile_block('parse arguments')
Expand Down Expand Up @@ -1244,7 +1240,7 @@ def phase_setup(options, state, newargs, settings_map):
# library and attempt to find a library by the same name in our own library path.
# TODO(sbc): Do we really need this feature? See test_other.py:test_local_link
libname = strip_prefix(unsuffixed_basename(arg), 'lib')
state.libs.append((i, libname))
add_link_flag(state, i, '-l' + libname)
else:
input_files.append((i, arg))
elif arg.startswith('-L'):
Expand Down Expand Up @@ -1986,7 +1982,7 @@ def check_memory_setting(setting):
# When not declaring wasm module exports in outer scope one by one, disable minifying
# wasm module export names so that the names can be passed directly to the outer scope.
# Also, if using library_exports.js API, disable minification so that the feature can work.
if not settings.DECLARE_ASM_MODULE_EXPORTS or 'exports.js' in [x for _, x in state.libs]:
if not settings.DECLARE_ASM_MODULE_EXPORTS or '-lexports.js' in [x for _, x in state.link_flags]:
settings.MINIFY_ASMJS_EXPORT_NAMES = 0

# Enable minification of wasm imports and exports when appropriate, if we
Expand Down Expand Up @@ -3506,13 +3502,26 @@ def find_library(lib, lib_dirs):
return None


def process_libraries(libs, lib_dirs, linker_inputs):
def process_libraries(link_flags, lib_dirs, linker_inputs):
new_flags = []
libraries = []
consumed = []
suffixes = STATICLIB_ENDINGS + DYNAMICLIB_ENDINGS
system_libs_map = system_libs.Library.get_usable_variations()

# Find library files
for i, lib in libs:
for i, flag in link_flags:
if not flag.startswith('-l'):
new_flags.append((i, flag))
continue
lib = strip_prefix(flag, '-l')
# We don't need to resolve system libraries to absolute paths here, we can just
# let wasm-ld handle that. However, we do want to map to the correct variant.
# For example we map `-lc` to `-lc-mt` if we are building with threading support.
if 'lib' + lib in system_libs_map:
lib = system_libs_map['lib' + lib]
new_flags.append((i, '-l' + strip_prefix(lib.get_base_name(), 'lib')))
continue

logger.debug('looking for library "%s"', lib)

path = None
Expand All @@ -3524,22 +3533,20 @@ def process_libraries(libs, lib_dirs, linker_inputs):

if path:
linker_inputs.append((i, path))
consumed.append(i)
else:
jslibs = building.map_to_js_libs(lib)
if jslibs is not None:
libraries += [(i, jslib) for jslib in jslibs]
consumed.append(i)
elif building.map_and_apply_to_settings(lib):
consumed.append(i)
continue
jslibs = building.map_to_js_libs(lib)
if jslibs is not None:
libraries += [(i, jslib) for jslib in jslibs]
elif not building.map_and_apply_to_settings(lib):
new_flags.append((i, flag))

settings.SYSTEM_JS_LIBRARIES += libraries

# At this point processing SYSTEM_JS_LIBRARIES is finished, no more items will be added to it.
# Sort the input list from (order, lib_name) pairs to a flat array in the right order.
settings.SYSTEM_JS_LIBRARIES.sort(key=lambda lib: lib[0])
settings.SYSTEM_JS_LIBRARIES = [lib[1] for lib in settings.SYSTEM_JS_LIBRARIES]
return consumed
return new_flags


class ScriptSource:
Expand Down
14 changes: 13 additions & 1 deletion tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from runner import RunnerCore, path_from_root, is_slow_test, ensure_dir, disabled, make_executable
from runner import env_modify, no_mac, no_windows, requires_native_clang, with_env_modify
from runner import create_file, parameterized, NON_ZERO, node_pthreads, TEST_ROOT, test_file
from runner import compiler_for, read_file, read_binary
from runner import compiler_for, read_file, read_binary, EMBUILDER
from tools import shared, building, utils, deps_info
import jsrun
import clang_native
Expand Down Expand Up @@ -10630,3 +10630,15 @@ def test_no_deprecated(self):
def test_bad_export_name(self):
err = self.expect_fail([EMCC, '-sEXPORT_NAME=foo bar', test_file('hello_world.c')])
self.assertContained('error: EXPORT_NAME is not a valid JS identifier: `foo bar`', err)

def test_standard_library_mapping(self):
# Test the `-l` flags on the command line get mapped the correct libraries variant
self.run_process([EMBUILDER, 'build', 'libc-mt', 'libcompiler_rt-mt', 'libdlmalloc-mt'])
Copy link
Member

Choose a reason for hiding this comment

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

Why is the embuilder line needed? (auto library building should happen on line 10639?)

lgtm without line 10636

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Automatic building currently only works for libraries that are added automatically in system_libs.py:calculate().

Libraries specified on the command line are not currently auto-built. This is just how its always been as far as I cant tell. We could change that but its what this change is about.

Also, -nostdlib essentially bypasses system_libs.py:calculate() and doesn't do any auto-adding or auto-building of libraries.

This PR is an incremental improvement. We could integrating -l flags with the auto-building system but that would be bigger/separate change.


libs = ['-lc', '-lc_rt_wasm', '-lcompiler_rt', '-lmalloc']
err = self.run_process([EMCC, test_file('hello_world.c'), '-pthread', '-nostdlib', '-v'] + libs, stderr=PIPE).stderr

# Check the the linker was run with `-mt` variants because `-pthread` was passed.
self.assertContained(' -lc-mt ', err)
self.assertContained(' -ldlmalloc-mt ', err)
self.assertContained(' -lcompiler_rt-mt ', err)
15 changes: 8 additions & 7 deletions tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,13 +490,14 @@ def get_usable_variations(cls):

This returns a dictionary of simple names to Library objects.
"""
result = {}
for subclass in cls.get_inheritance_tree():
if subclass.name:
library = subclass.get_default_variation()
if library.can_build() and library.can_use():
result[subclass.name] = library
return result
if not hasattr(cls, 'useable_variations'):
cls.useable_variations = {}
for subclass in cls.get_inheritance_tree():
if subclass.name:
library = subclass.get_default_variation()
if library.can_build() and library.can_use():
cls.useable_variations[subclass.name] = library
return cls.useable_variations


class MTLibrary(Library):
Expand Down