Switch from addAtMain to addAtInit in library_fs.js (#14306)

sbc100 · web-flow · commit a56817a698e5 · 2021-05-26T18:40:09.000-07:00
Using addAtMain stopped working for programs without main functions (HAS_MAIN not defined) after #13901. Code that needs to populate the FS is mostly likely going to be run during preRun callback which all happen before ATINITs.
diff --git a/src/library_fs.js b/src/library_fs.js
@@ -27,13 +27,17 @@ mergeInto(LibraryManager.library, {
     ],
   $FS__postset: function() {
     // TODO: do we need noFSInit?
-    addAtInit('if (!Module["noFSInit"] && !FS.init.initialized) FS.init();');
-    addAtMain('FS.ignorePermissions = false;');
+    addAtInit(`
+if (!Module["noFSInit"] && !FS.init.initialized)
+  FS.init();
+FS.ignorePermissions = false;
+`)
     addAtExit('FS.quit();');
     // We must statically create FS.FSNode here so that it is created in a manner
     // that is visible to Closure compiler. That lets us use type annotations for
     // Closure to the "this" pointer in various node creation functions.
-    return `var FSNode = /** @constructor */ function(parent, name, mode, rdev) {
+    return `
+var FSNode = /** @constructor */ function(parent, name, mode, rdev) {
   if (!parent) {
     parent = this;  // root node sets parent to itself
   }
@@ -97,8 +101,8 @@ FS.staticInit();` +
     initialized: false,
     // Whether we are currently ignoring permissions. Useful when preparing the
     // filesystem and creating files inside read-only folders.
-    // This is set to false when the runtime is initialized, allowing you
-    // to modify the filesystem freely before run() is called.
+    // This is set to false during `preInit`, allowing you to modify the
+    // filesystem freely up until that point (e.g. during `preRun`).
     ignorePermissions: true,
     trackingDelegate: {},
     tracking: {
diff --git a/src/parseTools.js b/src/parseTools.js
@@ -1078,6 +1078,8 @@ function addAtInit(code) {
   ATINITS.push(code);
 }
 
+// TODO(sbc): There are no more uses to ATMAINS or addAtMain in emscripten.
+// We should look into removing these.
 global.ATMAINS = [];
 
 function addAtMain(code) {
diff --git a/tests/test_core.py b/tests/test_core.py
@@ -5328,6 +5328,27 @@ def test_fs_mmap(self):
         self.emcc_args += ['-lnodefs.js', '-lnoderawfs.js']
       self.do_run_in_out_file_test('fs/test_mmap.c')
 
+  @parameterized({
+    '': [],
+    'minimal_runtime': ['-s', 'MINIMAL_RUNTIME=1']
+  })
+  def test_fs_no_main(self, *args):
+    # library_fs.js uses hooks to enable ignoreing of permisions up until ATMAINs are run.  This
+    # test verified that they work correctly, even in programs without a main function.
+    create_file('pre.js', '''
+Module['preRun'] = function() {
+  assert(FS.ignorePermissions, "ignorePermissions not set during preRun");
+}
+Module['onRuntimeInitialized'] = function() {
+  assert(!FS.ignorePermissions, "ignorePermissions not unset during onRuntimeInitialized");
+  assert(_foo() == 42);
+}
+''')
+    self.set_setting('EXPORTED_FUNCTIONS', '_foo')
+    self.set_setting('FORCE_FILESYSTEM')
+    self.emcc_args += ['--pre-js', 'pre.js'] + list(args)
+    self.do_run('int foo() { return 42; }', '', force_c=True)
+
   @also_with_noderawfs
   def test_fs_errorstack(self):
     # Enables strict mode, which may catch some strict-mode-only errors

Original file line number	Diff line number	Diff line change
`@@ -1078,6 +1078,8 @@ function addAtInit(code) {`
`1078`	`1078`	`ATINITS.push(code);`
`1079`	`1079`	`}`
`1080`	`1080`
	`1081`	`+// TODO(sbc): There are no more uses to ATMAINS or addAtMain in emscripten.`
	`1082`	`+// We should look into removing these.`
`1081`	`1083`	`global.ATMAINS = [];`
`1082`	`1084`
`1083`	`1085`	`function addAtMain(code) {`