Why instrlist_first_app in inscount.cpp?

[edmund.gri...@gmail.com]

I've been working with two clients: one emulates certain instructions and the other is a modified version of inscount.cpp. I have observed inaccurate emulation when both clients are used simultaneously and I think that the cause is that my modified inscount client is inserting a clean call into the middle of a group of instructions that was generated by the emulation client. I hope to investigate further, but I have also observed that the problem goes away if I replace instrlist_first_app with instrlist_first in the modified inscount client.

It is presumably not good for one client to insert a sequence of instructions into the middle of a sequence of instructions generated by another client because both clients might use the same spill slots.

So my specific question is: why does inscount.cpp use instrlist_first_app rather than instrlist_first here? https://github.com/DynamoRIO/dynamorio/blob/3877732522c122577ea990e4be8884e966921f71/api/samples/inscount.cpp#L197

(There is also the more general question of how to prevent clients from interfering with each other and with each other's use of spill slots. However, not all general solutions may be applicable to my case because the emulation client sometimes has to do things in a slightly unorthodox way for performance reasons.)

Edmund

[Derek Bruening]

> It is presumably not good for one client to insert a sequence of instructions into the middle of a sequence of instructions generated by another client because both clients might use the same spill slots.

This is why we generally do not recommend using two clients and instead suggest turning one into a library, and why we were considering dropping the support for multiple top level clients at once.  This was also discussed at https://github.com/DynamoRIO/dynamorio/issues/4501#issuecomment-718911120.  Pasting from there:

For combining features, a library interface with explicit API calls generally works better than having separate top-level clients. Separating out concerns, isolating functionality, and creating modular units makes these libraries more re-usable. E.g., where possible it seems better to have a library function that takes in a scratch register, rather than a function that goes and obtains its own scratch registers, pushing all responsibility for which scratches to use up to the top level client, avoiding conflicts and confusion over the scratch strategy. Similarly, for logging or how to handle errors it is best to push that up to a single top-level client.  Thus, instead of just having a drcov code coverage client that is run at the top level alongside others, we have a drcovlib code coverage library that can be added to any client (and is used by drmemory for fuzzing features). Instead of just a drsyscall or drstrace client, we have a drsyscall extension library.

--
You received this message because you are subscribed to the Google Groups "DynamoRIO Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dynamorio-use...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/dynamorio-users/e21451e4-c7d3-4be0-90bd-b1a94cbe57dfn%40googlegroups.com.

[edmund.gri...@gmail.com]

> This is why we generally do not recommend using two clients and instead suggest turning one into a library, and why we were considering dropping the support for multiple top level clients at once.  This was also discussed at https://github.com/DynamoRIO/dynamorio/issues/4501#issuecomment-718911120.  Pasting from there:

Thanks, we will consider that! However currently it is still a key requirement for us to provide a closed-source emulation client and some source-code examples of instrumentation clients that work with it (so that users can modify or write their own instrumentation clients). Switching to a closed-source emulation library with instructions for building an instrumentation client using it would be big disruptive change so, if there is no real reason for the instrumentation client to use instrlist_first_app, and we can make it work by using instrlist_first instead, then we might just do that for now.