Fix return value of _mmap_js (#17853)

_mmap_js previously returned an inptr_t that was either a pointer on success or
a negative error code on failure. However, all negative results were interpreted
as failure, so successfully allocated memory in the second half of memory would
have been incorrectly freed before being returned to the user. To fix this bug,
have _mmap_js return 0 on success and write the returned pointer in an
out-param.

Author: tlively

src/library_syscall.js

@@ -121,13 +121,13 @@ var SyscallsLibrary = {
 #endif // SYSCALLS_REQUIRE_FILESYSTEM
   },
 
-  _mmap_js__sig: 'ppiiipp',
+  _mmap_js__sig: 'ipiiippp',
   _mmap_js__deps: ['$SYSCALLS',
 #if FILESYSTEM && SYSCALLS_REQUIRE_FILESYSTEM
     '$FS',
 #endif
   ],
-  _mmap_js: function(len, prot, flags, fd, off, allocated) {
+  _mmap_js: function(len, prot, flags, fd, off, allocated, addr) {
 #if FILESYSTEM && SYSCALLS_REQUIRE_FILESYSTEM
     var stream = SYSCALLS.getStreamFromFD(fd);
     var res = FS.mmap(stream, len, off, prot, flags);
@@ -136,7 +136,8 @@ var SyscallsLibrary = {
 #if CAN_ADDRESS_2GB
     ptr >>>= 0;
 #endif
-    return ptr;
+    {{{ makeSetValue('addr', 0, 'ptr', '*') }}};
+    return 0;
 #else // no filesystem support; report lack of support
     return -{{{ cDefine('ENOSYS') }}};
 #endif

system/lib/libc/emscripten_mmap.c

@@ -33,7 +33,13 @@ static volatile int lock[1];
 static struct map* mappings;
 
 // JS library functions.  Used only when mapping files (not MAP_ANONYMOUS)
-intptr_t _mmap_js(size_t length, int prot, int flags, int fd, size_t offset, int* allocated);
+int _mmap_js(size_t length,
+             int prot,
+             int flags,
+             int fd,
+             size_t offset,
+             int* allocated,
+             void** addr);
 int _munmap_js(intptr_t addr, size_t length, int prot, int flags, int fd, size_t offset);
 int _msync_js(intptr_t addr, size_t length, int flags, int fd);
 
@@ -132,12 +138,12 @@ intptr_t __syscall_mmap2(intptr_t addr, size_t len, int prot, int flags, int fd,
     new_map->allocated = true;
   } else {
     new_map = emscripten_builtin_malloc(sizeof(struct map));
-    long rtn = _mmap_js(len, prot, flags, fd, off, &new_map->allocated);
+    int rtn =
+      _mmap_js(len, prot, flags, fd, off, &new_map->allocated, &new_map->addr);
     if (rtn < 0) {
       emscripten_builtin_free(new_map);
       return rtn;
     }
-    new_map->addr = (void*)rtn;
     new_map->fd = fd;
   }
 

system/lib/standalone/standalone.c

@@ -66,8 +66,13 @@ int clock_getres(clockid_t clk_id, struct timespec *tp) {
 
 // Mark these as weak so that wasmfs does not collide with it. That is, if
 // wasmfs is in use, we want to use that and not this.
-__attribute__((__weak__)) intptr_t _mmap_js(
-  size_t length, int prot, int flags, int fd, size_t offset, int* allocated) {
+__attribute__((__weak__)) int _mmap_js(size_t length,
+                                       int prot,
+                                       int flags,
+                                       int fd,
+                                       size_t offset,
+                                       int* allocated,
+                                       void** addr) {
   return -ENOSYS;
 }
 

system/lib/wasmfs/syscalls.cpp

@@ -1527,8 +1527,13 @@ int __syscall_fstatfs64(int fd, size_t size, intptr_t buf) {
   return doStatFS(openFile->locked().getFile(), size, (struct statfs*)buf);
 }
 
-intptr_t _mmap_js(
-  size_t length, int prot, int flags, int fd, size_t offset, int* allocated) {
+int _mmap_js(size_t length,
+             int prot,
+             int flags,
+             int fd,
+             size_t offset,
+             int* allocated,
+             void** addr) {
   auto openFile = wasmFS.getFileTable().locked().getEntry(fd);
   if (!openFile) {
     return -EBADF;
@@ -1585,14 +1590,15 @@ intptr_t _mmap_js(
   // From here on, we have succeeded, and can mark the allocation as having
   // occurred (which means that the caller has the responsibility to free it).
   *allocated = true;
+  *addr = (void*)ptr;
 
   // The read must be of a valid amount, or we have had an internal logic error.
   assert(nread <= length);
 
   // mmap clears any extra bytes after the data itself.
   memset(ptr + nread, 0, length - nread);
 
-  return (intptr_t)ptr;
+  return 0;
 }
 
 // Stubs (at least for now)