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

VERIFIED FIXED in flash10.1

Status

P3
normal
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: dschaffe, Assigned: trbaker)

Tracking

unspecified
flash10.1
x86
Mac OS X
Bug Flags:
wanted-flashplayer10 +
flashplayer-qrb +
flashplayer-triage +

Details

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Created attachment 348042 [details]
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
(Reporter)

Comment 1

10 years ago
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.
Flags: wanted-flashplayer10+
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
(Assignee)

Comment 2

10 years ago
Lars, can you look into this?  thanks
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Flags: flashplayer-qrb?
Priority: -- → P3
Target Milestone: --- → flash10.x

Comment 3

10 years ago
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

10 years ago
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

10 years ago
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

10 years ago
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.
Summary: sunspider crypto-aes performance test returns incorrect results → Regexp bug (NUL not allowed in regexp) causes sunspider crypto-aes performance test to return incorrect results

Comment 7

10 years ago
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.
Assignee: lhansen → trbaker

Comment 8

10 years ago
Created attachment 348786 [details]
Rewritten escape / unescape functions

Comment 9

10 years ago
Filed bug 465816 to track the pcre issue.

Comment 10

10 years ago
Pushed patch to TR - changeset: 1126:5602dbf59708
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Flags: flashplayer-qrb+

Updated

10 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.