#208 Switch statement fails if inline assembler contains label

open
nobody
None
5
2007-05-22
2007-05-21
GrahamNZ
No

Hi all,
I have researched the problem described below and not found reference to it before - hence the bug report.

SDCC complier version 2.6.0 #4039 (July 28 2006)

I don't know if it is a linker or optimiser issue so have placed this report in the mcs51 category.

I have discovered that if a switch statement contains an in-line assembler block (within a case...break statement) which happens to contain a code label, as per this example...

switch (mode)
{
case MEASURING_A:
_asm
A_label:
_endasm ;

break ;
case MEASURING_B:
P0 = 0xFB ;
break;

case MEASURING_C:
P0 = 0xFE ;
break;
}

then the following error message is produced (one of several examples produced for the above code):

?ASxxxx-Error-<u> in line 1047 of Switch.asm
<u> undefined symbol encountered during assembly

Note: The presence or not of real lines of assembler code have no effect - only removing the label is effective at clearing the error.

Examination of the asm file reveals that the switch statement jump labels, although appearing in the asm file, are reported as 'undefined'.

I attach all files needed to compile the above project.

Regards,
Graham

Discussion

  • GrahamNZ
    GrahamNZ
    2007-05-21

    Source to prove Switch statement problems

     
    Attachments
  • wek
    wek
    2007-05-21

    Logged In: YES
    user_id=1201677
    Originator: NO

    Graham,

    Your code violates the rule for inline assembly labels (see chapter 3.12.3 of sdccman). Please retry with conformant label (of the xxxx$: form, xxxx is a hexadecimal number less than 100).

    Jan Waclawek

     
  • Borut Ražem
    Borut Ražem
    2007-05-21

    • milestone: --> 100455
    • status: open --> closed-rejected
     
  • Borut Ražem
    Borut Ražem
    2007-05-21

    Logged In: YES
    user_id=568035
    Originator: NO

    My suspicion was wrong and Jan's explanation is correct and it works.

    I'm closing it since it is not a bug.

    Borut

     
  • GrahamNZ
    GrahamNZ
    2007-05-21

    Logged In: YES
    user_id=1797831
    Originator: YES

    Hi guys,
    Thanks for your help.

    I have tried snapshot 2.7.0 and found the problem still exists with my label but is OK with 0001$.

    I had extensively read SDCC Manual 3.12.3, and the best I can describe is that it is confusing. I suggest we all read it, discuss and try to understand given the text and the example what is allowed as a label.

    The text states that 'older versions of the compiler HAD TO USE nnnn$' - this implies that this form is no longer mandatory.

    Also, the example includes 'clabel:' as a label - this is outside an _asm block so it is reasonable that it cannot be referenced inline, but my point is that this label is obviously legal and acceptable to the compiler.

    Also, we have inline elsewhere in our program:

    jbc _TI, tx_character ; See if we have finished sending the last byte
    sjmp isr_done

    tx_character:
    clr c ; Check for more characters to send
    mov a, _tx_index
    subb a, _tx_length
    jnc isr_done
    movx a, @dptr
    mov _SBUF, a ; Send the byte
    inc _tx_index
    isr_done:

    and this seems to be totally acceptable to the compiler.

    So, there seems to be some inconsistancy in the documentation and compiler performance, and maybe all that is required is to clearly state that real names cannot be used as labels. I personally feel that this would be the wrong approach to resolve the issue.

    I come back to the original point: the label inside the case part of a switch statement corrupts the switch statement code. The same code outside the switch statement compiles (I just tried it and can upload the code if desired) - which in my mind confirms that there is a bug with the switch/case/in-line label code generation.

    In the interim I will continue with the old label form and try to resolve the 'assertion failed' and 'caught signal 22: SIGABRT' errors that have now appeared and stop the project from compiling.

    Hopefully I am not being too harsh here - I think the compiler is great and you folks perform an excellent task to us all. Together we can make a good product even better.

    Your comments welcome,
    Graham

     
  • wek
    wek
    2007-05-21

    Logged In: YES
    user_id=1201677
    Originator: NO

    Graham,

    I already submitted a patch to the documentation containing rewording of the said paragraph addressing most of your concerns; but Frieder who applied the patch felt that it is not justified enough.

    Maarten, Frieder and other developers, can you please review that patch and decide whether that rewording is good or not. Thanks.

    JW

     
  • GrahamNZ
    GrahamNZ
    2007-05-22

    Logged In: YES
    user_id=1797831
    Originator: YES

    Hello Borut and Jan,

    I think the issue I have raised has not been investigated by the support team.

    A synopsis of it is:
    "Labels within inline assembler can consist of alphanumeric characters EXCEPT if the inline assembler is within a C switch/case statement where they MUST comply with the form nnnn$, and if not then the switch/case statement cannot be compiled."

    This to me would seem to be an inconsistancy.

    We can as Jan has proposed rewording the 3.12.3 section, presumably to insist that labels are of the nnnn$ form, but let stop and think about the constraint that that is for a moment.

    The nnnn$ label is only machine readable - it totally precludes using sensible names which when programming speed and resource optimised complex functions is mandatory (IMHO). Even if not programming at that level using meaningful labels makes the code substancially more readable.

    I truely believe that if alphanumeric names are allowed at one point then they should be allowed anywhere inline assembler is used. To do any less is to take a backward step.

    I encourage you to re-read my comments, try the sample program I provided and reopen this issue as a bug that needs to be resolved correctly.

    That is my 10 cents worth and I leave it in your hands!

    Regards,
    Graham

     
  • wek
    wek
    2007-05-22

    Logged In: YES
    user_id=1201677
    Originator: NO

    Graham,

    You wrote:
    > A synopsis of it is:
    > "Labels within inline assembler can consist of alphanumeric characters
    > EXCEPT if the inline assembler is within a C switch/case statement where
    > they MUST comply with the form nnnn$, and if not then the switch/case
    > statement cannot be compiled."

    Not quite so. If the inline asm is within ANY C-code which would like to pass control crossing the inline asm part (e.g. if the inline asm is in a cycle), a non-xxx$ label would break the local labels of the C-code and the compile would fail. It is hard to predict ALL cases where this could happen and establish an easy rule when using non-xxx$ labels in inline asm IS absolutely safe.

    On the other hand, the ability of using labels in inline asm is only using (or abusing) an existing facility in the assembler used by SDCC. On the other hand, to be able to use ANY labels in the inline asm (or almost any, with only a few restrictions), the compiler, the assembler, or both would have to be patched. I don't think the developers would see this as a priority, given other tasks to do :-(

    However, you might consider submitting the same as a Feature Request... (that's my SKK 0.02, practically zero)

    JW

     
  • Borut Ražem
    Borut Ražem
    2007-05-22

    Logged In: YES
    user_id=568035
    Originator: NO

    I agree with Graham that we have an ugly inconsistency in the implementation of labels and also agree with Jan that it is not the highest priority task to correct the implementation, at least not for 2.7.0 release.

    I reopened the issue and recategorized it to a feature request.

    Frieder and Jan: can you please review the chapter 3.12.3 in sdccman once more and specify more clearly when to use which type of labels, if possible before the 2.7.0 release?

    Borut

     
  • Borut Ražem
    Borut Ražem
    2007-05-22

    • labels: 101550 -->
    • milestone: 100455 -->
    • status: closed-rejected --> open-rejected
     
  • wek
    wek
    2007-05-22

    Logged In: YES
    user_id=1201677
    Originator: NO

    Borut,

    My wording in the said chapter is such, that the "in older version" is omitted, thus _requiring_ the xxxx$ form of labels in inline asm. This is always safe (with xxxx < 100). Then there is a footnoote explaining partially the background.

    IIRC Frieder's concern was, that the footnote is misleading - Frieder will kindly correct me if I am wrong (I don't have the record of our conversation here now).

    ---

    If this gets ever patched - and I agree it's not worth doing it for 2.7.0 - both the compiler and as has to be modified: the compiler has to insert some explicit "begin local labels" and "end local labels" tags; and as has to be modified to confine the "locality" of xxxx$-form labels within these explicit tags, rather than within any other general labels. I don't say it's complicated, but still I don't have established the toolchain to accomplish it, with proper testing.

    JW

     
    • status: open-rejected --> open
     
  • Logged In: YES
    user_id=589052
    Originator: NO

    Greetings,
    > IIRC Frieder's concern was, that the footnote is misleading

    Yes, that was about it. Additionally I was at first not aware
    there was a difference between the human readable ':' labels
    and the 'nnnnn$:' labels (Jan pointed that out to me).
    So I feel I cannot give an authoritative review for that section.

    Like Graham I would not like to loose (or undocument/discourage)
    the human readable ':' labels though. This kind of labels
    is in wide-spread use within library code
    (f.e. device/lib/expf.c printf_tiny.c or _gptrget.c).

    Greetings,
    Frieder

     
  • wek
    wek
    2007-05-22

    Logged In: YES
    user_id=1201677
    Originator: NO

    The truth is, you CAN use non-xxxx$-form labels in inline asm as long as thi does not get into conflict with the compiler's xxxx$-form labels ("break"/"split" their "locality") - and in case of such conflict, the user wil unambigouosly know it did happen (the abovementioned error will happen and no hex will be output).

    But, shall we write in the manual, "use labels in asm at your will, but if error occurs, resort to the xxx$-form labels only"? I don't think so.

    JW

    PS. As an additional caveat, with free-form labels in inline asm, nasty things can be done, like jumping directly from one funcion to another... OK this cannot be prevented altogether anyway, you CAN do it now, too, if you want, in one form or other... this was just a (super)cautious note...

     
  • Maarten Brock
    Maarten Brock
    2007-05-22

    Logged In: YES
    user_id=888171
    Originator: NO

    "Additionally I was at first not aware there was a difference between the human readable ':' labels and the 'nnnnn$:' labels (Jan pointed that out to me)."

    Hmm, this is odd. Because it was you Frieder who changed the manual saying that it was "older compiler version" behaviour. See #2920.

    Where is this patch? Can we all have a look at it?

    Maarten

     
  • Logged In: YES
    user_id=589052
    Originator: NO

    > Hmm, this is odd. Because it was you Frieder who changed the manual saying
    > that it was "older compiler version" behaviour. See #2920.
    > Where is this patch? Can we all have a look at it?

    There is no patch.

    Sorry if I substituted worse for bad in:
    http://svn.sourceforge.net/viewvc/sdcc/trunk/sdcc/doc/sdccman.lyx?r1=2859&r2=2920
    line 9940..9943

    At the risk of repeating myself:
    "I feel I cannot give an authoritative review for that section."

    Proceed as you wish.

     
  • Maarten Brock
    Maarten Brock
    2007-05-23

    Logged In: YES
    user_id=888171
    Originator: NO

    Ok, I found patch 1697136 which contains this footnote. Apart from some spelling errors I think it is correct. My advice is to put it in and remove the "older version" statement.

    It resembles nooverlay with functions called from ISR's. You don't have to supply it if you already know there will be no overlaying or if you check yourself it doesn't overlay with stuff whose access can be interrupted. Nonetheless the recommendation is to use nooverlay on functions called from ISR's.

    Using asm one can always do dangerous things. Using non-local asm labels in inline-asm is one of them. The best we can do is warn, explain and recommend.