Skip to content

emscripten_strict #4665

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 18 commits into from
Dec 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6a148a2
If environment variable EMSCRIPTEN_STRICT is set, don't pass the prep…
juj Oct 24, 2016
8e3bf5b
-s ERROR_ON_UNDEFINED_SYMBOLS=1 should be the default value, so set i…
juj Oct 24, 2016
b8e9130
Convert "emcc: cannot find library" warning to an option configurable…
juj Oct 24, 2016
4e1eabe
Remove Emscripten include path system/include/emscripten in EMSCRIPTE…
juj Oct 24, 2016
f2d975a
Add EMSCRIPTEN_STRICT as a linker setting as well.
juj Oct 24, 2016
1b3fe42
In EMSCRIPTEN_STRICT mode, stop autolinking to all system provided JS…
juj Oct 24, 2016
72c97d1
Rename Emscripten strict mode to EMCC_STRICT=1 env var/-s STRICT=1 li…
juj Oct 26, 2016
381eca8
Add root level include emscripten.h so that both forms #include <emsc…
juj Oct 26, 2016
af265a5
Finalize EMCC_STRICT mode for linking to libraries. Improve browser s…
juj Oct 26, 2016
1b4e784
Move the deduction of which system JS libraries to link to class Buil…
juj Nov 19, 2016
11e4634
Apply the set of system JS libraries based on the link settings via a…
juj Nov 22, 2016
12d47b0
Indentation fix in src/library_fs.js
juj Nov 22, 2016
815a35b
Revise how EMCC_STRICT and -s STRICT=1 are read so that they are alwa…
juj Nov 22, 2016
022c4db
Add test for scenario when not explicitly linking to system JS librar…
juj Nov 22, 2016
08b70e5
Accept EMCC_STRICT=somethingotherthanzero as true for EMCC_STRICT
juj Dec 7, 2016
267c4ee
Add export of JS lib ifdefs for all libs, even when not in strict mode.
juj Dec 7, 2016
8455fba
Fix typo in looking up path_from_root()
juj Dec 7, 2016
9b727e8
Apply code review to Emscripten strict mode.
juj Dec 17, 2016
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
47 changes: 44 additions & 3 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,11 @@ def filter_emscripten_options(argv):
if compiler == shared.EMCC: compiler = [shared.PYTHON, shared.EMCC]
else: compiler = [compiler]
cmd = compiler + list(filter_emscripten_options(sys.argv[1:]))
if not use_js: cmd += shared.EMSDK_OPTS + ['-D__EMSCRIPTEN__', '-DEMSCRIPTEN']
if not use_js:
cmd += shared.EMSDK_OPTS + ['-D__EMSCRIPTEN__']
# The preprocessor define EMSCRIPTEN is deprecated. Don't pass it to code in strict mode. Code should use the define __EMSCRIPTEN__ instead.
if not shared.Settings.STRICT:
cmd += ['-DEMSCRIPTEN']
if use_js: cmd += ['-s', 'ERROR_ON_UNDEFINED_SYMBOLS=1'] # configure tests should fail when an undefined symbol exists

logging.debug('just configuring: ' + ' '.join(cmd))
Expand Down Expand Up @@ -918,6 +922,29 @@ def detect_fixed_language_mode(args):
if separate_asm:
shared.Settings.SEPARATE_ASM = os.path.basename(asm_target)

if 'EMCC_STRICT' in os.environ:
Copy link
Member

Choose a reason for hiding this comment

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

this happens after the code above for -DEMSCRIPTEN, so it seems it would only work for the setting and not the environment var?

Copy link
Member

Choose a reason for hiding this comment

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

or wait, is the above for the configure stuff? then it's ok.

shared.Settings.STRICT = os.environ.get('EMCC_STRICT') != '0'

# Libraries are searched before settings_changes are applied, so apply the value for STRICT and ERROR_ON_MISSING_LIBRARIES from
# command line already now.

def get_last_setting_change(setting):
return ([None] + filter(lambda x: x.startswith(setting + '='), settings_changes))[-1]

strict_cmdline = get_last_setting_change('STRICT')
if strict_cmdline:
shared.Settings.STRICT = int(strict_cmdline[len('STRICT='):])

if shared.Settings.STRICT:
shared.Settings.ERROR_ON_UNDEFINED_SYMBOLS = 1
shared.Settings.ERROR_ON_MISSING_LIBRARIES = 1

error_on_missing_libraries_cmdline = get_last_setting_change('ERROR_ON_MISSING_LIBRARIES')
if error_on_missing_libraries_cmdline:
shared.Settings.ERROR_ON_MISSING_LIBRARIES = int(error_on_missing_libraries_cmdline[len('ERROR_ON_MISSING_LIBRARIES='):])

system_js_libraries = []

# Find library files
for i, lib in libs:
logging.debug('looking for library "%s"', lib)
Expand All @@ -934,8 +961,13 @@ def detect_fixed_language_mode(args):
break
if found: break
if found: break
if not found and lib not in ['GL', 'GLU', 'glut', 'm', 'c', 'SDL', 'stdc++', 'pthread']: # whitelist our default libraries
logging.warning('emcc: cannot find library "%s"', lib)
if not found:
system_js_libraries += shared.Building.path_to_system_js_libraries(lib)

# Certain linker flags imply some link libraries to be pulled in by default.
Copy link
Member

Choose a reason for hiding this comment

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

i still feel these lines are nicer in something like shared.Building.add_default_system_libraries_from_settings_changes (or a better name ;) ), but i don't feel strongly about it.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe leave that for a possible followup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, added a function for that in Building. Not sure if I managed a better name though :)

system_js_libraries += shared.Building.path_to_system_js_libraries_for_settings(settings_changes)

settings_changes.append('SYSTEM_JS_LIBRARIES="' + ','.join(system_js_libraries) + '"')

# If not compiling to JS, then we are compiling to an intermediate bitcode objects or library, so
# ignore dynamic linking, since multiple dynamic linkings can interfere with each other
Expand Down Expand Up @@ -999,6 +1031,15 @@ def check(input_file):
if shared.get_llvm_target() == shared.WASM_TARGET:
shared.Settings.WASM_BACKEND = 1

if not shared.Settings.STRICT:
# The preprocessor define EMSCRIPTEN is deprecated. Don't pass it to code in strict mode. Code should use the define __EMSCRIPTEN__ instead.
shared.COMPILER_OPTS += ['-DEMSCRIPTEN']

# The system include path system/include/emscripten/ is deprecated, i.e. instead of #include <emscripten.h>, one should pass in #include <emscripten/emscripten.h>.
# This path is not available in Emscripten strict mode.
if shared.USE_EMSDK:
shared.C_INCLUDE_PATHS += [shared.path_from_root('system', 'include', 'emscripten')]

# Use settings

try:
Expand Down
18 changes: 17 additions & 1 deletion src/library_fs.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
mergeInto(LibraryManager.library, {
$FS__deps: ['$ERRNO_CODES', '$ERRNO_MESSAGES', '__setErrNo', '$PATH', '$TTY', '$MEMFS', '$IDBFS', '$NODEFS', '$WORKERFS', 'stdin', 'stdout', 'stderr'],
$FS__deps: ['$ERRNO_CODES', '$ERRNO_MESSAGES', '__setErrNo', '$PATH', '$TTY', '$MEMFS',
#if __EMSCRIPTEN_HAS_idbfs_js__
'$IDBFS',
#endif
#if __EMSCRIPTEN_HAS_nodefs_js__
'$NODEFS',
#endif
#if __EMSCRIPTEN_HAS_workerfs_js__
'$WORKERFS',
#endif
'stdin', 'stdout', 'stderr'],
$FS__postset: 'FS.staticInit();' +
'__ATINIT__.unshift(function() { if (!Module["noFSInit"] && !FS.init.initialized) FS.init() });' +
'__ATMAIN__.push(function() { FS.ignorePermissions = false });' +
Expand Down Expand Up @@ -1382,9 +1392,15 @@ mergeInto(LibraryManager.library, {

FS.filesystems = {
'MEMFS': MEMFS,
#if __EMSCRIPTEN_HAS_idbfs_js__
'IDBFS': IDBFS,
#endif
#if __EMSCRIPTEN_HAS_nodefs_js__
'NODEFS': NODEFS,
#endif
#if __EMSCRIPTEN_HAS_workerfs_js__
'WORKERFS': WORKERFS,
#endif
};
},
init: function(input, output, error) {
Expand Down
70 changes: 48 additions & 22 deletions src/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,41 +96,67 @@ var LibraryManager = {
load: function() {
if (this.library) return;

// Core system libraries (always linked against)
var libraries = [
'library.js',
'library_browser.js',
'library_formatString.js',
'library_path.js',
'library_syscall.js'
'library_signals.js',
'library_syscall.js',
'library_html5.js'
];

if (!NO_FILESYSTEM) {
// Core filesystem libraries (always linked against, unless -s NO_FILESYSTEM=1 is specified)
libraries = libraries.concat([
'library_fs.js',
'library_idbfs.js',
'library_memfs.js',
'library_nodefs.js',
'library_sockfs.js',
'library_workerfs.js',
'library_tty.js',
'library_lz4.js',
]);

// Additional filesystem libraries (in strict mode, link to these explicitly via -lxxx.js)
if (!STRICT) {
libraries = libraries.concat([
'library_idbfs.js',
'library_nodefs.js',
'library_sockfs.js',
'library_workerfs.js',
'library_lz4.js',
]);
}
}

// Additional JS libraries (in strict mode, link to these explicitly via -lxxx.js)
if (!STRICT) {
libraries = libraries.concat([
'library_sdl.js',
'library_gl.js',
'library_glut.js',
'library_xlib.js',
'library_egl.js',
'library_openal.js',
'library_glfw.js',
'library_uuid.js',
'library_glew.js',
'library_idbstore.js',
'library_async.js',
'library_vr.js'
]);
}

// If there are any explicitly specified system JS libraries to link to, add those to link.
if (SYSTEM_JS_LIBRARIES) {
libraries = libraries.concat(SYSTEM_JS_LIBRARIES);
}

libraries = libraries.concat(additionalLibraries);

// For each JS library library_xxx.js, add a preprocessor token __EMSCRIPTEN_HAS_xxx_js__ so that code can conditionally dead code eliminate out
// if a particular feature is not being linked in.
for (var i = 0; i < libraries.length; ++i) {
global['__EMSCRIPTEN_HAS_' + libraries[i].replace('.', '_').replace('library_', '') + '__'] = 1
}
libraries = libraries.concat([
'library_sdl.js',
'library_gl.js',
'library_glut.js',
'library_xlib.js',
'library_egl.js',
'library_openal.js',
'library_glfw.js',
'library_uuid.js',
'library_glew.js',
'library_html5.js',
'library_signals.js',
'library_idbstore.js',
'library_async.js',
'library_vr.js'
]).concat(additionalLibraries);

if (BOOTSTRAPPING_STRUCT_INFO) libraries = ['library_bootstrap_structInfo.js', 'library_formatString.js'];
if (ONLY_MY_CODE) {
Expand Down
20 changes: 20 additions & 0 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,10 @@ var LINKABLE = 0; // If set to 1, this file can be linked with others, either as
// LINKABLE of 0 is very useful in that we can reduce the size of the
// generated code very significantly, by removing everything not actually used.

var STRICT = 0; // Emscripten 'strict' build mode: Drop supporting any deprecated build options.
// Set the environment variable EMCC_STRICT=1 or pass -s STRICT=1
// to test that a codebase builds nicely in forward compatible manner.

var WARN_ON_UNDEFINED_SYMBOLS = 1; // If set to 1, we will warn on any undefined symbols that
// are not resolved by the library_*.js files. Note that
// it is common in large projects to
Expand All @@ -501,6 +505,22 @@ var WARN_ON_UNDEFINED_SYMBOLS = 1; // If set to 1, we will warn on any undefined
var ERROR_ON_UNDEFINED_SYMBOLS = 0; // If set to 1, we will give a compile-time error on any
// undefined symbols (see WARN_ON_UNDEFINED_SYMBOLS).

// The default value for this is currently 0, but will be
// transitioned to 1 in the future. To keep relying on
// building with -s ERROR_ON_UNDEFINED_SYMBOLS=0 setting,
// prefer to set that option explicitly in your build system.

var ERROR_ON_MISSING_LIBRARIES = 0; // If set to 1, any -lfoo directives pointing to nonexisting
// library files will issue a linker error.

// The default value for this is currently 0, but will be
// transitioned to 1 in the future. To keep relying on
// building with -s ERROR_ON_MISSING_LIBRARIES=0 setting,
// prefer to set that option explicitly in your build system.

var SYSTEM_JS_LIBRARIES = []; // Specifies a list of Emscripten-provided JS libraries to link against.
Copy link
Member

Choose a reason for hiding this comment

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

Currently we have js_libraries in emcc.py, which invokes emscripten.py with --libraries. It seems like emcc.py could use this new variable, and we could get rid of --libraries? That would avoid duplicated functionality.

Copy link
Member

Choose a reason for hiding this comment

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

Although for people using emscripten.py directly, maybe we shouldn't remove that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, that might be true, though not sure if I want to change this right now at least, there doesn't seem to be any tests for the --libraries flag in our test suite.

// (internal, use -lfoo or -lfoo.js to link to Emscripten system JS libraries)

var SMALL_XHR_CHUNKS = 0; // Use small chunk size for binary synchronous XHR's in Web Workers.
// Used for testing.
// See test_chunked_synchronous_xhr in runner.py and library.js.
Expand Down
1 change: 1 addition & 0 deletions system/include/emscripten.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#include "emscripten/emscripten.h"
Loading