Clozure / ccl

Clozure Common Lisp
http://ccl.clozure.com
Apache License 2.0
855 stars 103 forks source link

Bug in make-array #332

Closed mgraffam closed 4 years ago

mgraffam commented 4 years ago

(length (setf FOO (make-array (* 4096 4096) :initial-element 0))) 0

Playing around this border gives more weird results .. (length (setf FOO (make-array 16778120 :initial-element 0))) 904

The debugger is not invoked.

This happens on Linux/ARM for both v1.12 and v1.11.5. It does not happen on x86-64, for either MacOS or Linux. Nor does it happen on other CL's for ARM32 (SBCL, CLISP) - so it does not appear to be an architecture issue. And anyhow, 4096^2 is a far cry from 2^32

CPUinfo: processor : 7 model name : ARMv7 Processor rev 3 (v7l) BogoMIPS : 120.00 Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae CPU implementer : 0x41 CPU architecture: 7 CPU variant : 0x2 CPU part : 0xc0f CPU revision : 3

Hardware : ODROID-XU4 Revision : 0100 Serial : 0000000000000000

Linux odroid-1 4.14.180-176 #1 SMP PREEMPT Tue May 19 00:40:55 -03 2020 armv7l armv7l armv7l GNU/Linux

svspire commented 4 years ago

array-total-size-limit on 32-bit architectures in CCL is 2^24. It's probably going to be less than 2^32 on any 32-bit CL implementation because of the need for tag bits in the array header, and there are 8 in CCL. Your first form probably should have worked because it's exactly 2^24. Your second form should have thrown an error because it's trying to make an array that's slightly too big.

My ARM machines are not readily available so I can't test this but it looks fishy.

phoe commented 4 years ago

On 01.07.2020 05:55, Shannon Spires wrote:

Your first form probably should have worked because it's exactly 2^24.

Off by one error - the ranges are actually from 0 to 2^24 - 1. See CCL32 x86:

? (length (make-array (1- (expt 2 24))))    
16777215
? (length (make-array (expt 2 24)))
Error: Cannot allocate a SIMPLE-VECTOR with 16777216 elements.
       Objects of type SIMPLE-VECTOR can can have at most
       16777215 elements in this implementation.
While executing: CCL::MAKE-UARRAY-1, in process listener(1).
Type :POP to abort, :R for a list of available restarts.
Type :? for other options.
phoe commented 4 years ago

This behavior is correct because ARRAY-TOTAL-SIZE-LIMIT is defined to be

The upper exclusive bound on the array total size of an array.

And so it is illegal to (make-array array-total-size-limit).

So, not a bug in CCL, I guess. A technical limitation, sure, but nothing that breaks standards conformance.

mgraffam commented 4 years ago

This behavior is correct because ARRAY-TOTAL-SIZE-LIMIT is defined to be

The upper exclusive bound on the array total size of an array.

And so it is illegal to (make-array array-total-size-limit).

So, not a bug in CCL, I guess. A technical limitation, sure, but nothing that breaks standards conformance.

I disagree. If CCL can't make an array greater than array-total-size-limit that is 100% fine.

The bug is not throwing an error, and instead returning an under-sized array as if nothing happened.

mgraffam commented 4 years ago

Clozure Common Lisp Version 1.11.5/v1.11.5 (LinuxARM32)

For more information about CCL, please see http://ccl.clozure.com.

CCL is free software. It is distributed under the terms of the Apache Licence, Version 2.0. ? (length (make-array (+ 1 (expt 2 24))))
1 ?

phoe commented 4 years ago

Yes, I understand now; it's certainly concerning that CCL signals an error on x86 but does not signal that same error on arm32. And that is contrary to user expectations and additionally differs between platforms, so I agree that it's a bug from the practical point of view.

(If we take a strict formalist approach, CLHS MAKE-ARRAY does not specify any situations where make-array must return an error. In the text, dimensions is specified to be a valid array dimension or a list of valid array dimensions, and therefore specifying an invalid array dimension invokes undefined behavior. Hence, there is no ANSI conformance issue on CCL side in its current behavior.)

phoe commented 4 years ago

Just to verify - did you try the same on a more recent version, e.g. 1.12?

mgraffam commented 4 years ago

Just to verify - did you try the same on a more recent version, e.g. 1.12?

Yes, same behavior on 1.12.

Clozure Common Lisp Version 1.12 (v1.12) LinuxARM32

For more information about CCL, please see http://ccl.clozure.com.

CCL is free software. It is distributed under the terms of the Apache Licence, Version 2.0. ? (length (make-array (+ 1 (expt 2 24)))) 1 ?

svspire commented 4 years ago

These are not merely under-sized arrays. These are wraparound errors in the 24-bit field that specifies the length of the array. Two things need to be checked:

  1. Why isn't this wraparound error being caught at make-array time?
  2. How much--if any--memory is being allocated for this array? Possibly the requested amount, and if so, it's not accessible, and furthermore, it's likely not GC-able in which case it's a memory leak.
phoe commented 4 years ago

1) is the real question for me; 2) should be checkable in any kind of process manager, such as top or htop by monitoring the memory usage of the ccl process before and after the make-array call.

mgraffam commented 4 years ago

Seems like there may be a memory leak.

I created a number of arrays sized 2^24+1, then removed my reference to them, did this repeatedly. It hit 40% memory usage, then called (gc) .. armcl's memory usage creeped from 1.1% at cold startup to 3.1% after playing around.

xrme commented 4 years ago

Fixed in 67c237885b. Also cherry-picked to the 1.11 branch in b7124985d5f.

SPmisc_alloc was doing the check for the maximum element count, but it was doing it wrong. When that was fixed, I found that it was also calling SPksignalerr incorrectly.

To pick up the fix, pull and rebuild the lisp kernel (via the usual cd lisp-kernel/linuxarm; make clean; make).