Anonymous | Login | 02-28-2021 21:55 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ View Advanced ] [ Issue History ] [ Print ] | |||||||||||
ID | Category | Severity | Reproducibility | Date Submitted | Last Update | |||||||
0006987 | [Squeak] VM | major | always | 03-22-08 05:22 | 07-27-11 12:56 | |||||||
Reporter | andreas | View Status | public | |||||||||
Assigned To | lewis | |||||||||||
Priority | normal | Resolution | open | |||||||||
Status | assigned | Product Version | ||||||||||
Summary | 0006987: signed32BitValueOf:, signed64BitValueOf: etc. broken | |||||||||||
Description |
The attached changes fix the following methods: * Interpreter>>signed32BitValueOf: * Interpreter>>signed64BitValueOf: * Interpreter>>signed64BitIntegerFor: * Interpreter>>positive64BitIntegerFor: All of the above are broken to varying extent. The first two fail to check for boundary conditions, i.e., if the integer has 32/64 bits of magnitude instead of 31+sign/63+sign. The latter two create non-normalized integers which will later fail to compare correctly. |
|||||||||||
Additional Information | ||||||||||||
Attached Files |
![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() |
|||||||||||
|
![]() |
||||||||||||||||||||||||||
SYSTEM WARNING: Creating default object from empty value SYSTEM WARNING: Creating default object from empty value SYSTEM WARNING: Creating default object from empty value SYSTEM WARNING: Creating default object from empty value
|
![]() |
|
(0012182 - 34 - 34 - 34 - 34 - 34 - 34) tim 05-27-08 17:50 |
Changes accepted in VMMaker-tpr.71 |
(0012810 - 356 - 441 - 601 - 601 - 601 - 601) johnmci 11-26-08 06:20 |
John notes: I was writing some Sunits for the Alien stuff and found alien signedLongAt: 1 put: -1*16r80000000. " -2147483648" failed. triggered by value < 0 ifTrue:[^self primitiveFail]. as coded below since value is -2147483648 range is Signed: −2,147,483,648 to +2,147,483,647 http://en.wikipedia.org/wiki/Integer_(computer_science) [^] |
(0012811 - 117 - 117 - 117 - 117 - 117 - 117) johnmci 11-26-08 06:22 |
Also note Alien has same tests for byte, short, long, *AND* long long. long long is NOT supported by FFI at this time |
(0012812 - 238 - 262 - 262 - 262 - 262 - 262) johnmci 11-26-08 06:59 |
Add mathfixforlowestlongandlonglong.1.cs which handles the case of −2,147,483,648 and −9,223,372,036,854,775,808 The existing code would fail for signed32BitValueOf: and both signed64BitIntegerFor: and signed64BitValueOf: |
(0012816 - 227 - 242 - 242 - 332 - 332 - 332) lewis 11-28-08 22:48 |
Not directly related to the problem at hand, but it's worth noting that the test case (FFITestCase.st) crashes the VM on a 64-bit machine. This is related to issue 0007237. (ByteArray new: 4) unsignedLongAt: 1 put: 7. ==> boom |
(0012828 - 587 - 647 - 647 - 647 - 647 - 647) lewis 12-04-08 04:46 edited on: 12-04-08 05:06 |
Files uploaded: IntegerArrayTestCase-dtl.3.cs - Additional tests to verify IntegerArray. Note, some tests will fail due to problems with primitive fallback code in IntegerArray, this should be the subject of a separate bug report. SignedIntFixesPatch-dtl.4.cs - Fix #signed32BitValueOf: and #signed64BitValueOf: to handle largest negative magnitudes properly. See also VmUpdates-1008-dtl.cs in Mantis 7240 required for 64-bit word size. Eliot has provided a number of additional fixes that go beyond these patches and may supercede them. These should also be uploaded here. |
(0013028 - 944 - 1004 - 1004 - 1004 - 1004 - 1004) lewis 03-08-09 17:09 edited on: 03-08-09 19:18 |
Uploaded IntegerArrayTestCase-dtl-M6987.3.cs and deleted the earlier IntegerArrayTestCase-dtl.3.cs version. The test case addresses 32 bit integer functions for IntegerArray, but does not cover 64 bit int arithmetic. It also covers a issue in the prim fallback code, see Mantis 7309. Uploaded Int32Fixes-dtl-M6987.1.cs which updates #signed32BitValueOf: based on Andreas' fix plus extra range checks to fail prim on invalid values, and adds type casts to #primitiveIntegerAt, required for 64 bit image. I do not currently have any tests to cover the 64 bit int cases. Disregard my earlier reference to Mantis 7240. That bug report was incorrect, and I included the necessary type cast in this update for #primitiveIntegerAt. With Int32Fixes-dtl-M6987.1.cs and IntegerArrayFallbackFix-dtl-M7309.3.cs both applied, the tests in IntegerArrayTestCase-dtl-M6987.3.cs are all green for both 32-bit and 64-bit images (on a 64 bit host). |
(0013031 - 173 - 185 - 185 - 185 - 185 - 185) lewis 03-10-09 02:40 |
Deleted SignedIntFixesPatch-dtl.4.cs (obsolete). Added Int64Fixes-dtl-M6987.1.cs. Added IntegerArrayTestCase-dtl-M6987.6.cs, replacing IntegerArrayTestCase-dtl-M6987.3.cs. |
(0013032 - 418 - 466 - 466 - 466 - 466 - 466) lewis 03-10-09 02:49 |
Summary: Int32Fixes-dtl-M6987.1.cs - Fixes for 32-bit integer operations. Int64Fixes-dtl-M6987.1.cs - Fixes for 64-bit integer operations. IntegerArrayFallbackFix-dtl-M7309.3.cs - Fix for primitive fallback code in IntegerArray. IntegerArrayTestCase-dtl-M6987.6.cs - Test case and supporting test primitives. Adds two primitives and modifies the #initializePrimitiveTable (use in a test image and discard). |
(0013035 - 342 - 367 - 367 - 367 - 367 - 367) lewis 03-10-09 22:15 |
I replaced Int64Fixes-dtl-M6987.1.cs with Int64Fixes-dtl-M6987.2.cs, which is a cleaner fix for #signed64BitIntegerFor:. The only problem with the original method from Andreas was that #magnitude had to be declared as unsigned in order for the "magnitude <= 16r7FFFFFFF" comparison to work properly when magnitude is 16r8000000000000000. |
(0013037 - 116 - 116 - 116 - 116 - 116 - 116) lewis 03-13-09 02:19 |
Int64Fixes-dtl-M6987.2.cs and Int32Fixes-dtl-M6987.1.cs are now included in VMMaker-dtl.117 in SqueakSource VMMaker. |
(0013073 - 483 - 483 - 483 - 483 - 483 - 483) lewis 04-03-09 02:42 |
One additional followup item: The FFITestCase.st tests identify an additional issue pertaining to short ints. This is an issue with #primitiveFFIIntegerAt, or more properly it is an issue with the memory access macro shortUnsignedAtPointer(). I have attached FFI-primitiveFFIIntegerAt-dtl-M6987-M7237.1.cs which provides a fix. The change set assumes implementation of the 64-bit FFI fixes in Mantis 7237, so this is best left as a separate follow up issue to the 64-bit FFI update. |
(0013081 - 607 - 643 - 643 - 643 - 643 - 643) lewis 04-12-09 16:56 |
Uploaded Interpreter-signed32BitIntegerFor-dtl-M6987, which is included in VMMaker-dtl.120 in SqueakSource VMMaker. This is a fix for #signed32BitIntegerFor: on 64-bit images. It adds a declaration for the method parameter as type int, such that the passed value is coerced from (possibly 64 bit) sqInt to int. This provides the correct behavior when used on a 64 bit image. This change set should be applied in addition to the changes in Int32Fixes-dtl-M6987. If the 64-bit FFI changes for M7237 are also applied, then the FFITestCase tests in M6987 are all green for 64-bit host and 64-bit image. |
(0014167 - 769 - 817 - 1019 - 1019 - 1019 - 1019) lewis 07-27-11 12:56 |
An additional issue identified by Eliot Miranda is discussed here: <http://lists.squeakfoundation.org/pipermail/vm-dev/2011-July/008905.html> [^] Summary: For signed int arithmetic, overflow produces undefined results (this to enable C compiler optimizations). Therefore the expression (0 - 16r8000000000) may not yield the expected result of 16r8000000000 when performed with signed integer arithmetic. Eliot provides a fix in oscog with explicit test for 16r8000000000 (test that shift left results in zero value). Dave suggests type cast to unsigned int to defeat compiler optimizations (but this needs to be tested with a problematic C compiler to verify that it works). Trunk VMMaker currently has this issue for #signed32BitValueOf: and #signed64BitValueOf: |
Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
168 total queries executed. 68 unique queries executed. |