cross-tool-compatible annotations

[derekbruening]

From bruen...@google.com on February 01, 2011 11:05:18

Timur suggested creating some kind of combined annotation handling
so app devs don't have to use 3 different annotations between Memcheck,
Dr. Memory, and ThreadSanitizer.

Dr. Memory does not yet have any so we can design it to be
compatible with the others. xref issue #61 on the
AmIRunningUnderDrMemory() annotation.

memcheck uses certain instr sequences.
tsan uses empty func calls, which is what I would lean toward as well.
users don't want to put in 3 different annotations for 3 different tools:
to cover both, maybe have func call that includes memcheck sequence?
though perf hit of call for memcheck: prob not a big deal, not
often in hot code

memcheck's anotations:

• am I under valgrind
• cache flush of mem region (V8)
• is this mem region defined
  used for earlier check of memory that may be sent over network
  much later, e.g., or when add false pos suppression can put
  this prior to passing to complex code (e.g., zlib) and next one (mark
  def) when data comes back.
• mark this region defined
  to fix false pos instead of changing code

Original issue: http://code.google.com/p/drmemory/issues/detail?id=283

[derekbruening]

From timurrrr@google.com on May 20, 2011 02:05:46

We should also add the "mark this region reachable / intentionally leaky" annotation
(see https://code.google.com/p/chromium/issues/detail?id=83345 )

[derekbruening]

From bruen...@google.com on August 05, 2011 13:38:38

also from issue #544 :
We need to either change how they're killed, or add an annotation ( issue #283 )
to send a nudge requesting a summary + leak scan right before they are killed: simplest to have the nudge also kill to avoid needing bi-directional communication.

[derekbruening]

From bruen...@google.com on August 29, 2011 13:14:24

this Issue covers supporting at the source level the Valgrind and Memcheck annotations: I don't think it's worth it to support them at the binary level

splitting out the annotation implementation to issue #572 and the list of annotations to provide to issue #573

[derekbruening]

From rnk@google.com on December 06, 2011 08:31:02

I fixed http://crbug.com/106326 during my sheriff rotation by adding a memcheck annotation, and now we have lots of uninit reports on our new DrMemory full bot: http://build.chromium.org/p/chromium.fyi/builders/Windows%20Tests%20%28DrMemory%20full%29/builds/0/steps/memory%20test%3A%20base/logs/stdio I'm going to spend a bit of time right now to look at the magic valgrind instruction sequences and see if it would be easy to handle this particular annotation.

Owner: rnk@google.com
Labels: Hotlist-Chrome

[derekbruening]

From timurrrr@google.com on December 06, 2011 08:35:28

I'd vote for source-compat, not binary-compat annotations.
Please have a look at TSan annotations, we use them fine on all the platforms.

[derekbruening]

From rnk@google.com on December 06, 2011 10:26:35

Where can I find more/better documentation on how the TSan annotations work? The layers of macro definitions are difficult to decipher, while reversing the asm for this one particular valgrind client request looks easy. It's also sort of easier for us to intercept special instruction sequences than it is to intercept calls. Those require symbol information. Looking at dynamic_annotations.h it looks like all the symbols are weak, which means they are not exported, so private symbols/debug information are required. Is that really what we want?

[derekbruening]

From timurrrr@google.com on December 07, 2011 00:19:11

Where can I find more/better documentation on how the TSan annotations work?
We don't have documentation for that :)
The annotations are just empty functions with a predefined signature; we just intercept them.
If you want to re-use the Valgrind annotations, you can call them in the annotation function bodies

The layers of macro definitions are difficult to decipher,
The main reason for these macro layers and weak symbols is that we ended up having macros in many projects built into the same binary (e.g. Chrome, v8, third_party/tcmalloc). We add unique prefixes to the macro names to avoid collisions.
I think the "weak" is not very necessary.

while reversing the asm for this one particular valgrind client request looks easy.
But then you have to either support Valgrind asm or put two asm sequences at a time.

It's also sort of easier for us to intercept special instruction sequences than it is to intercept calls.
Those require symbol information.
This isn't a problem for Valgrind.
Do you mean "require symbols information" makes things slower or you've meant that usually we don't need symbol info?

[derekbruening]

From timurrrr@google.com on December 07, 2011 00:22:38

Look at the pre-many-macro-layers versions of dynamic_annotations: https://code.google.com/p/data-race-test/source/browse/trunk/dynamic_annotations/dynamic_annotations.h?spec=svn2255&r=2058 https://code.google.com/p/data-race-test/source/browse/trunk/dynamic_annotations/dynamic_annotations.c?spec=svn2255&r=2058 This is pretty much the annotations used in Chrome.

[derekbruening]

From rnk@google.com on December 07, 2011 06:47:11

Right now we only need debug info to find statically linked heap routines so we can wrap them (and also to find str* wchar_t* overloads, but that's a corner case). I was thinking it would be nice to not have annotations depend on having debug info. In the context of Chrome, we really need PDB files in order to find those allocators at all, so making annotations depend on debug info isn't really a big deal. For a normal Linux application that doesn't link statically to libc, malloc is a libc export, and we only need debug info to provide symbolized traces. It would be nice if I could put annotations in a binary and ship it without debug info and have it work as intended.

I don't really find the above argument particularly compelling, because anyone trying to use the tool seriously is going to want a special build like Chrome's to disable optimizations that introduce false positives and produce debug info.

The argument that I do find compelling for just using Valgrind's annotations is that they already exist in Chrome, V8, and other apps, I've already written the code to pattern match them, and adding this code will help us turn the full bot green now. We can design the Right annotation system later.

[derekbruening]

From timurrrr@google.com on December 07, 2011 06:59:51

It would be nice if I could put annotations in a binary and ship it without
debug info and have it work as intended.
Is it valuable to test binaries w/o debug info?

That said, supporting Valgrind code sequences is a good short-term solution (does it work on Windows?) but when we want to create new annotations I'd vote to use empty functions and require debug info.

[derekbruening]

From bruen...@google.com on December 07, 2011 07:04:44

the idea that symbols are only needed for callstacks is appealing, as results can be re-processed post-run w/ the right symbols even when the run itself lacked them: e.g., when running on a remote or production or customer machine and bringing the results back to HQ to analyze, as well as when supporting very large symbol files for 32-bit apps.

as mentioned above, issue #572 covers the (long-term) annotation design while this case just covers whether and how to make them source-compatible or binary-compatible. b/c the V8 ones are best handled in DR itself, the long-term idea is to have DR or an Extension provide some kind of annotation infrastructure (as mentioned in issue #572 ). also note that we have several annotations we want to provide beyond the ones existing in other tools (see issue #573 ).

I personally like the conditional-over-dead-code approach in issue #573 .