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.

Sign up for GitHub

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" sym_resolve in python bindings #273

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

"Fix" sym_resolve in python bindings #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f413561
py3 compat
wsxarcher Jan 6, 2017
58b7eb9
correct usage of ctypes pointer
wsxarcher Jan 6, 2017
c95d3c1
clean test
wsxarcher Jan 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bindings/python/sample.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ def test_ks(arch, mode, code, syntax=0):

# test symbol resolver
def test_sym_resolver():
def sym_resolver(symbol, value):
def sym_resolver(symbol, p_value):
# is this the missing symbol we want to handle?
if symbol == "_l1":
# put value of this symbol in @value
value = 0x1002
p_value.contents.value = 0x1002
Copy link
Member

@aquynh aquynh Jan 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh now i see that this interface does not look good. would be simpler for user to return the value, rather than set the content value like this.

the reason is that this resolver must return bool value to indicate if it sets the symbol. perhaps we should change the API a bit: this function either returns symbol value, or None when it does not care. so the function above will be changed to be:

    def sym_resolver(symbol):
        # is this the missing symbol we want to handle?
        if symbol == "_l1":
            # we handled this symbol, so return symbol value
            return 0x1002

        # we did not handle this symbol, so return None
        return None

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks simpler to the end user but the C API should be changed... And what about dealing with the symbols resolved 0 in the C? Another problem

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a problem if the bindings must respect the C API, but of course it can differ.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need a wrapper for this.

btw, with this PR applied, suite/regress/x64_sym_resolver.py still does not pass yet, right?

Copy link
Contributor Author

@wsxarcher wsxarcher Jan 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use [] but maybe it's less explicit I think

def sym_resolver(symbol, value):
        # is this the missing symbol we want to handle?
        if symbol == "_l1":
            # we handled this symbol, so return symbol value
            value[0] = 0x1002
            return True

        # we did not handle this symbol, so return None
        return None

Yes, x64_sym_resolver.py still not pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix does not work at all.
sample.py works because jump relative symbol are not resolved by sym_resolve, so it is not called in any way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(because in that case is calculated by the starting point + instruction length)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just fixed Python binding in the "test" branch, so sample.py works now. but your x86_call_ptr_sym.py still fails for some unknown reason ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call is resolved differently from jmp? What about suite/regress/x64_sym_resolver.py ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a bug in the core for that testcase. will see how to fix that.

# we handled this symbol, so return true
return True

Expand Down
8 changes: 4 additions & 4 deletions suite/regress/x64_sym_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@

class TestX86(regress.RegressTest):
def runTest(self):
def sym_resolver(symbol, value):
def sym_resolver(symbol, p_value):
# is this the missing symbol we want to handle?
if symbol == "ZwQueryInformationProcess":
if symbol == b"ZwQueryInformationProcess":
# put value of this symbol in @value
value = 0x7FF98A050840
p_value.contents.value = 0x7FF98A050840
# we handled this symbol, so return true
print 'sym_resolver called!'
print('sym_resolver called!')
return True

# we did not handle this symbol, so return false
Expand Down
7 changes: 2 additions & 5 deletions suite/regress/x86_call_ptr_sym.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,16 @@
from keystone import *
import regress

def sym_resolver(symbol, value):
def sym_resolver(symbol, p_value):
if symbol == b'GetPhoneBuildString':
value = 0x41b000
p_value.contents.value = 0x41b000
return True
return False

class TestX86Nasm(regress.RegressTest):
def runTest(self):
ks = Ks(KS_ARCH_X86, KS_MODE_32)
ks.syntax = KS_OPT_SYNTAX_NASM

dir(sym_resolver)

ks.sym_resolver = sym_resolver
encoding, count = ks.asm(b"call [GetPhoneBuildString]")
self.assertEqual(encoding, [ 0xff, 0x15, 0x00, 0xb0, 0x41, 0x00 ])
Expand Down