|
View:
New views
4 Messages
—
Rating Filter:
Alert me
|
|
|
code_pack issuesI've been working on a project that uses code_pack: it #include's a
generated HID descriptor in a code_pack section, and this was failing at link time due to the debug info (in the object file) being off in offset because of it being non word aligned. The attached patch (against svn trunk) fixes that. It's a bit interesting, it'll end up adding line entries for non-word aligned addresses, which wouldn't have ever happened before, but which I think is 'more obviously correct'. The patch to coff.c is the only part of the attached patch for this issue. In addition, listing was broken when you had labels between non-aligned words (with no code generating statement attached); it looks like the listing code had been refactored in svn and this cropped up. The bug manifests as an infinite loop while assembling. I fixed this, and in the patch there's a new test, code_pack2.asm, which acts as a regression test for this. The second part of the lst.c patch is for this issue, along with making sure the address the list outputs for a label on its own line is correct when not word aligned. I also changed the listing for code_pack sections to display byte-by-byte all the time. I started down that road because there was another listing bug grabbing the wrong byte out of a word when writing the list, though I can't remember which one... So if you don't take the listing changes, you should probably look at the listing for the code_pack2.asm file and make sure it's OK. This part of the patch also monkeys with how the spacing is written to the byte section of a listing line. This is the first part of that lst.c patch. Finally, the code_pack.o in the testsuite would need to be 'blessed' from the patched assembler before the testsuite will verify, because 'technically' the current object file has some incorrect line number address pairings. Thanks for everyone's work on gputils and take care, -- Michael Ballbach, N0ZTQ ballbach@... -- PGP KeyID: 0xA05D5555 http://www.rten.net/ diff -uNr -x entries gputils-svn.orig/gputils/gpasm/coff.c gputils-svn/gputils/gpasm/coff.c --- gputils-svn.orig/gputils/gpasm/coff.c 2008-11-28 23:41:12.000000000 -0500 +++ gputils-svn/gputils/gpasm/coff.c 2008-11-28 23:47:30.000000000 -0500 @@ -361,6 +361,10 @@ int i; int origin; static gp_boolean show_bad_debug = true; + gp_boolean emitted_pack_byte; + + /* note if we're doing code_pack work */ + emitted_pack_byte = state.obj.section && state.obj.section->emitted_pack_byte; if ((!state.obj.enabled) || (state.obj.section == NULL)) return; @@ -381,7 +385,7 @@ return; } - for (i = 0; i < emitted; i++) { + for (i = 0; i < emitted + (emitted_pack_byte ? 1 : 0); i++) { new = gp_coffgen_addlinenum(state.obj.section); if (state.debug_info) { @@ -391,7 +395,14 @@ new->symbol = state.src->file_symbol; new->line_number = state.src->line_number; } - new->address = origin + (i << _16bit_core); + + /* when emitting non-word aligned data, we must modify + * the origin address if our initial emission was a byte + * in an existing word. subsequent bytes/words then need + * to subtract 2, to compensate for the fact that the + * origin was off by a whole word. */ + new->address = origin + (i << _16bit_core) + - (emitted_pack_byte ? (i == 0 ? 1 : 2) : 0); } return; diff -uNr -x entries gputils-svn.orig/gputils/gpasm/lst.c gputils-svn/gputils/gpasm/lst.c --- gputils-svn.orig/gputils/gpasm/lst.c 2008-11-28 23:41:12.000000000 -0500 +++ gputils-svn/gputils/gpasm/lst.c 2008-11-28 23:33:20.000000000 -0500 @@ -218,36 +218,56 @@ char buf[BUFSIZ]; unsigned int i; unsigned int lst_bytes = 0; + size_t m_spacing, m_prev_len = strlen(m), m_after_len; - if ((byte_org & 1) != 0) { - /* not word-aligned */ - /* list first byte */ - unsigned char emit_byte = (unsigned char)(i_memory_get(state.i_memory, - (byte_org >> 1)) >> 8); - snprintf(buf, sizeof(buf), "%02X", emit_byte); - strncat(m, buf, sizeof_m); - ++lst_bytes; + if (bytes_emitted == 0) + return 0; + + /* when in a byte packed section, print byte by byte */ + if (state.obj.new_sec_flags & STYP_BPACK) { + gp_boolean started_odd = (byte_org & 1) != 0; + + if (started_odd) { + /* not word-aligned */ + /* list first byte */ + unsigned char emit_byte = (unsigned char)(i_memory_get(state.i_memory, + (byte_org >> 1)) >> 8); + snprintf(buf, sizeof(buf), "%02X ", emit_byte); + strncat(m, buf, sizeof_m); + ++lst_bytes; + } else if (bytes_emitted == 1) { + /* word-aligned */ + /* but only one byte */ + unsigned char emit_byte = (unsigned char)(i_memory_get(state.i_memory, + (byte_org >> 1)) & 0xff); + snprintf(buf, sizeof(buf), "%02X ", emit_byte); + strncat(m, buf, sizeof_m); + ++lst_bytes; + } /* list whole words */ - for (i = 0; (i < ((bytes_emitted-1) >> 1)) && (i < 1); ++i) { + for (i = 0; (i < ((bytes_emitted-started_odd) >> 1)) && (i < 1); ++i) { unsigned int emit_word = i_memory_get(state.i_memory, - ((byte_org+1) >> 1) + i) & 0xffff; - snprintf(buf, sizeof(buf), "%02X %02X", emit_word & 0x00ff, + ((byte_org+started_odd) >> 1) + i) & 0xffff; + snprintf(buf, sizeof(buf), "%02X %02X ", emit_word & 0x00ff, emit_word >> 8); strncat(m, buf, sizeof_m); lst_bytes += 2; } - /* list extra byte if odd */ - if (((byte_org+bytes_emitted) & 1) != 0) { - snprintf(buf, sizeof(buf), "%02X ", i_memory_get(state.i_memory, - ((byte_org + bytes_emitted - 2) >> 1)) & 0x00ff); + if (i != 1 && started_odd && bytes_emitted > 1) { + /* we have space for an extra byte if we had no whole word and we started odd */ + snprintf(buf, sizeof(buf), "%02X", i_memory_get(state.i_memory, + ((byte_org + 1) >> 1)) & 0xff00 >> 8); + strncat(m, buf, sizeof_m); + ++lst_bytes; + } else if (!started_odd && i == 1 && bytes_emitted > 2) { + /* we have space for another byte, word aligned */ + snprintf(buf, sizeof(buf), "%02X", i_memory_get(state.i_memory, + (byte_org + 2) >> 1) & 0xff); strncat(m, buf, sizeof_m); ++lst_bytes; - } - else { - strncat(m, " ", sizeof_m); } } - else { /* word-aligned */ + else { /* non-code pack section */ /* list full words as bytes */ for (i = 0; (i < (bytes_emitted >> 1)) && (i < 2); ++i) { unsigned int emit_word = i_memory_get(state.i_memory, @@ -270,6 +290,13 @@ } } + /* append appropriate spacing */ + m_after_len = strlen(m); + m_spacing = m_after_len - m_prev_len; + for(; m_spacing < 10; m_spacing++) { + m[m_after_len++] = ' '; + } + m[m_after_len] = '\0'; return lst_bytes; } @@ -307,10 +334,22 @@ if (state.obj.section) byte_org -= (state.obj.section->emitted_pack_byte ? 1 : 0); bytes_emitted = (state.org << 1) - byte_org; - if (state.obj.section) - bytes_emitted -= (state.obj.section->have_pack_byte ? 1 : 0); + + /* deal with offset changes for non-word aligned data */ + if (state.obj.section) { + /* do we have a pending byte for a full word? */ + if (state.obj.section->have_pack_byte) { + /* did we emit some bytes? if so we have to subtract a half word */ + if (bytes_emitted > 0 && state.obj.section->have_pack_byte) + bytes_emitted--; + /* is this a label or something with no instructions? then our + * org must be modified. */ + else if (bytes_emitted == 0) + byte_org--; + } + } emitted = (bytes_emitted >> 1); - if (((byte_org & 1) == 0) && ((bytes_emitted & 1) != 0)) + if (bytes_emitted > 0 && ((byte_org & 1) == 0) && ((bytes_emitted & 1) != 0)) emitted += 1; snprintf(m, sizeof(m), "%04X ", byte_org >> (1 - _16bit_core)); diff -uNr -x entries gputils-svn.orig/gputils/gpasm/testsuite/gpasm.project/objasm/code_pack2.asm gputils-svn/gputils/gpasm/testsuite/gpasm.project/objasm/code_pack2.asm --- gputils-svn.orig/gputils/gpasm/testsuite/gpasm.project/objasm/code_pack2.asm 1969-12-31 19:00:00.000000000 -0500 +++ gputils-svn/gputils/gpasm/testsuite/gpasm.project/objasm/code_pack2.asm 2008-11-28 23:47:56.000000000 -0500 @@ -0,0 +1,67 @@ +; this is my test file for the code_pack gpasm patch + +; +; the idea is to test as many permutations of packed data as possible, +; and to make sure that the label addresses generated are always correct. +; + + list p=18f2455 + +blech code_pack 0x100 + +label0 + db 1 +label1 + db 2 +label2 + db 3, 4 +label4 + db 5 +label5 + db 6, 7, 8 +label8 + db 9, 0xa, 0xb +label0xb + db 0xc +label0xc + db 0xd, 0xe, 0xf, 0x10 +label0x10 + db 0x11, 0x12, 0x13, 0x14, 0x15 +label0x15 + db 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b +label0x1b + db 0x1c +label0x1c + db 0x1d, 0x1e +label0x1e + db 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24 +label0x24 + db 0x25 +label0x25 + db 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c +label0x2c + db 0x2d + +blech2 code_pack 0x200 + +blabel0 db 1 +blabel1 db 2 +blabel2 db 3, 4 +blabel4 db 5 +blabel5 db 6, 7, 8 +blabel8 db 9, 0xa, 0xb +blabel0xb db 0xc +blabel0xc db 0xd, 0xe, 0xf, 0x10 +blabel0x10 db 0x11, 0x12, 0x13, 0x14, 0x15 +blabel0x15 db 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b +blabel0x1b db 0x1c +blabel0x1c db 0x1d, 0x1e +blabel0x1e db 0x1f, 0x20, 0x21, 0x22, 0x23, 0x24 +blabel0x24 db 0x25 +blabel0x25 db 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, 0x2c +blabel0x2c db 0x2d + +blech3 code + movlw 0 + end + Files gputils-svn.orig/gputils/gpasm/testsuite/gpasm.project/objfiles/code_pack2.o and gputils-svn/gputils/gpasm/testsuite/gpasm.project/objfiles/code_pack2.o differ Files gputils-svn.orig/gputils/gpasm/testsuite/gpasm.project/objfiles/code_pack.o and gputils-svn/gputils/gpasm/testsuite/gpasm.project/objfiles/code_pack.o differ |
|
|
Re: code_pack issuesThese are great changes. I don't have any objections to listing the
code_pack sections byte-by-byte if that works for everyone. I can't imagine it would cause any problems. Would I be right in assuming that the infinite loop bug is the same as our bug #1988473?( https://sourceforge.net/tracker/?func=detail&atid=431665&aid=1988473&group_id=41924 ) Thanks a bunch for the effort! David |
|
|
Re: code_pack issuesOn Sun, Nov 30, 2008 at 09:35:34AM -0500, David Barnett wrote:
> These are great changes. I don't have any objections to listing the > code_pack sections byte-by-byte if that works for everyone. I can't > imagine it would cause any problems. For comparison, it's what MPASM does. The only way we differ is they use 15 characters for this 'field' in the output, and we use only 10. > Would I be right in assuming that the infinite loop bug is the same as > our bug #1988473?( > https://sourceforge.net/tracker/?func=detail&atid=431665&aid=1988473&group_id=41924 > ) Yes, definitely. I would have looked at this earlier had I known it was an issue, I'm sorry to say that while I subscribe to the list I am not very attentive when I am busy with other things. Do I need to sign-up in there or something? I would do so if you wanted to assign bugs like that to me in the future. -- Michael Ballbach, N0ZTQ ballbach@... -- PGP KeyID: 0xA05D5555 http://www.rten.net/ |
|
|
Re: code_pack issuesOn Sun, Nov 30, 2008 at 2:31 PM, Michael Ballbach <ballbach@...> wrote:
> On Sun, Nov 30, 2008 at 09:35:34AM -0500, David Barnett wrote: > > Would I be right in assuming that the infinite loop bug is the same as > > our bug #1988473?( > > > https://sourceforge.net/tracker/?func=detail&atid=431665&aid=1988473&group_id=41924 > > ) > > Yes, definitely. I would have looked at this earlier had I known it was > an issue, I'm sorry to say that while I subscribe to the list I am not > very attentive when I am busy with other things. Do I need to sign-up in > there or something? I would do so if you wanted to assign bugs like that > to me in the future. No, that's quite alright. I just wanted to double-check I should close that ticket when I commit the patch. You've helped a bunch already. David |
| Free embeddable forum powered by Nabble | Forum Help |