[fuchsia] Add loop to _zx_cprng_draw.

[flowerhack]

Summary

The _zx_cprng_draw API expects to be looped rather than simply
called. Update the code to reflect this.

[mcgrathr]

LGTM

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

494 ↗

(On Diff #118465)

Pointer arithmetic on void* is a GNU extension, but fine if it's allowed in this code.

[cryptoad]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

490 ↗

(On Diff #118465)

length being a uptr, the condition comes down to while length != 0. I assume size is guaranteed to not be > length?

[flowerhack]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

490 ↗

(On Diff #118465)

that's correct; size will never be > length

494 ↗

(On Diff #118465)

interesting! how would i know whether a particular extension is allowed in a part of compiler-rt? i couldn't find anything about it in the LLVM coding standards

[mcgrathr]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

494 ↗

(On Diff #118465)

If it compiles then they haven't chosen -std=... options to enforce it, which means either they think it's fine or they don't really care.
As to the intent, the guys on the team will have to speak up.

[flowerhack]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

494 ↗

(On Diff #118465)

sgtm

[flowerhack]

added a couple reviewers with commit bits

please feel free to hit the "ship it" button on my behalf or, if that should not happen, let me know what to fix!

[phosek]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

494 ↗

(On Diff #118465)

Make sure you try to compile this code with Clang as your host compiler.

[phosek]

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cc

494 ↗

(On Diff #118465)

I tried build our toolchain with this patch (which uses Clang as a first stage compiler) and I'm getting an error on this line: error: arithmetic on a pointer to void

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 uptr GetRSS();
 void NoHugePagesInRegion(uptr addr, uptr length);
 void DontDumpShadowMemory(uptr addr, uptr length);
 // Check if the built VMA size matches the runtime one.
 void CheckVMASize();
 void RunMallocHooks(const void *ptr, uptr size);
 void RunFreeHooks(const void *ptr);
 
+class ReservedAddressRange {
+ public:
+  uptr Init(uptr size, const char *name = nullptr, uptr fixed_addr = 0);
+  uptr Map(uptr fixed_addr, uptr size, bool tolerate_enomem = false);
+  void Unmap(uptr addr, uptr size);
+  const void *base() { return base_; }
+  const uptr size() { return size_; }
+
+ private:
+  void* base_;
+  uptr size_;
+  const char* name_;
+};
+
 typedef void (*fill_profile_f)(uptr start, uptr rss, bool file,
                                /*out*/uptr *stats, uptr stats_size);
 
 // Parse the contents of /proc/self/smaps and generate a memory profile.
 // |cb| is a tool-specific callback that fills the |stats| array containing
 // |stats_size| elements.
 void GetMemoryProfile(fill_profile_f cb, uptr *stats, uptr stats_size);
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc

 void *MmapNoReserveOrDie(uptr size, const char *mem_type) {
   return MmapOrDie(size, mem_type);
 }
 
 void *MmapOrDieOnFatalError(uptr size, const char *mem_type) {
   return DoAnonymousMmapOrDie(size, mem_type, false, false);
 }
 
+uptr ReservedAddressRange::Init(uptr init_size, const char* name = nullptr,
+                                uptr fixed_addr = uptr(0)) {
+  base_ = MmapNoAccess(init_size);
+  size_ = size;
+  name_ = name;
+  return reinterpret_cast<uptr>(base_);
+}
+
+// Uses fixed_addr for now.
+// Will use offset instead once we've implemented this function for real.
+uptr ReservedAddressRange::Map(uptr fixed_addr, uptr map_size,
+                               bool tolerate_enomem = true) {
+  return reinterpret_cast<uptr>(MmapFixedOrDie(fixed_addr, size,
+                                               tolerate_enomem));
+}
+
+void ReservedAddressRange::Unmap(uptr addr, uptr size) {
+  void* addr_as_void = reinterpret_cast<void*>(addr);
+  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
+  // Only unmap at the beginning or end of the range.
+  CHECK_EQ((addr_as_void == base_) || (addr + size == base_as_uptr + size_),
+           true);
+  // Detect overflows.
+  CHECK_LE(size, (base_as_uptr + size_) - addr);
+  UnmapOrDie(reinterpret_cast<void*>(addr), size);
+  if (addr_as_void == base_) {
+    base_ = reinterpret_cast<void*>(addr + size);
+  }
+  size_ = size_ - size;
+}
+
 // MmapNoAccess and MmapFixedOrDie are used only by sanitizer_allocator.
 // Instead of doing exactly what they say, we make MmapNoAccess actually
 // just allocate a VMAR to reserve the address space.  Then MmapFixedOrDie
 // uses that VMAR instead of the root.
 
 zx_handle_t allocator_vmar = ZX_HANDLE_INVALID;
 uintptr_t allocator_vmar_base;
 size_t allocator_vmar_size;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc

void *MmapFixedNoReserve(uptr fixed_addr, uptr size, const char *name) {
   if (internal_iserror(p, &reserrno))
     Report("ERROR: %s failed to "
            "allocate 0x%zx (%zd) bytes at address %zx (errno: %d)\n",
            SanitizerToolName, size, size, fixed_addr, reserrno);
   IncreaseTotalMmap(size);
   return (void *)p;
 }
 
+uptr ReservedAddressRange::Init(uptr size, const char *name, uptr fixed_addr) {
+  if (fixed_addr) {
+    base_ = MmapFixedNoAccess(fixed_addr, size, name);
+  } else {
+    base_ = MmapNoAccess(size);
+  }
+  size_ = size;
+  name_ = name;
+  return reinterpret_cast<uptr>(base_);
+}
+
+// Uses fixed_addr for now.
+// Will use offset instead once we've implemented this function for real.
+uptr ReservedAddressRange::Map(uptr fixed_addr, uptr size,
+                               bool tolerate_enomem) {
+  if (tolerate_enomem) {
+    return reinterpret_cast<uptr>(MmapFixedOrDieOnFatalError(fixed_addr, size));
+  }
+  return reinterpret_cast<uptr>(MmapFixedOrDie(fixed_addr, size));
+}
+
+void ReservedAddressRange::Unmap(uptr addr, uptr size) {
+  void* addr_as_void = reinterpret_cast<void*>(addr);
+  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
+  // Only unmap at the beginning or end of the range.
+  CHECK_EQ((addr_as_void == base_) || (addr + size == base_as_uptr + size_),
+           true);
+  // Detect overflows.
+  CHECK_LE(size, (base_as_uptr + size_) - addr);
+  UnmapOrDie(reinterpret_cast<void*>(addr), size);
+  if (addr_as_void == base_) {
+    base_ = reinterpret_cast<void*>(addr + size);
+  }
+  size_ = size_ - size;
+}
+
 void *MmapFixedNoAccess(uptr fixed_addr, uptr size, const char *name) {
   int fd = name ? GetNamedMappingFd(name, size) : -1;
   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);
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

if (p == 0) {
     char mem_type[30];
     internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
                       fixed_addr);
     ReportMmapFailureAndDie(size, mem_type, "allocate", GetLastError());
   }
   return p;
 }
 
+// Uses fixed_addr for now.
+// Will use offset instead once we've implemented this function for real.
+uptr ReservedAddressRange::Map(uptr fixed_addr, uptr size,
+                               bool tolerate_enomem) {
+  if (tolerate_enomem) {
+    return reinterpret_cast<uptr>(MmapFixedOrDieOnFatalError(fixed_addr, size));
+  }
+  return reinterpret_cast<uptr>(MmapFixedOrDie(uptr fixed_addr, uptr size));
+}
+
+void ReservedAddressRange::Unmap(uptr addr, uptr size) {
+  void* addr_as_void = reinterpret_cast<void*>(addr);
+  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
+  // Only unmap at the beginning or end of the range.
+  CHECK_EQ((addr_as_void == base_) || (addr + size == base_as_uptr + size_),
+           true);
+  // Detect overflows.
+  CHECK_LE(size, (base_as_uptr + size_) - addr);
+  UnmapOrDie(reinterpret_cast<void*>(addr), size);
+  if (addr_as_void == base_) {
+    base_ = reinterpret_cast<void*>(addr + size);
+  }
+  size_ = size_ - size;
+}
+
 void *MmapFixedOrDieOnFatalError(uptr fixed_addr, uptr size) {
   void *p = VirtualAlloc((LPVOID)fixed_addr, size,
       MEM_COMMIT, PAGE_READWRITE);
   if (p == 0) {
     char mem_type[30];
     internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
                       fixed_addr);
     return ReturnNullptrOnOOMOrDie(size, mem_type, "allocate");
   }
   return p;
 }
 
 void *MmapNoReserveOrDie(uptr size, const char *mem_type) {
   // FIXME: make this really NoReserve?
   return MmapOrDie(size, mem_type);
 }
 
+uptr ReservedAddressRange::Init(uptr size, const char *name, uptr fixed_addr) {
+  if (fixed_addr) {
+    base_ = MmapFixedNoAccess(fixed_addr, size, name);
+  } else {
+    base_ = MmapNoAccess(size);
+  }
+  size_ = size;
+  name_ = name;
+  return reinterpret_cast<uptr>(base_);
+}
+
+
 void *MmapFixedNoAccess(uptr fixed_addr, uptr size, const char *name) {
   (void)name; // unsupported
   void *res = VirtualAlloc((LPVOID)fixed_addr, size,
                            MEM_RESERVE, PAGE_NOACCESS);
   if (res == 0)
     Report("WARNING: %s failed to "
            "mprotect %p (%zd) bytes at %p (error code: %d)\n",
            SanitizerToolName, size, size, fixed_addr, GetLastError());

compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_common_test.cc

for (uptr size = 4; size <= ARRAY_SIZE(buffer_1); size += 4) {
         EXPECT_TRUE(GetRandom(buffer_2, size, blocking));
         EXPECT_NE(internal_memcmp(buffer_1, buffer_2, size), 0);
       }
     }
   }
 }
 #endif
 
+TEST(SanitizerCommon, ReservedAddressRangeInit) {
+  uptr init_size = 0xffff;
+  ReservedAddressRange address_range;
+  uptr res = address_range.Init(init_size);
+  CHECK_NE(res, (void*)-1);
+  UnmapOrDie((void*)res, init_size);
+  // Should be able to map into the same space now.
+  ReservedAddressRange address_range2;
+  uptr res2 = address_range2.Init(init_size, nullptr, res);
+  CHECK_EQ(res, res2);
+
+  // TODO(flowerhack): Once this is switched to the "real" implementation
+  // (rather than passing through to MmapNoAccess*), enforce and test "no
+  // double initializations allowed"
+}
+
+TEST(SanitizerCommon, ReservedAddressRangeMap) {
+  constexpr uptr init_size = 0xffff;
+  ReservedAddressRange address_range;
+  uptr res = address_range.Init(init_size);
+  CHECK_NE(res, (void*) -1);
+
+  // Valid mappings should succeed.
+  CHECK_EQ(res, address_range.Map(res, init_size));
+
+  // Valid mappings should be readable.
+  unsigned char buffer[init_size];
+  memcpy(buffer, &res, sizeof(buffer));
+
+  // Invalid mappings should fail.
+  EXPECT_DEATH(address_range.Map(res, 0), ".*");
+
+  // TODO(flowerhack): Once this is switched to the "real" implementation, make
+  // sure you can only mmap into offsets in the Init range.
+}
+
+TEST(SanitizerCommon, ReservedAddressRangeUnmap) {
+  uptr PageSize = GetPageSizeCached();
+  uptr init_size = PageSize * 4;
+  ReservedAddressRange address_range;
+  uptr base_addr = address_range.Init(init_size);
+  CHECK_NE(base_addr, (void*)-1);
+  CHECK_EQ(base_addr, address_range.Map(base_addr, init_size));
+
+  // Unmapping at the beginning should succeed.
+  address_range.Unmap(base_addr, PageSize);
+  CHECK_EQ(base_addr + PageSize, address_range.base());
+  CHECK_EQ(init_size - PageSize, address_range.size());
+
+  // Unmapping at the end should succeed.
+  uptr old_size = address_range.size();
+  void* old_base = address_range.base();
+  uptr new_start = reinterpret_cast<uptr>(address_range.base()) +
+                   address_range.size() - PageSize;
+  address_range.Unmap(new_start, PageSize);
+  CHECK_EQ(old_size - PageSize, address_range.size());
+  CHECK_EQ(old_base, address_range.base());
+
+  // Unmapping in the middle of the ReservedAddressRange should fail.
+  EXPECT_DEATH(address_range.Unmap(base_addr + 0xf, 0xff), ".*");
+}
+
 }  // namespace __sanitizer