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

Sets a default radix #382

Merged
merged 2 commits into from
Feb 8, 2019
Merged

Sets a default radix #382

merged 2 commits into from
Feb 8, 2019

Conversation

catenacyber
Copy link
Contributor

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10437

Before it gets used in AsmLexer::LexDigit

@@ -522,6 +522,7 @@ ks_err ks_close(ks_engine *ks)
KEYSTONE_EXPORT
ks_err ks_option(ks_engine *ks, ks_opt_type type, size_t value)
{
ks->MAI->setRadix(10);
Copy link
Member

Choose a reason for hiding this comment

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

i think the default radix is 16, not 10?

@aquynh aquynh merged commit d49b6fa into keystone-engine:master Feb 8, 2019
@aquynh
Copy link
Member

aquynh commented Feb 8, 2019

merged, thanks!

@Summus-31c04089c3cd80
Copy link

hello,
I think the default radix should be 10, else, why set it to 16 again if KS_OPT_SYNTAX_RADIX16 ?
Moreover, since this commit, I have this bad behaviour :

➜ kstool x32nasm "push 10"
push 10 = [ 6a 10 ]

When I set the default radix to 10 :

➜ kstool x32nasm "push 10"
push 10 = [ 6a 0a ]

You can also verify results with this python code :

import keystone
ks10 = keystone.Ks(keystone.KS_ARCH_X86, keystone.KS_MODE_32)
ks10.syntax = keystone.KS_OPT_SYNTAX_NASM
print(' '.join('{:02x}'.format(x) for x in ks10.asm('push 10')[0]))

ks16 = keystone.Ks(keystone.KS_ARCH_X86, keystone.KS_MODE_32)
ks16.syntax = keystone.KS_OPT_SYNTAX_NASM | keystone.KS_OPT_SYNTAX_RADIX16
print(' '.join('{:02x}'.format(x) for x in ks16.asm('push 10')[0]))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants