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

Rewriting with StackMods broken #111

Closed
morehouse opened this issue Jun 28, 2016 · 8 comments · Fixed by #112
Closed

Rewriting with StackMods broken #111

morehouse opened this issue Jun 28, 2016 · 8 comments · Fixed by #112
Assignees
Labels
Milestone

Comments

@morehouse
Copy link
Contributor

morehouse commented Jun 28, 2016

I'm getting the following assertion failure during rewriting:

performStackMods: /p/paradyn/development/mm/githead/tmp/dyninst/common/src/arch-x86.C:8559: int NS_x86::ia32_decode_opcode(unsigned int, const unsigned char*, NS_x86::ia32_instruction&, NS_x86::ia32_entry**): Assertion "instruct.cond != __null" failed.

Full backtrace:

#0  0x0000003193032625 in raise () from /lib64/libc.so.6
#1  0x0000003193033e05 in abort () from /lib64/libc.so.6
#2  0x000000319302b74e in __assert_fail_base () from /lib64/libc.so.6
#3  0x000000319302b810 in __assert_fail () from /lib64/libc.so.6
#4  0x00007fb784d1d4f7 in NS_x86::ia32_decode_opcode (
    capa=<value optimized out>, addr=<value optimized out>, 
    instruct=<value optimized out>, gotit_ret=0x0)
    at /p/paradyn/development/mm/githead/tmp/dyninst/common/src/arch-x86.C:8559
#5  0x00007fb785fa0275 in insnCodeGen::modifyDisp (newDisp=168, 
    insn=<value optimized out>, gen=..., arch=Dyninst::Arch_x86_64, 
    addr=4403983)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/codegen-x86.C:1331
#6  0x00007fb785f516fa in Dyninst::Relocation::StackModPatch::apply (
    this=0xe3179a0, gen=...)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/Relocation/Widgets/StackModWidget.C:81
#7  0x00007fb785f6941d in Dyninst::Relocation::CodeBuffer::BufferElement::generate (this=0xe317a30, buf=0xb17e038, gen=..., shift=@0xb17e1cc, 
    regenerate=@0x7fff9a30595f)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/Relocation/CodeBuffer.C:117
#8  0x00007fb785f695b7 in Dyninst::Relocation::CodeBuffer::generate (
    this=0xb17e038, baseAddr=<value optimized out>)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/Relocation/CodeBuffer.C:274
#9  0x00007fb785ec6dcb in AddressSpace::generateCode (this=0x138b5b0, cm=..., 
    nearTo=4744750)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/addressSpace.C:1955
#10 0x00007fb785eca300 in AddressSpace::relocateInt (this=0x138b5b0, 
    begin=<value optimized out>, end=<value optimized out>, nearTo=4744750)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/addressSpace.C:1765
#11 0x00007fb785ecb5bb in AddressSpace::relocate (this=0x138b5b0)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/addressSpace.C:1721
#12 0x00007fb785f6c3cc in Dyninst::PatchAPI::DynInstrumenter::run (
    this=<value optimized out>)
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/Relocation/DynInstrumenter.C:54
#13 0x00007fb78446862b in Dyninst::PatchAPI::Patcher::run (this=0x1640c00)
    at /p/paradyn/development/mm/githead/tmp/dyninst/patchAPI/src/Command.C:113
#14 0x00007fb7844681da in Dyninst::PatchAPI::Command::commit (this=0x1640c00)
    at /p/paradyn/development/mm/githead/tmp/dyninst/patchAPI/src/Command.C:54
#15 0x00007fb785e94ebc in BPatch_binaryEdit::writeFile (this=0x138b520, 
    outFile=0x560be98 "rewrittenperlbench-orig")
    at /p/paradyn/development/mm/githead/tmp/dyninst/dyninstAPI/src/BPatch_binaryEdit.C:215
#16 0x000000000040467b in main (argc=<value optimized out>, 
    argv=<value optimized out>) at performStackMods.C:516

This issue seems to have been introduced by a series of commits by @jdetter starting with 60f1779.

@morehouse morehouse added the bug label Jun 28, 2016
@morehouse morehouse added this to the 9.2 Release milestone Jun 28, 2016
@jdetter jdetter self-assigned this Jun 28, 2016
@jdetter
Copy link
Contributor

jdetter commented Jun 28, 2016

Can you do a full rebuild and make sure master is merged into your local branch? I will take a look at this first thing tomorrow morning. Also could I have access to the binary you're running on and your reproducer?

@jdetter
Copy link
Contributor

jdetter commented Jun 28, 2016

@wrwilliams depending on the severity of this issue we might want to think about sneaking this into 9.2.X. I suspect this will be a quick fix.

@morehouse
Copy link
Contributor Author

This issue is on the vanilla master branch. No merging was done.

@jdetter: I'll email you the binary and mutator.

@wrwilliams
Copy link
Member

Yes, this is a regression and should be a simple fix.

@jdetter
Copy link
Contributor

jdetter commented Jun 28, 2016

@morehouse thanks that will speed this fix up quite a bit.

@wrwilliams I will create a new branch and hopefully have a pull request ready for you to look at tomorrow.

@jdetter
Copy link
Contributor

jdetter commented Jun 29, 2016

@morehouse Can you run my patch? I wasn't able to reproduce the assert unfortunately. Here is the git checkout command:

git checkout release9.2/fixes/rewriter_assert

I want to make sure there aren't any followup bugs before I create a pull request for master.

@jdetter
Copy link
Contributor

jdetter commented Jun 29, 2016

btw I got a different error when I ran your reproducer, not sure if I did something wrong or not:

./performStackMods called with:
         BIN_FILE = perlbench-orig
         OUT_FILE = perlbench-rewrite
         CANARIES = true
         VERBOSE = false
ADDMODS SUCCEEDED for 0x4741d0
./performStackMods: symbol lookup error: ./performStackMods: undefined symbol: _ZN15BPatch_function7getNameEv

@morehouse
Copy link
Contributor Author

@jdetter Your patch fixes the issue for me.

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 a pull request may close this issue.

3 participants