464773 - Regexp bug (NUL not allowed in regexp) causes sunspider crypto-aes performance test to return incorrect results

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P3)

Description

[Dan Schaffer]

Attachment: slightly modified crypto-aes source to output decrypted text

The crypto-aes algorithm takes a plain text string encrypts it, then decrypts it.

both the untyped, typed versions shows some invalid characters in the output stream.  The encryption seeds some randomness by using the current time.  Here is any example of the incorrect result.

Running the untyped code in a browser as javascript produces the correct result.  Is there something in the string implementation or perhaps a numerical computation round off error. 

plain text section:
ROMEO: But, soft! what light through yonder window breaks?
erroneous result:
ROMEO: But, soft! whÈÃ
                      æñ¥!ùough yonder window break

Comment 1

[Dan Schaffer]

ran into this problem when making sunspider tests self checking.  logging a separate bug since this I was not able to figure out the problem after spending like an hour.  will require more work.

Comment 2

[Trevor Baker]

Lars, can you look into this?  thanks

Comment 3

[Lars T Hansen]

I think the benchmark is broken.  I've downloaded an alternative AES implementation from http://www.movable-type.co.uk/scripts/aes.html (the code is very similar, the two programs are likely related by authorship) and it encrypts and decrypts the text correctly.

Comment 4

[Lars T Hansen]

Confirmed, the crypto-aes benchmark is based on older code, available at http://www.movable-type.co.uk/scripts/aes-old.html, and that code does not roundtrip encryption/decryption correctly.

Comment 5

[Lars T Hansen]

And yet... when run in Firefox, crypto-aes.as runs as it should.  So there is some trickery in the older code that AVM fails on, that's absent from the newer code.  I'll keep the bug.

Comment 6

[Lars T Hansen]

This is a regular expression bug.  There are two functions, escCtrlChars and unescCtrlChars, that use a regular expression with a function argument to escape and unescape problematic characters.  When these are replaced by hand-written routines with identical functionality, the encrypt/decrypt roundtrip works.  It appears that it is the regexp in escCtrlChars that is the culprit, and the reason that is not working is because "\0" is one of the characters in the regular expression.  Whether this is a problem with PCRE or the path down to PCRE I don't know yet.

Comment 7

[Lars T Hansen]

Apparently it's a known problem that PCRE depends on NUL-terminated strings.  ASC plays along, it emits [\0\t...] as the regex literal in the ABC file, ie, using an escape sequence, not a NUL character.  Presumably PCRE translates this into a NUL and subsequently fails to deal with it.

Thus it may be that the best way to fix this is to use my regexp-free rewrite, which I will attach.

Rerouting back to QE in case we have a master bug for this, and to make a decision about what we should do about the benchmark.

Comment 8

[Lars T Hansen]

Attachment: Rewritten escape / unescape functions

Comment 9

[Chris Peyer]

Filed bug 465816 to track the pcre issue.

Comment 10

[Chris Peyer]

Pushed patch to TR - changeset: 1126:5602dbf59708