-
Notifications
You must be signed in to change notification settings - Fork 566
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
DynamoRIO Incorrectly switch to thumb mode when encountering the MRRC instruction on armv7l #5233
Comments
So when the SIGILL arrives cpsr ends in 0x10 as expected for arm mode:
But when DR goes to deliver the signal it is now 0x30 (thumb mode):
So who changed it? |
Ah ok it's b/c fcache_return is Thumb, so that's just to run DR's generated code properly: /* We're going to our fcache_return gencode which uses DEFAULT_ISA_MODE */
set_pc_mode_in_cpsr(sc, DEFAULT_ISA_MODE); However, it seems to set it too early: and in fact it's set twice. The earlier one should just be removed it looks like as it causes the ISA mode to be drawn from the DR thumb target instead of the app handler target. But, there is further weirdness as @zenhumany pointed at on the list https://groups.google.com/g/dynamorio-users/c/E_4R3GbwhiA/m/Hzi3dcrMBAAJ:
The first one sets the dcontext isa mode from the next_pc; the second overrides it with the mode from the cpsr. But the cpsr is what we interrupted; we want the mode of the target, which may be a signal handler. I think that if you register a Thumb mode signal handler you have to pass LSB=1. And for re-execution of the same interrupted PC: I don't remember whether the kernel sets LSB=1 when it reports the PC?? Maybe it doesn't and that's what this is trying to handle. |
Thanks for your response! 1. Compile drrun and run sftp successfullyaccording to your advice that's removing lines 5716-5719 from core/unix/signal.c and compiler the DR, Run sftp with compiled drrun,it can success runing. 2. Run other program, may crashbut when i using this drrun run other program,
some information output by DR with -loglevel 3
|
Use lines with triple backtics to delimit literal output |
#5233 (comment) has two different issues:
|
1. Crash at different points during gdbwhen use gdb to debug dr, it may crash at different points. 2. crash point 1dr crash call stackusing gdb debug DR, the crash call stack like following:
Source code corresponding to the crash pointthe source code line 1518 corresponding to the crash point
3. crash point 2
the following is the output of dr with -loglevel 3
|
For your posts with verbatim output: use lines with triple backtics to delimit literal output like this:
|
We are about to put out a 9.0 release of DR. |
So far, I can't judge whether that assert in dispatch.c happens is introduced by removal of lines 5716-5719. |
So if I first work around QEMU bugs (#4719 (comment)) to get the linux.signalNNNN tests working under QEMU, and then I augments the linux.signalNNNN tests to vary whether the main code and the signal handler are arm or thumb, I can repro a problem:
Removing the 5716-5719 lines solves that one, but another one fails. Removing the dr_set_isa_mode from the cpsr in I think that should address this issue. I will post a PR; it would be great if you could test that patch. |
@zenhumany if you could try the patch in PR #5242 |
Removes a too-early-and-thus-incorrect call to set_pc_mode_in_cpsr() in execute_handler_from_cache() (transfer_from_sig_handler_to_fcache_return() does this for us at the right time). Removes an incorrect call to dr_set_isa_mode from the cpsr in transfer_from_sig_handler_to_fcache_return(): we want to only set the mode from the target, not the interruption point. Works around QEMU bugs with signals 63 and 64 by using 62 instead in the linux.signalNNNN tests. This allows adding them to the list of tests that work under QEMU. Augments the linux.signalNNNN tests to vary whether the main code and the signal handler are arm or thumb, helping to catch and test signal transition issues. Issue: #4719, #5233 Fixes #5233
Thank you for your hard work. 1.crash point 1
crash point2
|
Signal 12 (SIGUSR2) is used for suspending threads for synch_with_thread. This looks like a new scenario; I would argue that the original issue is indeed fixed. I would suggest filing a new issue for the ISA mode assert with synch_with_thread. Why is it doing a synch? Is it a flush? A reset? For the other assert |
Describe the bug
when i run the drrun, it crash like the following:
The following is the cpu information :
Architecture: armv7l
Byte Order: Little Endian
CPU(s): 2
On-line CPU(s) list: 0,1
Thread(s) per core: 1
Core(s) per socket: 2
Socket(s): 1
Model name: ARMv7 Processor rev 10 (v7l)
CPU max MHz: 996.0000
CPU min MHz: 396.0000
Some analysis
1. Switch to thumb mode incorrectly
The following is the output of DR with "loglevel 3" , in 53560 line ,DR switch to ARM mode, but at 53589 line , the DR use thumb decode the opcode.I think this is the first incorrectly place.
The IDA disassembly :
The DR output :
2. MRRC instruction lead to receare_app
the above switch to thumb mode incorrectly may be lead to by the mrrc instruction.
MRRC instruction raised the SIGILL natively(not under DR).
the following output is i run sftp natively under gdb.
The following may useful information to diag this issue which is output by DR with --loglevel 3.
some other information
i have open a session in the dynamorio users forum in this link [](https://groups.google.com/g/dynamorio-users/c/E_4R3GbwhiA) ,they suggested that I file an issue here.
The text was updated successfully, but these errors were encountered: