Details
- Reviewers
kcc
Diff Detail
Event Timeline
I'd ask rnk@ to also review this.
First question: do we want any STL here? Will STL conflict with DR's native exec?
lib/msandr/msandr.cc | ||
---|---|---|
13 | This deserves a detailed comment (and/or a link) describing why we needs this. | |
29 | Can we avoid using STL? | |
44 | Can this be VERBOSITY_LEVEL, which is one of 0, 1, 2 so that you use if () instead of #ifdef ? | |
60 | fixed sized array, please. No STL | |
74 | fixed size array, no STL, please | |
117 | any reason not to make this an inline function? | |
208 | grrr. std::set is not that easy to replace with non-STL. | |
363 | __asan ? |
lib/msandr/msandr.cc | ||
---|---|---|
29 | The real question here is why does it work at all? :) | |
117 | This way it has the same name in msan and msandr, easier to grep, etc. | |
208 | 4 :) |
Do you think we need more feedback from the community about if this is an OK place for this?
I also think we should add a script or README to msandr in the initial commit to at least document how to run msandr, even if it doesn't work for everyone.
lib/msandr/CMakeLists.txt | ||
---|---|---|
4 | I filed http://code.google.com/p/dynamorio/issues/detail?id=1083 on that | |
lib/msandr/msandr.cc | ||
29 | For regular (no -native_exec) dynamic DR, this Just Works because we spent all that effort on our private loader. :) For dynamic DR with -native_exec (not ready yet anyway), I don't think we'll be able to use the private loader and the segment swapping it does. Any STL or libc call can then clobber errno, which is a big problem. With STATIC_LIBRARY, I have no idea why it works. I think you'll end up using the app's STL copy which will allocate on the app heap, probably using the msan allocator, which is bad for isolation. So, since it works with dynamic DR right now, and apparently it works somewhat with static DR, we can probably commit as-is, but we should exorcise STL stuff eventually. | |
60 | FWIW DR exports #define MAX_PATH 260, but it's a total lie on both Windows and Linux of course. XD I think we have dr_strdup() which uses DR's heap, if you want to be robust. | |
208 | Yep. | |
266 | Can just say "clobbers nothing except xax" or something. | |
284 | ditto, no clobber | |
289 | I guess msan is not 32-bit ready? I still like for loop + OPND_CREATE_MEMPTR + i * sizeof(void*) better. | |
319 | If you use_DynamoRIO_extension(drcontainers) there's a C-style resizable array called dr_vector that uses DR's heap (as well as a C-style void* hashtable, but we don't need that). DR's heap is in the lower 2 GB, so when we instrument the STL lower_bound may explode. DR has a vmvector_t data structure we've been meaning to share for some time, but it's a little too special cased. | |
417 | Hrm, tail calls through the PLT? This should probably have a comment about why we include these. |
I've added a README with full build instructions (including DR and DrM). It should be enough to get people started, provided they are on x86_64 linux.
MSanDR plays the role of a runtime support library for MSan. Unlike the rest of compiler-rt, it depends on some heavyweight external libraries. At the moment it also has a weird way of "binding" to the executable (through a shell script), but that will change soon when we start linking it into the executable.
Compiler-rt seems like the right place for such library.
lib/msandr/msandr.cc | ||
---|---|---|
13 | done | |
29 | I agree that getting rid of STL is the right thing to do, and is not that hard, but I'd prefer to do this work on a versioned file, and land in a separate commit. | |
44 | done | |
289 | Yes, we don't care about 32 bits for now. | |
363 | __msan | |
417 | Added a comment. |
I filed http://code.google.com/p/dynamorio/issues/detail?id=1083 on that