[PATCH] D12790: ubsan: Implement memory permission validation for vtables.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: lib/ubsan/ubsan_type_hash_itanium.cc:199
@@ +198,3 @@
+
+#if SANITIZER_LINUX
+  // Validate the memory permissions of the vtable pointer and the first
----------------
kcc wrote:
> can this be if (SANITIZER_LINUX)? 
I don't think it can. The code may create references to undefined symbols on (e.g.) Windows.

================
Comment at: test/ubsan/TestCases/Linux/TypeCheck/vptr-bad-perms.cpp:2
@@ +1,3 @@
+// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr -g %s -O3 -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
----------------
kcc wrote:
> add an explanation comment to the test 
Done

================
Comment at: test/ubsan/TestCases/Linux/lit.local.cfg:1
@@ +1,2 @@
+def getRoot(config):
+  if not config.parent:
----------------
kcc wrote:
> I wonder if we can use something like 
>   REQUIRES: linux
> instead of putting this test into a separate dir. 
> We don't have this exact requirement now, but quite a few others. 
Introduced `REQUIRES: vptr-validation`.


http://reviews.llvm.org/D12790