[comp.os.minix] c68 has a switch bug

joerg@mwtech.UUCP (Joerg Werner) (02/27/91)

I use c68 all the time and I like it. But after I compiled the
complete Minix-ST 1.5 operating system, mdb (the Minix debugger)
doesn't work any longer.

Please have a look at the following programm:

testswitch(i)
{
  printf("%2d ", i);
  switch(i) {	/* such a switch occurs in kernel/system.c do_trace() */
    case -1:
    case  1:
    case  2:
    case  3:
    case  4:
    case  5:
    case  6:
    case  7:
    case  9: printf(":-)\n"); break;
    default: printf(":-(\n"); break;
  }
}

main()
{
  testswitch(-1);
}

You and I expect that the result would be

-1 :-)

but the result is

-1 :-(

(BTW: ACK handles this right!)

Is this problem known and is there a fix?

Ciao, Joerg

PS: Which programs must be compiled again, if the process-struct in the
    kernel is changed? The well-known ones are `ps' and `mdb'. Any others?
-- 
Joerg Werner, email: joerg@mwtech.UUCP, voice: 49-(0)6151-37 33 66

wahlsten@elixir.lne.kth.se (Jorgen Wahlsten) (03/01/91)

In article <1087@mwtech.UUCP> joerg@mwtech.UUCP (Joerg Werner) writes:
>
>I use c68 all the time and I like it. But after I compiled the
>complete Minix-ST 1.5 operating system, mdb (the Minix debugger)
>doesn't work any longer.
>
>Please have a look at the following programm:
> [...]

The bug is in genstmt.c when generating code for the 'switch'-statement.
The expresseion in switch(expr) is assumed to be unsigned (somehow), so
when you have an short int, 'i' in your example c68 never cast'ed it into a
long before looking up the value (it became 65535L instead of -1).
This only happens when you use more than 7 case-statements, that is, when
c68 generates a long-table with addresses instead of cmp/jmp...

I've made a modif. that (i think) work but i haven't tested it that much,
and i'm not sure my modification respects all there is (Better have CvW
confirm). But for a quick and easy change, use the diff that follows...

Regards,
/wahlsten@lne.kth.se

output from 'diff genstmt.c genstmt.c.org':
---------- Cut here! ----------
362d361
<     enum e_bt	    ctyp;
377d375
<     ctyp = stmt->exp->etype;
411d408
< 	if (size == 1 || size == 2)
413,419d409
< 	    ap = g_cast(ap, ctyp, bt_long, F_DREG | F_VOL);
< #endif
< #ifdef INTEL_386
< 	    ap = g_cast(ap, ctyp, bt_long, F_REG);
< #endif
< /*
< #ifdef MC680X0
431d420
< */

HBO043%DJUKFA11.BITNET@cunyvm.cuny.edu (Christoph van Wuellen) (03/03/91)

Well, this bug report urged me to ship out the last posting.
I have hesitated shipping out patch1 since I was busy and since I got
some bug reports. I have tried to send fixes to those who sent bug
reports, but this one was the first to go over the net...

Your solution (calling g_cast) is potentially weak.
Although it works at the moment, you cannot control the bahviour of
g_cast forever. It is dangerous to call g_cast
from a totally different module (normaly I would say it is only legal to
call g_expr, g_cast may well become static one day).

My solution was to do a swith statement here and to handle the types
char, unsigned char, short, and unsigned short specially.

C.v.W.