On 01/06/14 23:25, Luís Oliveira wrote:
On Mon, Jan 6, 2014 at 9:06 PM, Massimiliano Ghilardi massimiliano.ghilardi@gmail.com wrote:
In such cases, it means the underlying C mmap() function returned successfully a pointer in the top half of the addressable virtual memory, but the Lisp wrapper for mmap() and mremap() chokes on such pointers.
A patch that solves this problem is attached.
Feedback on the analysis and the solution is welcome :)
First off, there's another bug: we should be using :intptr rather than :long for the return type. To be fair, CFFI probably didn't have that type yet when that code was written.
I did not know about :intptr. From the cffi/src/types.lisp sources it turns out to be "a signed int as wide as :pointer", so either :intptr or :uintptr are good for this case.
Unluckily, converting a negative :intptr to a :pointer still requires some trickery. My solution was (cffi-sys:make-pointer (logand address +most-positive-pointer+))) but it has the annoyance of computing +most-positive-pointer+ correctly.
On the other hand, your solution
The return-filter can then cast the return value to a pointer using something like:
(with-foreign-object (p :intptr) (setf (mem-ref p :intptr) <the-return-value>) (mem-ref p :pointer))
for each call it allocates (or seems to allocate) memory for the foreign-object P, writes an :intptr to it, only to read it back as a :pointer - a type-punning that even some C programmers frown upon.
Alternatively, we could store a -1 cast to pointer and compare the return value against that. (Hmm, maybe CFFI should have a cast operation. :-))
I was rather thinking about defining some constant as ((void *)-1) (which is actually the definition of MAP_FAILED, at least on some systems) and grovel it, but groveled constants can only be integers :(
So your suggestion to have a Lisp constant "map-failed cast to pointer" sounds nice.
This raises another interesting point: at least on my system, 64-bit Lisps (SBCL, CCL) grovel (cl:defconstant map-failed 18446744073709551615 "mmap: failure") while 32-bit ones (CMUCL, CCL) grovel (cl:defconstant map-failed -1 "mmap: failure")
Quite annoying.
I'm not sure any of these alternatives are better than your solution. What do you think?
The situation is much more "dirty" than I thought initially: 1) on Win64, "long" is smaller than "intptr" - see http://permalink.gmane.org/gmane.lisp.cffi.devel/2112
2) groveled map-failed sometimes is -1 and sometimes is #xFFFF...FFFF
Also, I'd rather minimize consing and thus call (with-foreign-object ...) sparingly. For the same reason, I'd rather minimize the use of :uintptr, which can exceed fixnum and thus allocate memory.
My opinion is:
1) using *signed* integers (of any size) to represent pointers is asking for trouble. Also, cffi-sys:make-pointer does not accept them. So I strongly prefer to use :uintptr as mmap/mremap return type.
2) Thinking that (or actually defining) MAP_FAILED = -1 is misleading. MAP_FAILED is (void *)-1.
3) There is no predefined constant for "the maximum value of a :pointer" or more exactly "the maximum value of an :uintptr". An accurate definition requires groveling CHAR_BIT from <limits.h> and then defining (defconstant +most-positive-uintptr+ (1- (ash 1 (* char-bit (cffi:foreign-type-size :uintptr)))))
alternatively, the type-punning trick you suggested can be used as well (defconstant +most-positive-uintptr+ (with-foreign-object (p :intptr) (setf (mem-ref p :intptr) -1) (mem-ref p :uintptr)))
I'd go for this second option, as it avoids groveling CHAR_BIT.
4) once we have +most-positive-uintptr+, we can easily work around the fact that sometimes groveled map-failed is positive and sometimes not.
In conclusion, I like your ideas and I updated the patch by integrating most of them.