Skip to content

Conversation

@GandholiSarat
Copy link

Adds drutil_expand_rep_string() to the instrace_simple and instrace_x86
sample clients so that REP-prefixed string instructions are expanded
consistently, matching the behavior of memtrace samples.

The expansion is performed in drmgr's app2app phase with proper drutil
initialization and teardown.

This avoids confusion for users comparing instrace and memtrace behavior.

Fixes #4138.

@abhinav92003
Copy link
Contributor

I can help review this if it is ready. Also note that there's also drx_expand_scatter_gather which performs a similar function as drutil_expand_rep_string; you may want to add that too in a separate PR perhaps.

@abhinav92003 abhinav92003 self-requested a review January 29, 2026 22:04
@GandholiSarat
Copy link
Author

Thanks! Yes, this PR is ready for review.

Good point about drx_expand_scatter_gather — I agree that would be a useful addition.
I’ll plan to do that as a separate PR.

Could you please share a bit more detail on what change you had in mind for
drx_expand_scatter_gather (e.g., which sample or section to update)?
That would help me make the follow-up correctly.

event_bb(void *drcontext, void *tag, instrlist_t *bb, bool for_trace, bool translating)
{
instr_t *first_app = instrlist_first_app(bb);
if (first_app == NULL || !drmgr_is_first_instr(drcontext, first_app))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this condition required?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check ensures REP expansion runs only once per application basic block, avoiding duplicate expansion during re-instrumentation or trace construction.

@GandholiSarat
Copy link
Author

Thanks for the detailed review!

I’ve updated both instrace_simple.c and instrace_x86.c to:

  • remove the app2app priority logic and register the callback with NULL priority
  • restore the /analysis_func/ annotation
  • use drmgr_is_first_instr_app(bb) to guard REP expansion
  • add DR_ASSERT on drutil_expand_rep_string()

The samples build and run correctly locally, and REP string instructions are now expanded and logged as expected.

Please let me know if anything else should be adjusted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add drutil_expand_rep_string to instrace_*.c samples

2 participants