Re: [hatari-devel] regression in debugger / disasm ? -> tricky out of limit memory access

[ Thread Index | Date Index | More Archives ]

Le 25/12/2013 13:31, Nicolas Pomarède a écrit :

So, I don't think it's a config problem. I will try to look at the ext
disasm sources, sthg seems wrong here.

Took some time, it was a nasty bug due to out of limit memory access ; why it triggers with gcc 4.8.2, I don't know, different data structure / internal optimisations in gcc certainly.

So, in 68kDisass.c / Disass68k, we have a loop to look for the decoding of the EA of the opcode, around line 2020 :

                // Parse the EAs for all operands
                ea = opcode[0] & 0x3F;
                dbuf = operandBuffer;

                for(i=0; i<(sizeof(ots->op)/sizeof(ots->op[0])); ++i)

In that case, sizeof(ots->op)/sizeof(ots->op[0]) = 5 (see op[5] in the definition of OpcodeTableStruct) ; so the for loop should stop after 5 iterations ? Well, in my case it doesn't ! It goes on to 7 or 8, until op[i] is a non recognized enum in Disass68kOpcodeFormat, which creates the "DC.W" output as, the instruction in considered unknown.

I tried replacing the loop by a simple "for (i=0;i<5;i++)", but even then, the loop doesn't stop at 5 ...

After a lot of printf to try to follow the instruction flow, I saw these lines around 2420 :

         // does another operand follow => add separator
         if(ots->op[i+1] != ofNone)
              *dbuf++ = ',';

The problem is that when i=4, we test op[5], which is outside of the limit. In that case, a ',' is added to dbuf ; but dbuf is very large, we should not overwrite anything, so I don't see why it alter the processing of the for loop... (certainly the way gcc handles the stack for local variable or sthg like that)

Still, by doing :

        if ( (i+1<5) && ( ots->op[i+1] != ofNone) )
                  *dbuf++ = ',';

Then everything works again as before ; so I will commit a patch to this later.

In the meantime, if someones wants to try with mudflap or similar tools to see if an access error is reported with proper debugging flags, it could be interesting to see (is this error really detected by mudflap or others ?)


Mail converted by MHonArc 2.6.19+