Details
- Reviewers
cryptoad pcc hctim - Commits
- rG07c92b2e9581: [scudo] Add memtag_test
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
---|---|---|
50 | Should we restore to the previous MTE mode (SYNC/ASYNC) instead of just turning it to SYNC? | |
100 | It wasn't obvious to me on first glance why *P was a deterministic fault. Might be worth adding a comment to selectRandomTag that tag zero is excluded from when the prctl was issued. | |
105 | Wouldn't we want to die if the prctl failed? | |
152 | Untag after test? | |
173 | Implicit invariant that 0x100 >= 4 * archMemoryTagGranuleSize(). Maybe make it explicit? uptr WorkingRegionSize = 4 * archMemoryTagGranuleSize(); for (uptr Size = 0; Size < WorkingRegionSize; ++Size, P += WorkingRegionSize) { |
compiler-rt/lib/scudo/standalone/memtag.h | ||
---|---|---|
176 | This isn't necessary, the caller should do that themselves. | |
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
51 | We normally handle this by skipping the tests if the system isn't detecting tag faults. Can we set the tagging mode in the main function? That way, both these tests and the combined allocator tests should run. | |
103 | We have ScopedDisableMemoryTagChecks for this purpose. |
compiler-rt/lib/scudo/standalone/memtag.h | ||
---|---|---|
176 | DCHECK? | |
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
50 | going with main() function, so nothing to restore | |
100 | And removing 0 from prctl does not look nice. | |
103 | That one uses TSO, here I tests prctl. | |
152 | ::TearDown munpap entire buffer |
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
---|---|---|
103 | The {disable,enable}MemoryTagChecksTestOnly functions use TCO, same as ScopedDisableMemoryTagChecks. There's a use of ScopedDisableMemoryTagChecks in combined.h and we could probably start using ScopedDisableMemoryTagChecks in combined_test.cpp which would let us remove {disable,enable}MemoryTagChecksTestOnly. |
This is causing failures https://lab.llvm.org/buildbot/#/builders/57/builds/7002/steps/6/logs/stdio:
/home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1527:11: error: comparison of integers of different signs: 'const int' and 'const unsigned long' [-Werror,-Wsign-compare] if (lhs == rhs) { ~~~ ^ ~~~ /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1554:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned long>' requested here return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs); ^ /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp:77:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<int, unsigned long, nullptr>' requested here EXPECT_EQ(0xffff, Tags); ^ /home/buildbots/docker-RHEL-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2028:54: note: expanded from macro 'EXPECT_EQ' EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2) ^ 1 error generated.
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
---|---|---|
15 | Why do we need this? The tests should all fail to detect MTE on non-Linux platforms, shouldn't they? |
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp | ||
---|---|---|
15 | it uses GTEST stuff which is likely not available on zxtest of Fucsia |
This isn't necessary, the caller should do that themselves.