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

Armt: add some instructions #1029

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

b14aa178
Copy link
Contributor

im not ensure it right, but dis is ok.

@b14aa178 b14aa178 changed the title add LDMIA instructions [WIP] add LDMIA instructions May 3, 2019
@b14aa178 b14aa178 changed the title [WIP] add LDMIA instructions [WIP] add armt instructions May 3, 2019
@serpilliere
Copy link
Contributor

I am restarting CI here, as it seems the faulty test is due to bad travis.

@b14aa178 b14aa178 force-pushed the add_arm_inst branch 2 times, most recently from ae9f08d to d17b762 Compare May 17, 2019 16:58
@b14aa178
Copy link
Contributor Author

hi @serpilliere, i add test code for instruction, but why test/arch/arm/sem.py seems do not include armt semantic test? do you forget it or other reason? if i write some armt instruction semantic test, it' s a right place?

@serpilliere
Copy link
Contributor

Hi @wzrybuddy !
Currently in miasm, we have tests for the assembler/disassebmler in test/arch/arm/arch.py in which we try to assemble/disassemble each supported instruction (ok, maybe not every form of every instructions...)
It's a bit different for semantic: we only add regression tests for complex semantic or if the semantic is really prone to error.

To test the semantic, instead of testing the semantic directly, our preference goes to implement a little code and emulate it with special case, then check the registers/flags.

For example, here is the test for an instruction in aarch64, in miasm/test/arch/aarch64/unit/mn_ubfm.py:

#! /usr/bin/env python2

import sys

from asm_test import Asm_Test
from pdb import pm


class Test_UBFM1(Asm_Test):
    TXT = '''
main:
       MOVZ    X0, 0x5600
       UBFM    X0, X0, 8, 15
       RET     LR
    '''
    def check(self):
        assert(self.myjit.cpu.X0 == 0x56)
        pass

class Test_UBFM2(Asm_Test):
    TXT = '''
main:
       MOVZ    X0, 0x56
       UBFM    X0, X0, 4, 55
       RET     LR
    '''
    def check(self):
        assert(self.myjit.cpu.X0 == 0x5)
        pass


if __name__ == "__main__":
    [test(*sys.argv[1:])() for test in [Test_UBFM1, Test_UBFM2 ]]

You have a bunch of these for x86 in miasm/test/arch/x86/unit/` if you want more examples!

@serpilliere
Copy link
Contributor

Ho, by the way, I will fix the tipo and relaunch your tests: it seems broken tests here are not your fault: it's only due to our bad english...

@serpilliere
Copy link
Contributor

I fix tipo with #1041
Once it's ok and merged, I will hint you here, then you will be able to rebase your PR and fully pass regression tests.

@b14aa178 b14aa178 changed the title [WIP] add armt instructions [WIP] Armt: add some instructions May 18, 2019
@serpilliere
Copy link
Contributor

It seems some of your instructions have a length of 31 bits.

miasm/core/cpu.py Outdated Show resolved Hide resolved
@serpilliere
Copy link
Contributor

Hi @327135569 ,
Thank you for the PR update! (and sorry for the delay)
The PR seems ok. The only thing that may a problem is the semantic.
Maybe we could either:

  • remove the semantic for the todo (which will create an error on run time if we encounter it =
  • or explicitly add a raise` NotimplementedError, which will also raise an error at run time

I think this is necessary because if we let code empty, it will silently give an IR which will be wrong (as mnemonic will miss) and it is quite disturbing during analysis.

Are you ok to fix this or do you want me to do a pr on your pr ?

@b14aa178
Copy link
Contributor Author

b14aa178 commented Sep 8, 2021

Thank you! I will try to add semantic later.

# one dp to two gp
# a = c[32:]
# b = c[:32]
e.append(ExprAssign(a, (c & ExprInt(0xffffffff, 64))[0:32]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, you are close to a good semantic.
Just a remark here, as you are doing a [0:32] (which will extract the first 32 bits) you don't need to mask it before with the & 0xFFFFFFFF

# a = c[32:]
# b = c[:32]
e.append(ExprAssign(a, (c & ExprInt(0xffffffff, 64))[0:32]))
e.append(ExprAssign(b, ((c >> ExprInt(32, 64)) & ExprInt(0xffffffff, 64))[0:32]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, the bit truncation will do the mask job

@serpilliere
Copy link
Contributor

Hey @327135569
It seems your are very close to the good semantic, I just added 2 remarks here are I think everything will be ok!

@b14aa178
Copy link
Contributor Author

b14aa178 commented Sep 9, 2021

Gosh, i am too slow to add semantic.. but seems OK, can you merge ?

@b14aa178 b14aa178 changed the title [WIP] Armt: add some instructions Armt: add some instructions Sep 9, 2021
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.

2 participants