[PATCH] D54080: [unittest] Skip W+X MappedMemoryTests when MPROTECT is enabled

Kamil Rytarowski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 3 14:38:32 PDT 2018


krytarowski added inline comments.


================
Comment at: unittests/Support/MemoryTest.cpp:18
+#include <sys/types.h>
+#include <sys/param.h>
+#include <sys/sysctl.h>
----------------
`sys/param.h` before `sys/types.h` according to the NetBSD style.

Please decorate this include section with
`// clang-format off`
and
`// clang-format on`


================
Comment at: unittests/Support/MemoryTest.cpp:38
+
+  assert(sysctl(mib, 3, &paxflags, &len, NULL, 0) == 0);
+
----------------
Using `assert()` might have a negative impact on functionality... because under some conditions this code might not be compiled at all and `sysctl(3)` won't be called.

I propose to go for:

```
if (sysctl(mib, 3, &paxflags, &len, NULL, 0) != 0)
  err(EXIT_FAILURE, "sysctl");
```

(and include `<err.h>`)


================
Comment at: unittests/Support/MemoryTest.cpp:91
+      return; \
+  } while (0); \
+
----------------
I would drop `: \` here. Some compilers will complain about redundant `;`.


================
Comment at: unittests/Support/MemoryTest.cpp:223
+  // to block any variant with X.
+  if ((Flags && Memory::MF_EXEC) && IsMPROTECT())
+    return;
----------------
`Flags & Memory::MF_EXEC`?


Repository:
  rL LLVM

https://reviews.llvm.org/D54080





More information about the llvm-commits mailing list