simonowen / pyz80

pyz80 - a Z80 cross assembler
GNU General Public License v2.0
18 stars 8 forks source link

Invalid instructions accepted after Python 3 support update #8

Closed simonowen closed 5 years ago

simonowen commented 6 years ago

Since commit 3fb98e4799dc813a4962114e5710bcb3edd01e0c, testing/test_invalid.sh in the test suite reports that some invalid instructions operand combinations are being accepted as valid. All valid instructions are still correctly assembled.

Need to investigate which part of the commit is causing the breakage.

stefandrissen commented 6 years ago

Sorry about that - I was focusing on the pass. I do not speak Python and the magic of 2.x is lost on me.

The first instruction that fails is bit 0,n

def single is returning:

return prefix,m,postfix

Prior to my 3.7 fix m would be '' when invalid, now it's -1. And -1 is completely valid for multiplication as used by def op_bit_type .

I will need to look further as to /how/ '' was resulting in 'normal' error handling.

stefandrissen commented 6 years ago

ok - so previously it was simply falling - I'm assuming due to the attempt at multiplying with '' out of this part:

        functioncall = 'op_'+inst+'(p,args)'
        if PYTHONERRORS:
            return eval(functioncall)
        else:
            try:
                return eval(functioncall)
            except SystemExit as e:
                sys.exit(e)
            except:
                fatal("OpCode not recognised")

And throwing 'OpCode not recognised'

simonowen commented 6 years ago

Thanks for taking a look Stefan. It doesn't affect normal use so it's fairly low priority.

I'm probably not going to get a chance to dig into it very soon. It does look like the original behaviour was fairly subtle, and might take some tracking through to understand what else it touches.

stefandrissen commented 6 years ago

I've added a Travis build that executes both test-valid and test-invalid to my fork @ https://github.com/stefandrissen/pyz80 - see the clickable failing build badge :-)