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

fix insnCodeGen::modifyData's 64-bit conversion #163

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

cuviper
Copy link
Contributor

@cuviper cuviper commented Aug 27, 2016

On RHEL6 with a prelinked /lib64/libc-2.12.so, all of tests involving
fork instrumentation were getting SIGSEGV in the mutatee. This worked
in 9.1, and it also works fine after prelink -u to undo libc. Using
git-bisect found 2b86eb4 as the point of regression.

It seems prelink ends up with libc sitting far away from the relocation
buffer, more than a 32-bit displacement, so insnCodeGen::modifyData
decides to rewrite that to a 64-bit immediate. To do this, it has to
emit additional instructions first. But after the commit above, part of
the rewritten instruction has already been written when we're trying to
emit those extras, and things gets clobbered.

This patch emits those preamble instructions first, before any part of
the newly rewritten instruction is copied out.

On RHEL6 with a prelinked `/lib64/libc-2.12.so`, all of tests involving
fork instrumentation were getting SIGSEGV in the mutatee.  This worked
in 9.1, and it also works fine after `prelink -u` to undo libc.  Using
git-bisect found 2b86eb4 as the point of regression.

It seems prelink ends up with libc sitting far away from the relocation
buffer, more than a 32-bit displacement, so `insnCodeGen::modifyData`
decides to rewrite that to a 64-bit immediate.  To do this, it has to
emit additional instructions first.  But after the commit above, part of
the rewritten instruction has already been written when we're trying to
emit those extras, and things gets clobbered.

This patch emits those preamble instructions first, before any part of
the newly rewritten instruction is copied out.
@jdetter
Copy link
Contributor

jdetter commented Aug 29, 2016

This looks good to me, were you getting this crash on 64 bit x86 or 32 bit?

@jdetter jdetter added the bug label Aug 29, 2016
@jdetter jdetter added this to the Release 9.2.1 milestone Aug 29, 2016
@cuviper
Copy link
Contributor Author

cuviper commented Aug 29, 2016

This was crashing on x86_64. Isn't this conversion impossible for 32-bit?

@jdetter
Copy link
Contributor

jdetter commented Aug 29, 2016

Oh yeah, duh

@cuviper
Copy link
Contributor Author

cuviper commented Aug 29, 2016

Ok, is there anything to hold this up? We outsiders can't see what Jenkins is doing on follis...

@jdetter
Copy link
Contributor

jdetter commented Aug 30, 2016

You should be good!

@wrwilliams
Copy link
Member

@cuviper Any experience with the AWS-hosted Jenkins here: https://bitnami.com/stacks/developer-tools? An opaque-to-world Jenkins (that also can't properly respond to github) is not the world's most useful CI tool, and I'd certainly rather get it off of my development box...

@jdetter
Copy link
Contributor

jdetter commented Aug 30, 2016

@wrwilliams I've done a ton of work with AWS in the past if you need any help, especially if it's setting up a Jenkins box

@cuviper
Copy link
Contributor Author

cuviper commented Aug 30, 2016

I don't have any experience setting up Jenkins.

Another option is buildbot, e.g. my colleague runs this for GDB: http://gdb-build.sergiodj.net/. I'm not sure where the individual builders are though -- probably still just random development boxes. I think Rust uses buildbot on AWS, with Homu for GitHub integration -- see rust-infra. But again, I haven't run any of these things myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants