[PATCH] D67929: builtins test: Move clear_cache_test.c from a mprotect()ed global to a mmap()ed variable

[Dan Liew via Phabricator via llvm-commits]

delcypher requested changes to this revision.
delcypher added inline comments.
This revision now requires changes to proceed.


================
Comment at: compiler-rt/test/builtins/Unit/clear_cache_test.c:57
+#if !defined(_WIN32)
+    int *execution_buffer = mmap(0, 128, PROT_READ | PROT_WRITE | PROT_EXEC,
+                                 MAP_ANON | MAP_PRIVATE, 0, 0);
----------------
I don't like repetition of the `128` constant. We could define a global const (e.g. `kExecutionBufferSize`) and use it here.
However, we aren't using `page_size` anymore here and arguably we could use it here instead of `128` (which seems like a really arbitrary size). However, `mmap` has to round up to the page size anyway so maybe dropping `page_size` and the code that computes it is the cleanest thing to do?


================
Comment at: compiler-rt/test/builtins/Unit/clear_cache_test.c:76
     char* start = (char*)((uintptr_t)execution_buffer & (-page_size));
     char* end = (char*)((uintptr_t)(&execution_buffer[128+page_size]) & (-page_size));
 
----------------
`start`, and `end` aren't being used anymore. We should just remove them.

If we decide not to use `page_size` we should remove it too (and the code that computes `page_size).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67929/new/

https://reviews.llvm.org/D67929