-
Notifications
You must be signed in to change notification settings - Fork 42
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
Update sleigh to 10.2.3 #160
base: master
Are you sure you want to change the base?
Conversation
AttributeId::initialize(); | ||
ElementId::initialize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required with 10.2.
We had to make similar changes here lifting-bits/sleigh@f38eee4#diff-aed2f30b604fcd3c832b0255d6cd0ef5a7cca4cab0d8d2c77ee6d125d2fcf301
Hmmmm. Seems we need to change some compiler flags somewhere https://github.com/trailofbits/maat/actions/runs/3476336902/jobs/5811473141#step:5:21433 I think it's using the pre-built sleigh libraries, so I can try looking into this on lifting-bits/sleigh
|
086990c
to
5b8a674
Compare
Fixed the sleigh PIC library (and also updated to 10.2.2). Now a test is failing https://github.com/trailofbits/maat/actions/runs/3482037438/jobs/5823881087#step:10:24
|
We used the wrong `python3` command compared to what CMake found
Digging more into this... Since there were multiple assertions with the same message, I added something to differentiate them (I can commit this if you want), and the specific test that is failing is here. maat/tests/unit-tests/test_archX86.cpp Lines 6384 to 6391 in e3e6e1b
I still need to debug to determine the actual value that is causing the failure, though. I can also run and pass all tests on |
tests/unit-tests/test_archX86.cpp
Outdated
@@ -6388,7 +6388,7 @@ namespace test | |||
sym.cpu.ctx().set(X86::EAX, exprcst(32, 0x1900)); | |||
sym.mem->write(0x1900, 0xab001200abababab, 8); | |||
sym.run_from(0x1050, 1); | |||
nb += _assert( sym.cpu.ctx().get(X86::MM0).as_uint() == 0xababababdeadbeef, "ArchX86: failed to disassembly and/or execute PUNPCKHDQ"); | |||
nb += _assert( sym.cpu.ctx().get(X86::MM0).as_uint() == 0xab001200deadbeef, "ArchX86: failed to disassembly and/or execute PUNPCKHDQ"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running with a debugger, I see that the failing test has values as follows:
actual: 0xab001200deadbeef
expected: 0xababababdeadbeef
I pushed the change (to see if CI passes), but I'm not sure if this was a bug in the last implementation or a bug now 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update on this: Using a microx
script (with a fix), I can confirm that the original value expected (0xababababdeadbeef
) is correct, and there is likely a bug in sleigh somehow.
Click to see `microx` script
import traceback
import microx
def main():
o = microx.Operations()
code = microx.ArrayMemoryMap(o, 0x1000, 0x2000, can_write=False, can_execute=True)
stack = microx.ArrayMemoryMap(o, 0x80000, 0x82000)
heap = microx.ArrayMemoryMap(o, 0x10000, 0x12000)
code.store_bytes(
0x1050,
# punpckhdq mm0, [eax]
b"\x0F\x6A\x00\xeb\x0e"
)
heap.store_bytes(
0x10900,
b"\xab\x00\x12\x00\xab\xab\xab\xab"
)
m = microx.Memory(o, 32)
m.add_map(code)
m.add_map(stack)
m.add_map(heap)
t = microx.EmptyThread(o)
t.write_register("EIP", 0x1050)
t.write_register("ESP", 0x81000)
t.write_register("MM0", 0xdeadbeef12121212)
t.write_register("EAX", 0x10900)
p = microx.Process(o, m)
try:
pc = t.read_register("EIP", t.REG_HINT_PROGRAM_COUNTER)
eax = t.read_register("EAX", t.REG_HINT_MEMORY_BASE_ADDRESS)
mm0 = t.read_register("MM0", t.REG_HINT_GENERAL)
print(
f"Emulating instruction at {pc:08x} (EAX={eax:08x}, MM0={mm0:08x}"
)
p.execute(t, 1)
pc = t.read_register("EIP", t.REG_HINT_PROGRAM_COUNTER)
eax = t.read_register("EAX", t.REG_HINT_MEMORY_BASE_ADDRESS)
mm0 = t.read_register("MM0", t.REG_HINT_GENERAL)
print(
f"Finished emulating instruction: (EAX={eax:08x}, MM0={mm0:08x}"
)
except Exception as e:
print(e)
print(traceback.format_exc())
if __name__ == "__main__":
main()
Not sure if this value is correct or not but at least it shows what the value actually is for further debugging without reproducing
Seems that Ghidra has fixed some bugs for these instructions. I tested other instructions that are commented but they still fail
dfb8f62
to
5c03f5c
Compare
*Not sure if these are true fixes. Found while investigating sleigh regression
Future versions of sleigh will remove 'using namespace std;' from headers
Update language spec files
Upgrading Z3 causes test failures due to incomplete model for all expected variables. Previous versions reported values for all variables, but now tests should check whether a value has been assigned before testing the value.
Another update: The bug seems to be an internal sleigh issue because replacing the new |
I confirmed that the Bisecting might be made a little easier by using sleigh-cmake |
Contains some other small fixes/refactor.