DynamoRIO / dynamorio

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

[cleanup] refactor public interface headers and eliminate patchwork exports via genapi.pl #3092

Closed derekbruening closed 3 years ago

derekbruening commented 6 years ago

Currently DR's public interface headers are constructed from a patchwork of existing internal headers using the genapi.pl script. This issue covers refactoring the headers to properly separate the public interface portions into their own headers that can be directly exported, eliminating the need for a script and simplifying the project.

derekbruening commented 6 years ago

Xref #68

derekbruening commented 5 years ago

Xref #3158

derekbruening commented 3 years ago

The general approach here is that each exported header should be copied as-is (or possibly with CMake variable expansion, as we do for dr_api.h) from a checked-in header. No more having a script extract pieces from the middle of headers.

This means that we need to extract the public, exported content from the headers and move it to new entirely-exported files.

For headers also used for internal code, the naming convention is xxx_api.h. A header with that name must contain only exported content. A CMake rule DR_export_header is used to copy it to the build dir's include/ directory and that copy can rename it: e.g., from core/module_api.h to include/dr_modules.h.

For headers only used for exported code, we use the same name as the exported header: so core/lib/dr_events.h.

derekbruening commented 3 years ago

There are a lot of complications and details to deal with here.

E.g., genapi.pl does s/dcontext_t *\*dcontext/void *drcontext/;, but w/ the internal header having the full type, the impl files do as well. So if we make the header void, the impl file won't match; do we make it void and add a cast and assume it's zero cost (could cost extra local var slot)?

What about DR_UNS_EXCEPT_TESTS_API for decode_cti? I think we have to put it in decode_api.h and define that token and just have no doxygen header and no dllexport/visible symbol.

What about AVOID_API_EXPORT? I've been trying to just reword to be fine to sit in exported header (not in doxygen comment).

What about API_EXPORT_ONLY? E.g., dr_ir_utils.h including dr_ir_instr.h? Might have to live w/ non-include-what-you-use there.

We'll have to drop not-HAVE_FVISIBILITY support so we can drop genapi.pl -filter and fully get rid of genapi.pl.

derekbruening commented 3 years ago

My task list showing all the sources of exported content and what I ended up doing with each:

derekbruening commented 3 years ago

I closed this but perhaps one final thing to consider is splitting up the large instrument.c file, maybe to match the events vs utils header split at least.