DavidKinder / Git

Fast Glulx interpreter, originally written by Iain Merrick.
MIT License
46 stars 13 forks source link

Undefined behavior when dividing -0x80000000/-1 #24

Closed dfoxfranke closed 2 months ago

dfoxfranke commented 2 months ago

Git translates the Glulx div instruction as a C / without checking the case of -0x80000000/-1. According to C17 §6.5.5.6, this is undefined behavior:

When integers are divided, the result of the / operator is the algebraic quotient with any fractional part discarded. If the quotient a/b is representable, the expression (a/b)*b + a%b shall equal a; otherwise, the behavior of both a/b and a%b is undefined.

On my particular compiler and machine, Git dumps core. I've opened an issue against the Glulx spec calling for it to define whether the correct result for this case is -0x80000000 (as in Glulxe and Quixe) or whether it's a fatal error. But whatever is decided, Git should handle it explicitly.

erkyrath commented 2 months ago

Let's say this is an error.

DavidKinder commented 2 months ago

Test case:

Global gg_mainwin = 0;

[ Main a x;
    @setiosys 2 0;
    gg_mainwin = glk($0023, 0, 0, 0, 3, 201); ! window_open
    glk($002F, gg_mainwin); ! set_window

    a = -$80000000;
    @div a 1 x;
    print  x, "^";
    @div a (-1) x;
    print  x, "^";
];
DavidKinder commented 2 months ago

Fixed in commits 5ae06ca59375c9e12c617d7087651753ff9dadbe and 9184bbb466c31997076b083c4f6da583b042943c.