Handle pathological cases more consistently #1651
Conversation
|
I have added a third commit which should address my concerns on #1627. This uses the uc->size_recur_mem field, which from what I can tell was completely unused prior to this, to detect whether we are in an unaligned write (which is implementing by a recursive call into the write helper). |
Regarding #1627, could you add a unit test? I assume it should be easy to reproduce? |
|
Pushed! |
|
Idk what's up with the failing test - it passed on my previous commit and I only changed an unrelated testcase. Is it flaky? |
mingw32 runner has some bugs for a while… I really should disable it. Anyway let me rerun and it will be fine. |
| cflags &= ~CF_COUNT_MASK; | ||
| cflags |= CF_NOCACHE | 1; | ||
| /* Generate a temporary TB; do not cache */ | ||
| cflags |= CF_NOCACHE; |
There was a problem hiding this comment.
I tried your patch but I failed to understand the change here. Any reproduction code?
There was a problem hiding this comment.
Are you asking for a testcase?
The meaning of the original code is to set the count bitfield of the cflags to 1 (only lift 1 instruction) and set the CF_NOCACHE bit (do not cache the tb). The new code performs only the latter change.
There was a problem hiding this comment.
Are you asking for a testcase?
The meaning of the original code is to set the count bitfield of the cflags to 1 (only lift 1 instruction) and set the CF_NOCACHE bit (do not cache the tb). The new code performs only the latter change.
It's better if a testcase could be provided so that I can understand it better. My question is that, why setting the count to 1 solves the problem?
There was a problem hiding this comment.
You're misunderstanding the order of the diff - the old code set the count to 1, and this was undesired behavior because then it would only lift one instruction. Presumably the old code optimizes guests using exceptional control flow, but it produces inaccurate metrics (block counts).
- add test - linting for previous commit
|
It seems that CI is failing on aarch64 and ppc64le platforms. I don't have either of those machines to debug this. How can I proceed? |
Weird, I can't come up with why your code will fail on these platforms. Maybe any undefined behaviors? Let me re-run it firstly. |
|
Merged and fixed in |
|
When performing memory read operation on aarch64 and ppc64, no |
These are two changes I've needed to make to make angr work with unicorn 2.
First commit: presently, if we try to execute a non-executable region, the lifting code never checks to see if reads succeed, it just blindly trusts the result. As a consequence, we may execute some non-executable guest code or hooks before checking cpu->exit_request and breaking out of the loop. This fixes that.
Second commit: If you respond to an unmapped memory fault during instruction fetch by mapping the related memory in a callback, qemu will only attempt to lift a single instruction for the current block. This is incredibly annoying (and breaks angr's tests, which are sensitive to block counts and boundaries), so I fixed it.