Add Intel PT support on Windows - #3845
Conversation
Marcondiro
changed the title
c8eb4e5 to
a88305d
Compare
|
@Marcondiro I want to release 0.16 soon, do you think this should / could be part of the release or needs more time? |
|
Hi, I will mark this as ready tomorrow. I think it could go in 0.16 |
|
Wide downgrade is in #3873 |
| /// use libafl_intelpt::{IntelPTBuilder, PAGE_SIZE}; | ||
| /// let builder = IntelPTBuilder::default() | ||
| /// .pid(None) | ||
| /// // 0 means "Current process" ONLY on Linux! |
There was a problem hiding this comment.
should we just make this NonZero?
There was a problem hiding this comment.
On Windows .pid(0) should trace the Idle process, so changing this to NonZero on Linux would result in a different API between Linux and Win, not ideal imho.
It was Option<Pid> before but I thought that there was no need for the Option since instead of .pid(None), it is enough to just not specify .pid at all and it will default to the current process as pid(None) was doing
Moving that bit of documentation to fn pid() definition and removing .pid(0) from the default.
No strong opinion anyway, maybe adding a current_pid() method alongside pid() can make it more explicit and ergonomic?
There was a problem hiding this comment.
ah so it's just a behavior diff in linux vs windows? In that case makes sense
| /// Not calling this function will default to the current process. | ||
| /// | ||
| /// Calling this with `pid: 0` means "Current process" ONLY on Linux! | ||
| /// Prefer not setting the pid at all if you want a cross-platform "Current process". |
There was a problem hiding this comment.
Maybe By default, this will trace the current process. Set a pid to instead trace another process, instead or similar
There was a problem hiding this comment.
sounds better thanks
|
Looks good now I think. |
2 participants
Description
Support Intel PT based coverage collection through Windows'
ipt.sysdriverChecklist
./scripts/precommit.shand addressed all comments