119314 - JS assertion in jsdtoa.c when built with CodeWarrior Pro 7.1 compilers

Status: VERIFIED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Simon Fraser [no longer active]]

There is a compiler error in CodeWarrior Pro 7.1 that causes an assertion in
JavaScript engine code at:

Assertion failure: b->wds <= n, at jsdtoa.c:1734

which happens when doing float->string conversions.

Here's the bug report I posted to the codewarrior newsgroup:

We're seeing a new compiler bug in the Pro 7.1 compilers
(CFM) that is not present in 7.0, and happens even when all
optimizations are disabled.

It's an incorrect sign usage when a signed integer is cast to an
unsigned long long and multiplied.

Sample code:

#include <MacTypes.h>

int main(void)
{
  SInt32    m = 10;  /* note signed! */
  UInt32    x = 2;
  UInt64    y = x * (UInt64)m;
  return 0;
}

Pro 7.0 disassembly:

int main(void)
{
00000000: 93E1FFFC  stw        r31,-4(SP)
00000004: 93C1FFF8  stw        r30,-8(SP)
  SInt32    m = 10;
00000008: 3BE0000A  li         r31,10
  UInt32    x = 2;
0000000C: 3BC00002  li         r30,2
  UInt64    y = x * (UInt64)m;
00000010: 7C7EF9D6  mullw      r3,r30,r31
00000014: 7C1EF816  mulhwu     r0,r30,r31   /* here /*
00000018: 9061FFF4  stw        r3,-12(SP)
0000001C: 9001FFF0  stw        r0,-16(SP)
  return 0;
00000020: 38600000  li         r3,0
00000024: 83E1FFFC  lwz        r31,-4(SP)
00000028: 83C1FFF8  lwz        r30,-8(SP)
0000002C: 4E800020  blr
}


Pro 7.1 disassembly:

int main(void)
{
00000000: 93E1FFFC  stw        r31,-4(SP)
00000004: 93C1FFF8  stw        r30,-8(SP)
  SInt32    m = 10;
00000008: 3BE0000A  li         r31,10
  UInt32    x = 2;
0000000C: 3BC00002  li         r30,2
  UInt64    y = x * (UInt64)m;
00000010: 7C7EF9D6  mullw      r3,r30,r31
00000014: 7C1EF896  mulhw      r0,r30,r31  /* bad! */
00000018: 9061FFF4  stw        r3,-12(SP)
0000001C: 9001FFF0  stw        r0,-16(SP)
   return 0;
00000020: 38600000  li         r3,0
00000024: 83E1FFFC  lwz        r31,-4(SP)
00000028: 83C1FFF8  lwz        r30,-8(SP)
0000002C: 4E800020  blr
}


If I were to hazard a guess, I'd say that this might be
a regression from the following fix noted in the 7.1 release
notes:

WB1-27591     Left shift of a signed long into a signed long long needs
              to sign extend the high part part of the result

Comment 1

[Simon Fraser [no longer active]]

Note that we're *NOT* builing with CW Pro 7.1 currently, so this problem does
not affect mainstream builds. But we'd like to move to 7.1 for its better
debugging support.

Comment 2

[Phil Schwartau]

Reassigning to Kenton. I'm a little unclear - is this bug for information
purposes only? Or must something be changed in our code to work around
the bug in CW 7.1? In the latter case, would it be worth waiting to see
if the CW people come up with a fix before we change anything?  

Comment 3

[Simon Fraser [no longer active]]

It's for informational purposes only. Consider it a blocker for moving Mac 
developers to Pro 7.1

Comment 4

[Phil Schwartau]

OK, thanks - cc'ing Waldemar, Steve on this

Comment 5

[Waldemar Horwat]

Did someone file a Metrowerks bug on this?  I'm not on MPTP any more for some
reason.

Comment 6

[Simon Fraser [no longer active]]

I posted on codewarrior.mac. They seem to pick stuff up from there.

Comment 7

[Jonathan Brecher]

Isn't there a real logic bug here, regardless of the compiler issues?  The code 
at this point reads like this:

JS_ASSERT(b->wds <= n);
if (b->wds < n)
    return 0;

I don't know what the code is doing here, but most cases of an assert followed 
by an if/return, the comparisons are the opposite in both cases.  I would have 
expected to see >=,< or <=,> for the two comparisons rather than <=,< 

Comment 8

[Phil Schwartau]

cc'ing Brendan for the question raised in Comment #7; and
[e-mail] for his recent work on bug 120992

Comment 9

[Daniel Bratell]

No, it's fine. 

The function calculates b/S and returns the remainder in b. It first checks that
b doesn't uses more memory than S because that's a prerequisite of the function
and is mentioned in the comment for quorem. if b->wds > S-wds (called |n|), then
the function asserts. This is just a debug time check.

It then checks if b has less memory words than S (the variable |n|) and if
that's the case b/S is obviously 0 and the remainder is b, therefore the |return
0;|. It then goes on with the heavy calculations. 

Let's the bug go back to the CW7.1 compiler bug mode.

Comment 10

[Kenton Hanson (gone)]

Simon,
Have you seen any response from codewarrioe.mac?

Comment 11

[Simon Fraser [no longer active]]

The CodeWarrior 7.2 update fixes this. We're moving to it soon.

Comment 12

[timeless]

we moved to cw7.2.

Comment 13

[Phil Schwartau]

Marking Verified -