Page MenuHomePhabricator

[sanitizer] Decorate /proc/self/maps.
ClosedPublic

Authored by eugenis on May 26 2015, 2:14 PM.

Details

Summary

Add descriptive names to sanitizer entries in /proc/self/maps. Helps debugging.
This is done by creating a named shared memory region, unlinking it and setting up a private (i.e. copy-on-write) mapping of that instead of a regular anonymous mapping. I've experimented with regular (sparse) files, but they can not be scaled to the size of MSan shadow mapping, at least on Linux/X86_64 and ext3 fs.

Controlled by a common flag, decorate_proc_maps, disabled by default.

This patch has a few shortcomings:

  • not all mappings are annotated, especially in TSan.
  • out handling of memset() of shadow via mmap() puts small anonymous mappings inside larger named mappings, which looks ugly and can, in theory, hit the mapping number limit.

Diff Detail

Event Timeline

eugenis updated this revision to Diff 26535.May 26 2015, 2:14 PM
eugenis retitled this revision from to [sanitizer] Decorate /proc/self/maps..
eugenis updated this object.
eugenis edited the test plan for this revision. (Show Details)
eugenis added reviewers: kcc, samsonov, dvyukov.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: Unknown Object (MLST).
samsonov edited edge metadata.May 26 2015, 4:43 PM

Nice.

lib/sanitizer_common/sanitizer_posix.cc
160
CHECK_EQ(0, res);
lib/tsan/rtl/tsan_rtl.cc
71

Is it ok to have ~6000 named mappings?

test/msan/decorate_proc_maps.cc
1 ↗(On Diff #26535)

I'd really prefer to have the test under test/sanitizer_common. If needed, we may introduce CHECK-%SAN substitution to combine CHECK-ASAN:, CHECK-MSAN: and CHECK-TSAN: in the same file.

eugenis updated this revision to Diff 26553.May 26 2015, 4:57 PM
eugenis edited edge metadata.
eugenis removed rL LLVM as the repository for this revision.
eugenis added inline comments.
lib/sanitizer_common/sanitizer_posix.cc
160

done

lib/tsan/rtl/tsan_rtl.cc
71

6000 could be ok. We already run the risk of having that many _disjoint_ mappings, because TSan leaves growth room at the end of each one. We can fix it when it happens.

test/msan/decorate_proc_maps.cc
1 ↗(On Diff #26535)

And CHECK-MSAN-ORIGINS:, too?
Ideally, we should allow any CHECK-XXX to be split into CHECK-XXX-%SAN. I don't think it is possible with lit.

samsonov added inline comments.May 26 2015, 5:16 PM
test/msan/decorate_proc_maps.cc
1 ↗(On Diff #26535)

Yeah, origins mode is a problem... As for CHECK-XXX, you can pass several --check-prefix flags to FileCheck, so that FileCheck --check-prefix=CHECK-XXX --check-prefix=CHECK-%san-XXX would check for both common and sanitizer-specific parts.

test/utils/utils.h
1 ↗(On Diff #26535)

either indicate that it's linux/posix specific in the file name, or guard all platform-specific code with ifdefs.

eugenis updated this revision to Diff 26650.May 27 2015, 5:19 PM

I've had to move the two memory mapping functions to a _libcdep header. Shm_open is not a syscall, and it's implementation in libc includes a lot of platform-specific details that we would not want to replicate in compiler-rt (like searching for a tmpfs/shmfs mount point in mtab).

I've moved the test to sanitizer_common and lost the msan-origin checks - they are not very interesting. In the future it could be nice to add msan-origins as yet another mode we run sanitizer_common tests in.

samsonov accepted this revision.May 29 2015, 3:17 PM
samsonov edited edge metadata.

LGTM

lib/tsan/go/buildgo.sh
53

Add -lrt here as well

67

And here

This revision is now accepted and ready to land.May 29 2015, 3:17 PM
eugenis added inline comments.May 29 2015, 3:33 PM
lib/tsan/go/buildgo.sh
53

I'm not sure they have an -lrt over there.

eugenis closed this revision.May 29 2015, 3:35 PM

Committed as r238621. Thanks!