Jass real/float literals parsing bug

[oephun]

'real' literals are sometimes parsed incorrectly:

function testJassRealLiteralParsing takes nothing returns nothing
  local integer a
  local real array xs
  set xs[0] = 0.03           // 0.030000000
  set xs[1] = 0.033          // 0.032999998
  set xs[2] = 0.0333         // 0.033300000
  set xs[3] = 0.03333        // 0.033330004
  set xs[4] = 0.033333       // 0.033333002
  set xs[5] = 0.0333333      // 0.033333300
  set xs[6] = 0.03333333     // 0.033333330
  set xs[7] = 0.033333333    // 0.033333330
  set xs[8] = 0.0333333333   // 0.236395648
  set xs[9] = 0.03333333333  // -0.790978688
  set a = 0 - 1
  loop
    set a = a + 1
    exitwhen 10 == a
    call BJDebugMsg(R2SW(xs[a], 1, 9))
  endloop
endfunction

The values of 'xs[8]' and 'xs[9]' are very strange. @LeP, it would be nice if pjass could detect and warn about these "bad" literals.
I ran into this with '1.0f / 30.0f' getting translated to '0.0333333333' in cjc.

 

[LeP]

I would gladly add it to pjass but do you have a rule on how to spot these bad literals?

 

[oephun]

It seems that 'float/real' literals in jass are parsed using a function similar to the 'jassParseRealLiteral' below. It parses the integer and fractional parts of the literal as integers, converts the fractional part to a float, converts the integer part to a float and then finally adds them. Overflows can happen when parsing the integers and when the fractional part is converted to a float (divided by a power of 10).

#include <stdio.h>
#include <assert.h>

#define ARRAY_LEN(x) ((int)(sizeof(x) / sizeof(x[0])))

// assumes 'p' points to a '\0'-terminated string of a 'real' literal: [0-9]* '.'? [0-9]*
static float jassParseRealLiteral(char const* p) {
  if ('\0' == *p) { return 0.0f; }

  int int_ = 0;
  for (;; ++p) {
    if ('.' == *p) { break; }
    if ('\0' == *p) { return (float)int_; }
    int_ = 10 * int_ + ((int)*p - '0');
  }

  int frac = 0;
  int fracPow10 = 1;
  assert('.' == *p); ++p;
  for (;; ++p) {
    if ('\0' == *p) { break; }
    frac = 10 * frac + ((int)*p - '0');
    fracPow10 *= 10;
  }

  float fracf = (float)frac / (float)fracPow10;
  float int_f = (float)int_;
  return int_f + fracf;
}

// *a += b;
// returns 1 if '*a' overflows
static int addCheckOverflow(int* a, int b) {
  if (b > 0x7FFFFFFF - *a) { return 1; }
  *a += b;
  return 0;
}

// *a *= b;
// returns 1 if '*a' overflows
static int mulCheckOverflow(int* a, int b) {
  if (*a > 0 && b > 0x7FFFFFFF / *a) { return 1; }
  *a *= b;
  return 0;
}

static int isBadJassRealLiteral(char const* p) {
  if ('\0' == *p) { return 0; }

  int int_ = 0;
  for (;; ++p) {
    if ('.' == *p) { break; }
    if ('\0' == *p) { return 0; }
    if (mulCheckOverflow(&int_, 10)) { return 1; }
    if (addCheckOverflow(&int_, (int)*p - '0')) { return 1; }
  }

  int frac = 0;
  int fracPow10 = 1;
  int nfrac = 0;
  assert('.' == *p); ++p;
  for (;; ++p) {
    if ('\0' == *p) { break; }
    ++nfrac;
    if (mulCheckOverflow(&frac, 10)) { return 1; }
    if (addCheckOverflow(&frac, (int)*p - '0')) { return 1; }
    if (mulCheckOverflow(&fracPow10, 10)) {
      // allow: x.00000000000000000000000000000000 (up to 31 zeroes)
      if (0 != frac) { return 1; }
      if (32 == nfrac) { return 1; }
    }
  }

  return 0;
}

static void testJassParseRealLiteral(void) {
  char const* xs[] = {
    "0.03",
    "0.033",
    "0.0333",
    "0.03333",
    "0.033333",
    "0.0333333",
    "0.03333333",
    "0.033333333",
    "0.0333333333",
    "0.03333333333",
    "1.00000000000000000000000000000000",
  };
  for (int a = 0; a < ARRAY_LEN(xs); ++a) {
    char const* x = xs[a];
    printf("%1.9f (%s), is bad?: %d\n",
      jassParseRealLiteral(x), x, isBadJassRealLiteral(x)
    );
  }
}

int main(void) {
  testJassParseRealLiteral();
  return 0;
}

 

[LeP]

Thank you very much. Added it. Basically took your code and also added an int overflow check. It's hidden behind the +checknumberliterals flag though, because it's still technically valid jass.

 

[oephun]

Well, that was fast :^)! Thanks, it already caught a bug in 'cjc' emitting '-18446744071562067968' instead of '-2147483648' for '-2147483647 - 1'.

 

[Prometheus3375]

Yeah, I reported this back in 2018, it is really cool that pjass can now detect this!