#2831 z80instructionSize() failed to parse a label (e.g. '1$:', 'label:')

[Tom Li]

Version

SDCC : z80 3.8.1 #10616 (Linux)
published under GNU General Public License (GPL)

Issue

Attempting to compile the following sample code, by calling sdcc -mz80 -o label.ihx label.c triggers a bug. The compiler will be confused by the label, and report the following error message...

label.c:14: info 218: z80instructionSize() failed to parse line node, assuming 999 bytes
'label:'

Solution

We can check if the current line included a colon, and returns 0 to handle it.

diff -upr sdcc-src-20181016-10616/src/z80/peep.c sdcc-src-20181016-10616.patch/src/z80/peep.c
--- sdcc-src-20181016-10616/src/z80/peep.c      2018-10-24 01:36:50.611826478 +0800
+++ sdcc-src-20181016-10616.patch/src/z80/peep.c        2018-10-24 01:37:22.687963925 +0800
@@ -1086,6 +1086,11 @@ int z80instructionSize(lineNode *pl)
       return(i);
     }


+  if (strchr(pl->line, ':')) {
+    /* a label, not an instruction */
+    return(0);
+  }
+
   /* If the instruction is unrecognized, we shouldn't try to optimize.  */
   /* For all we know it might be some .ds or similar possibly long line */
   /* Return a large value to discourage optimization.                   */

The patch needed for the change above has been submitted as an attachment.

Sample Code

#include <string.h>

void inline_asm_label(unsigned char *ptr)
{
    unsigned char buf[10];

    if (strlen(ptr) == NULL) {
        goto fail;
    }

__asm
label:
    .db 0x1234
__endasm;

fail:
    return;
}

[Tom Li]

It should be noted that, labels like $1: must be used instead of label:, but both usage triggers the same bug.

[Maarten Brock]

NB: The labels should be like 1$: with the $ sign after the number.

[Philipp Klaus Krause]

The patch looks like a workaround, not a fix.
AFAIK, labels shouldn't even reach z80instructionSize().

Is this a failure of isLabelDefinition() in src/SDCCpeeph.c to correctly mark the label as such?

Philipp

[Tom Li]

I don't know how SDCC works, but I tried your suggestion and added some printf()s in isLabelDefinition() of src/SDCCpeeph.c, but it seems this function is working correctly and the bug is hidden in other logics.

isLabelDefinition:    ld        l, #%3  false!
isLabelDefinition:    .db       #0xc2  false!
isLabelDefinition:    %2:  true!
isLabelDefinition:    ld        l, #%4  false!
isLabelDefinition:    %1:  true!
isLabelDefinition:    _inline_asm_label::  true!
isLabelDefinition:    $1:  true!
isLabelDefinition:    .db       0x1234  false!
isLabelDefinition:    00103$:  true!
isLabelDefinition:    00104$:  true!
label.c:14: info 218: z80instructionSize() failed to parse line node, assuming 999 bytes
'$1:'

[Philipp Klaus Krause]

Should the label lines in inline asm already be marked as such in src/SDCCgen.c? That would be consistent with how other labels are marked in code generation.

Philipp

[Tom Li]

Thanks for your suggestion, I skimmed through src/SDCCgen.c, and made the following change, and the bug is indeed gone.

diff -urp sdcc-src-20181016-10616/src/SDCCgen.c sdcc-src-20181016-10616.patch/src/SDCCgen.c
--- sdcc-src-20181016-10616/src/SDCCgen.c       2018-10-25 12:05:23.405965581 +0800
+++ sdcc-src-20181016-10616.patch/src/SDCCgen.c 2018-10-25 12:06:36.748285936 +0800
@@ -237,6 +237,7 @@ genInline (iCode * ic)
               *bp = '\0';
               ++bp;
               emitcode (begin, NULL);

+              genLine.lineCurr->isLabel = 1;
               begin = bp;
             }
           else

But I'm not sure if it's a proper fix.

[Sebastian Riedel]

Me neither, but it looks good on first sight.
I implemented this fix in [r13121] together with better label detection and space removal.

[Sebastian Riedel]

This still doesn’t work, you can’t use named labels in inline assembly that way.
It becomes:

    jr  Z, 00103$
;inlineasmtest.c:14: __endasm;
label:
    .db 0x1234
;inlineasmtest.c:16: fail:
00103$:

00103$ is in the scope of label:, which means jr can’t find it

[Tom Li]

Yes, I later realized that named labels are completely unsupported and it was not a bug. But even numerical labels were unusable, which was the actual bug. With this patch, at least numerical labels are usable, so I would say the problem is fixed.

[Sebastian Riedel]

I opened a request for inline-asm specific labels [feature-requests:#792]