DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.66k stars 562 forks source link

Targeted tests for trace building #5057

Open abhinav92003 opened 3 years ago

abhinav92003 commented 3 years ago

Today we have very limited testing for trace building. client.events has tests for the trace client event. client.null_instrument only verifies whether a trace is built or not. Mostly we rely on implicit trace creation in other tests in our suite, which doesn't allow detection of bugs besides those that would cause crashes. We should add comprehensive testing for corner cases.

Vincent-lau commented 3 years ago

I had a look at these register_events methods and they seem to me to be client-side, and DR does not seem to have methods that expose the fragment to the client

With dr_register_trace_event you can register a callback that takes the trace as input. The documentation also states that "trace is a pointer to the list of instructions that comprise the trace." and " The user is free to inspect and modify the non control-flow instructions in the trace before it executes, with certain restrictions". Are you saying that the given trace is not the one after the trace creation code runs?

void dr_register_trace_event(dr_emit_flags_t (*func)(void *drcontext, void *tag,
                                                     instrlist_t *trace,
                                                     bool translating));

https://github.com/DynamoRIO/dynamorio/blob/271d357cd5609cd1668e67c4617b17364c33c67e/core/lib/dr_events.h#L392

You are right that we can register a callback and inspect the trace with a client, but according to the documentation: "Registers a callback function for the trace event. DR calls func before inserting a new trace into the code cache. " which means that trace is a instrlist that has not been fixedup as it is not in the code cache. And so it is not possible for the client to check if there is a, say, cbnz instruction present, and we can only check if the trace event is triggered (and therefore trace is created) using this method.

derekbruening commented 3 years ago

I think there is value in having just sanity checks that exercise every path. If they're in a client test, they wouldn't look at trace stitching/mangling internals as you say, but they could at least hit every branch case and catch basic problems. We'd have an app that has a trace-triggering loop for every type of branch to hit all the stitching cases. If it's a pure-asm app we could have it check the cache exit stats and confirm there are no barriers or gaps in between traces and that all hot code is staying in the cache (a not-easy-to-find problem we've had before). And if we run it with -stress_recreate_state we can test translation of all the mangling/stitching.

What types of tests were you imagining that would look at the actual internal stitching/mangling instructions?

Vincent-lau commented 3 years ago

What types of tests were you imagining that would look at the actual internal stitching/mangling instructions?

What I was thinking was to check the shape of a trace before it is emitted into the code cache. For example, we might expect a trace to have something like:

mov x0 jump_target
eor x0 x0 #trace_next
cbnz 
etc

And we want to check the presence of these instructions to make sure the trace is mangled correctly. I think that is a way of directly covering the code path of newly added code for trace creation. But what you said about triggering trace building and checking all the stats are certainly worth testing as well.