Mantis Bugtracker
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  SignedIntFixes.3.cs [^] (5,150 bytes) 03-22-08 05:22
 FFITestCase.st [^] (2,441 bytes) 11-26-08 06:21
 mathfixforlowestlongandlonglong.1.cs [^] (4,571 bytes) 11-26-08 06:57
 Int32Fixes-dtl-M6987.1.cs [^] (2,548 bytes) 03-08-09 16:56
 Int64Fixes-dtl-M6987.2.cs [^] (2,741 bytes) 03-10-09 22:12
 IntegerArrayTestCase-dtl-M6987.8.cs [^] (35,067 bytes) 03-16-09 10:58
 FFI-primitiveFFIIntegerAt-dtl-M6987-M7237.1.cs [^] (2,773 bytes) 04-03-09 02:35
 Interpreter-signed32BitIntegerFor-dtl-M6987.1.cs [^] (1,678 bytes) 04-12-09 16:46

- Relationships

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

related to 0007240closed lewis Fix #primitiveIntegerAt and #primitiveIntegerAtPut for 64-bit object memory 
related to 0007237acknowledged lewis Make FFI work on 64 bit platforms 
related to 0007069closed tim FFIPlugin needs to make proper use of signed32BitValueOf: 
related to 0007309closed lewis IntegerArray>>at:put: needs range check in prim fallback code 
related to 0007705testing lewis Three bugs in LargeInteger primitives 

- Notes
(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:
 

- Issue History
Date Modified Username Field Change
03-22-08 05:22 andreas New Issue
03-22-08 05:22 andreas Status new => assigned
03-22-08 05:22 andreas Assigned To  => tim
03-22-08 05:22 andreas File Added: SignedIntFixes.3.cs
05-27-08 17:50 tim Note Added: 0012182
05-27-08 17:51 tim Status assigned => resolved
05-27-08 17:51 tim Resolution open => fixed
05-27-08 18:00 andreas Relationship added related to 0007069
11-26-08 06:20 johnmci Note Added: 0012810
11-26-08 06:20 johnmci Assigned To tim => andreas
11-26-08 06:20 johnmci Status resolved => assigned
11-26-08 06:21 johnmci File Added: FFITestCase.st
11-26-08 06:21 johnmci Assigned To andreas => johnmci
11-26-08 06:22 johnmci Note Added: 0012811
11-26-08 06:32 johnmci Resolution fixed => open
11-26-08 06:57 johnmci File Added: mathfixforlowestlongandlonglong.1.cs
11-26-08 06:59 johnmci Note Added: 0012812
11-26-08 19:36 lewis Issue Monitored: lewis
11-28-08 22:48 lewis Note Added: 0012816
12-04-08 04:22 lewis Relationship added related to 0007240
12-04-08 04:39 lewis File Added: IntegerArrayTestCase-dtl.3.cs
12-04-08 04:40 lewis File Added: SignedIntFixesPatch-dtl.3.cs
12-04-08 04:46 lewis Note Added: 0012828
12-04-08 05:05 lewis File Added: SignedIntFixesPatch-dtl.4.cs
12-04-08 05:05 lewis File Deleted: SignedIntFixesPatch-dtl.3.cs
12-04-08 05:06 lewis Note Edited: 0012828
03-08-09 16:45 lewis Relationship added related to 0007309
03-08-09 16:55 lewis File Added: IntegerArrayTestCase-dtl-M6987.1.cs
03-08-09 16:56 lewis File Added: Int32Fixes-dtl-M6987.1.cs
03-08-09 17:09 lewis Note Added: 0013028
03-08-09 17:09 lewis File Deleted: IntegerArrayTestCase-dtl.3.cs
03-08-09 19:16 lewis File Added: IntegerArrayTestCase-dtl-M6987.3.cs
03-08-09 19:16 lewis File Deleted: IntegerArrayTestCase-dtl-M6987.1.cs
03-08-09 19:18 lewis Note Edited: 0013028
03-10-09 02:34 lewis File Added: Int64Fixes-dtl-M6987.1.cs
03-10-09 02:37 lewis File Added: IntegerArrayTestCase-dtl-M6987.6.cs
03-10-09 02:40 lewis Note Added: 0013031
03-10-09 02:40 lewis File Deleted: SignedIntFixesPatch-dtl.4.cs
03-10-09 02:40 lewis File Deleted: IntegerArrayTestCase-dtl-M6987.3.cs
03-10-09 02:49 lewis Note Added: 0013032
03-10-09 22:12 lewis File Added: Int64Fixes-dtl-M6987.2.cs
03-10-09 22:15 lewis Note Added: 0013035
03-10-09 22:16 lewis File Deleted: Int64Fixes-dtl-M6987.1.cs
03-13-09 02:19 lewis Note Added: 0013037
03-16-09 10:58 lewis File Added: IntegerArrayTestCase-dtl-M6987.8.cs
03-16-09 10:59 lewis File Deleted: IntegerArrayTestCase-dtl-M6987.6.cs
04-03-09 02:35 lewis File Added: FFI-primitiveFFIIntegerAt-dtl-M6987-M7237.1.cs
04-03-09 02:42 lewis Note Added: 0013073
04-03-09 02:43 lewis Relationship added related to 0007237
04-12-09 16:46 lewis File Added: Interpreter-signed32BitIntegerFor-dtl-M6987.1.cs
04-12-09 16:56 lewis Note Added: 0013081
07-23-11 15:32 lewis Assigned To johnmci => lewis
07-27-11 12:56 lewis Note Added: 0014167
08-29-12 22:59 lewis Relationship added related to 0007705

Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
168 total queries executed.
68 unique queries executed.
Powered by Mantis Bugtracker