[PATCH] D23107: [MSAN][MIPS] Fix fork.cc test on MIPS

Evgeniy Stepanov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 2 12:09:33 PST 2016


eugenis added a comment.

Phabricator shows lots of affected files with empty diffs for some reason.



================
Comment at: lib/msan/msan.h:340
+    if (!SANITIZER_CAN_FAST_UNWIND)                                            \
+      GetStackTrace(&stack, 1, pc, bp,                                         \
+                    common_flags()->fast_unwind_on_malloc);                    \
----------------
max(1, flags()->store_context_size)


================
Comment at: lib/msan/msan.h:341
+      GetStackTrace(&stack, 1, pc, bp,                                         \
+                    common_flags()->fast_unwind_on_malloc);                    \
+    else                                                                       \
----------------
and this can be just "false"


================
Comment at: test/msan/chained_origin.cc:16
 // RUN:     not %run %t >%t.out 2>&1
-// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-HEAP < %t.out
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%unwind-HEAP -check-prefix=CHECK-HEAP < %t.out
 
----------------
I can't find any use of CHECK-(NON-)MIPS-FRAMES-HEAP in this test.


================
Comment at: test/msan/chained_origin_limits.cc:157
+// CHECK-NON-MIPS-FRAMES-PER-STACK: Uninitialized value was stored to memory at
+// CHECK-NON-MIPS-FRAMES-PER-STACK: in fn1
 // CHECK-PER-STACK: Uninitialized value was created
----------------
Hm, this test expects three instances of "Uninitialized value was stored to memory at" on non-mips, and only one instance on mips. Why is that?



================
Comment at: test/msan/lit.cfg:42
+# check the first frame since store_context size is 1.
+if config.host_arch in ['mips64', 'mips64el']:
+  config.substitutions.append( ('CHECK-%unwind', ("CHECK-MIPS-FRAMES")))
----------------
fast unwinder is also unavailable on sparc, please include that here


================
Comment at: test/msan/lit.cfg:43
+if config.host_arch in ['mips64', 'mips64el']:
+  config.substitutions.append( ('CHECK-%unwind', ("CHECK-MIPS-FRAMES")))
+else:
----------------
i don't think these braces around "CHECK-MIPS-FRAMES" are necessary


================
Comment at: test/msan/lit.cfg:45
+else:
+  config.substitutions.append( ('CHECK-%unwind', ("CHECK-NON-MIPS-FRAMES")))
----------------
Instead of mips and non-mips, let's call it something neutral, like
%short-stack -> "SHORT-STACK" on mips and sparc, and "" on other platforms.
Then any affected CHECK can be given a -%short-stack suffix.


Repository:
  rL LLVM

https://reviews.llvm.org/D23107





More information about the llvm-commits mailing list