atom.h:155: #ifdef AVMPLUS_64BIT // in 64-bit builds, integer atoms can hold a 53-bit signed value. // (this is so that integer values can be interchanged with doubles // with no loss of precision) const intptr_t atomMinIntValue = -(1LL<<53); const intptr_t atomMaxIntValue = (1LL<<53)-1; #else // in 32-bit builds, integer atoms can hold a 29-bit signed value. const intptr_t atomMinIntValue = -(1L<<29); const intptr_t atomMaxIntValue = (1L<<29)-1; #endif The 32-bit values are incorrect. It is correct that at atom can hold a 29-bit signed value, but the range is then -2^28..2^28-1, while 1L<<29 is 2^29. The 64-bit values are apparently correct, but only because the double representation is sign+magnitude so we technically have a 54-bit signed number (as the values reveal). atomMinIntValue is not used anywhere in the VM. atomMaxIntValue is only used in ::atomIsValidIntptrValue_u(), where it is shifted in such a way that the erroneous value doesn't matter, but it's not quite clear to me why that's going on except possibly to work around the problem with the value of the constant being too large. Are these constants used externally to the VM, ie, is this a public API?
Good catch. AFAIK these values are not used (or intended to be used) external to the VM itself. I'll do some grepping to verify. In the meantime, let's fix the values here (and add your comments expanding about how the values are derived).
Priority: -- → P3
Target Milestone: --- → flash10.2
Neither name is referenced in the player source code (frr-argo).
Another bug here is the shift used for 64-bit values in atomIsValidIntptrValue, it should be 10, not 8. (The function operates on untagged values, which have 54 bits of precision including the sign.)
Created attachment 437966 [details] [diff] [review] Patch
Attachment #437966 - Flags: review?(stejohns)
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
There is a tiny bug here in the 64-bit case because a sign+magnitude representation has a range from -(2^(n-1)-1) to 2(n-1)-1. The bug is not important because that value is not currently being used in the code, it is there for completeness.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.