CNCBASHER / mphidflash

Automatically exported from code.google.com/p/mphidflash
GNU General Public License v3.0
0 stars 0 forks source link

2 bugs in address tracking in type4 records #8

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. save this fragment as mini.hex
:020000040000fa
:020000041fc01b
:042ffc00f7ffff7f5d
:020000040000fa
:020000041fc01b
:080000000010400b000000009d
:020000040000fa
:020000041d00dd
2. build trunk with "-DDEBUG"

3. attempt to program

 USB HID device found
  Device family: PIC32
  Program memory: 512000 bytes free. 
Erasing...
Writing hex file 'mini.hex':Address: 1fc02ffc  Len 4
Writing
Completing
Address: 00000000  Len 8       <------------ ****
Writing
Completing
Address: 1d004000  Len 16
Writing
Completing
PASS 0 of 1 COMPLETE

What is the expected output? What do you see instead?

Inspection of the .hex file shows that it should write 4 bytes @ 1fc02ffc
then 8 bytes @ 1fc00000 then 16 bytes @ 1d004000

In fact, the second write is going to 0x00000000 instead of 0x1fc00000

What version of the product are you using? On what operating system?

trunk latest on Linux.

Please provide any additional information below.

The bug occurs in the handling of type 4 records:

        } else if(4 == type) { /* Extended linear address record */

          if(1 != sscanf(&hexFileData[offset+9],"%04x",&addrHi))
            return ERR_HEX_SYNTAX;
          addrHi <<= 16;
          addr32 = addrHi + addrLo;
          /* Assume this means a noncontiguous address jump; issue block
             and start anew.  The prior noncontiguous address code should
             already have this covered, but in the freak case of an
             extended address record with no subsequent data, make sure
             the last of the data is issued. */
          if(bufLen) {
            if(ERR_NONE != (status = issueBlock(addrSave,bufLen,pass)))
              return status;
            bufLen   = 0;
            addrSave = addr32;
          }

        } else { /* Unsupported record type */

bug 1. addrSave is only updated on a "flush". In the example .hex, there are 2 
adjacent type 4 records. The first one does a flush and sets addrSave to 0. The 
second sets addr32 to  0x1fc00000 but leaves addrSave at 0!! The next data 
record has an address field (low 16 bits of address) of 0 so is considered 
sequential (otherwise addrSave would have been updated and everything would be 
fine). Since it is considered sequential, it is saved ready for writing, but 
using the wrong address: 0

The fix to this is to do "addrSave = addr32" unconditionally.

bug 2. by inspection, "addr32 = addrHi + addrLo;" should be "addr32 = addrHi;". 
An addrLo value is only valid within the context/scope of a type 0 (data) 
record. To consider it another way, that addrLo value belongs with the 
*previous* addrHi. In a pathalogical situation this could cause an address to 
be considered continuous when it is not.

With these two fixes:

Erasing...
Writing hex file 'mini.hex':Address: 1fc02ffc  Len 4
Writing
Completing
Address: 1fc00000  Len 8
Writing
Completing
Address: 1d004000  Len 16
Writing
Completing
PASS 0 of 1 COMPLETE

The patch to fix these both is below:

--- hex.c.orig  2012-07-12 22:05:55.487515965 +0100
+++ hex.c       2012-07-12 22:27:47.048522549 +0100
@@ -308,7 +308,7 @@
              if(1 != sscanf(&hexFileData[offset+9],"%04x",&addrHi))
                return ERR_HEX_SYNTAX;
              addrHi <<= 16;
-             addr32 = addrHi + addrLo;
+             addr32 = addrHi;
              /* Assume this means a noncontiguous address jump; issue block
                 and start anew.  The prior noncontiguous address code should
                 already have this covered, but in the freak case of an
@@ -318,8 +318,8 @@
                if(ERR_NONE != (status = issueBlock(addrSave,bufLen,pass)))
                  return status;
                bufLen   = 0;
-               addrSave = addr32;
              }
+             addrSave = addr32;

            } else { /* Unsupported record type */
              return ERR_HEX_RECORD;

Thanks for a very useful program. I am glad of the opportunity to make it "more 
perfect".

regards,

Neal.

Original issue reported on code.google.com by foofoobe...@gmail.com on 12 Jul 2012 at 9:31

GoogleCodeExporter commented 9 years ago
patch applied

Original comment by fnargwibble on 30 Jun 2014 at 7:57