Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverage Pin Tool Gives Incorrect Output For 32-bit Traces #25

Closed
0xAlexei opened this issue Dec 2, 2017 · 4 comments
Closed

Coverage Pin Tool Gives Incorrect Output For 32-bit Traces #25

0xAlexei opened this issue Dec 2, 2017 · 4 comments
Labels
bug
Milestone

Comments

@0xAlexei
Copy link

@0xAlexei 0xAlexei commented Dec 2, 2017

The coverage Pin tool outputs address incorrectly for 32 bit traces.

I think this can be fixed by explicitly casting ADDRINTs image.low_ and image.high_ at https://github.com/gaasedelen/lighthouse/blob/master/coverage/pin/CodeCoverage.cpp#L208 to to uint64_t, as the format string calls for that, and if we're on a 32 bit trace, the format string will pull too much off the stack - and will also break outputting the module name. Doing so appeared to fix the issue on my system, but I haven't tested enough to issue a PR knowing it won't break anything...

@gaasedelen
Copy link
Owner

@gaasedelen gaasedelen commented Dec 2, 2017

@agustingianni
Copy link
Contributor

@agustingianni agustingianni commented Dec 2, 2017

Interesting, thanks for the report @bulaza I will try to fix this.

@gaasedelen gaasedelen added the bug label Dec 2, 2017
@gaasedelen gaasedelen added this to the v0.7.0 milestone Dec 2, 2017
@0xAlexei
Copy link
Author

@0xAlexei 0xAlexei commented Dec 5, 2017

Also note that the OnImageLoad callback printing out address works fine, as it uses the "lx" format specifier, but the one during output uses "llx"

@gaasedelen
Copy link
Owner

@gaasedelen gaasedelen commented Dec 5, 2017

This should now be fixed. I cleaned up a few other types that the compiler was throwing warnings for.

I'll build new binaries for the v0.7 release.

@gaasedelen gaasedelen closed this Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.