[PATCH] D50940: [sanitizer] Change Mmap*NoAccess to return MMAP_FAILED (~(uptr)0) on error

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
cryptoad added reviewers: eugenis, alekseyshl, dberris.
Herald added subscribers: Sanitizers, delcypher, kubamracek.

`MmapNoAccess` & `MmapFixedNoAccess` return directly the result of
`internal_mmap`, as opposed to other Mmap functions that return `~(uptr)0` (eg:
`MMAP_FAILED`) on error (and potentially deal with the errno).

This inconsistency leads to some confusion for the callers, as some check for
`~(uptr)0` for failure (while it can fail with `-ENOMEM` for example).

Two potential solutions: change the callers, or make the functions return
`~(uptr)0` on failure to abide to the precedent set by the other functions.
The second option looked more appropriate to me.

TODO for follow up CLs:

- There are a couple of `internal_mmap` calls in XRay that check for MMAP_FAILED as a result as well (cc: @dberris); they should use `internal_iserror`;
- Some calls appear to check for a non-null pointer instead of `~(uptr)0` (the sanitizer secondary allocator for example). This should be fixed to.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D50940

Files:
  lib/sanitizer_common/sanitizer_posix_libcdep.cc


Index: lib/sanitizer_common/sanitizer_posix_libcdep.cc
===================================================================
--- lib/sanitizer_common/sanitizer_posix_libcdep.cc
+++ lib/sanitizer_common/sanitizer_posix_libcdep.cc
@@ -387,13 +387,18 @@
   unsigned flags = MAP_PRIVATE | MAP_FIXED | MAP_NORESERVE;
   if (fd == -1) flags |= MAP_ANON;
 
-  return (void *)internal_mmap((void *)fixed_addr, size, PROT_NONE, flags, fd,
-                               0);
+  uptr p = internal_mmap((void *)fixed_addr, size, PROT_NONE, flags, fd, 0);
+  if (internal_iserror(p))
+    p = ~(uptr)0;
+  return (void *)p;
 }
 
 void *MmapNoAccess(uptr size) {
   unsigned flags = MAP_PRIVATE | MAP_ANON | MAP_NORESERVE;
-  return (void *)internal_mmap(nullptr, size, PROT_NONE, flags, -1, 0);
+  uptr p = internal_mmap(nullptr, size, PROT_NONE, flags, -1, 0);
+  if (internal_iserror(p))
+    p = ~(uptr)0;
+  return (void *)p;
 }
 
 // This function is defined elsewhere if we intercepted pthread_attr_getstack.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D50940.161384.patch
Type: text/x-patch
Size: 1001 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180819/78424bde/attachment.bin>