[Vm-dev] intAt from sqMemoryAccess.h does not extend sign
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Tue Nov 8 14:19:50 UTC 2016
Hi David,
Sorry, I've no linux/mac vm available currently to exhibit the problem.
But if you look at github head revision
https://raw.githubusercontent.com/OpenSmalltalk/opensmalltalk-vm/Cog/spur64src/vm/gcc3x-cointerp.c
then you'll find this code:
primitiveIntegerAt(void)
{ DECL_MAYBE_SQ_GLOBAL_STRUCT
sqInt addr;
... snip declarations of no interest ...
sqInt value;
... snip long preamble to check parameter types ...
/* for zero indexing */
addr = (rcvr + BaseHeaderSize) + ((index - 1) * 4);
value = intAt(addr);
/* begin pop: */
GIV(stackPointer) += 2 * BytesPerWord;
if ((((((usqInt) value) >> 60) + 1) & 15) <= 1) {
/* begin pushInteger: */
longAtput((sp = GIV(stackPointer) - BytesPerWord), ((value << 3) | 1));
GIV(stackPointer) = sp;
}
else {
... snip this branch is never reached ...
}
}
Above code clearly can't work if the 32bit contents of addr is not
sign-extended into the 64bits value...
So I don't know which version you used exactly,
...UNLESS...
there might be another reason for the mismatch in:
https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/Cog/platforms/Cross/vm/sqMemoryAccess.h
You can see that the two possible compiler branches don't agree and won't
behave the same sign-wise:
#ifdef USE_INLINE_MEMORY_ACCESSORS
/* Use static inline functions when the compiler produces efficient
code for small accessors.
These are preferred because static type checking will prevent
inadvertent confusion of pointers and oops. */
... snip ...
static inline sqInt intAtPointer(char *ptr) { return
(sqInt)(*((unsigned int *)ptr)); }
static inline sqInt intAtPointerput(char *ptr, int val) { return
(sqInt)(*((unsigned int *)ptr)= val); }
... snip ...
#else /* USE_INLINE_MEMORY_ACCESSORS */
/* Use macros when static inline functions aren't efficient. */
... snip ...
# define intAtPointer(ptr) ((sqInt)(*((int *)(ptr))))
# define intAtPointerput(ptr,val) ((sqInt)(*((int *)(ptr))= (int)(val)))
... snip ...
#endif /* USE_INLINE_MEMORY_ACCESSORS */
I presume you (or someone else) arranged to not USE_INLINE_MEMORY_ACCESSORS
Though it seems to be the default to use them on macosx (I'm just invoking
./mvm).
The two branches should better adopt the same (sane) behavior, shouldn't
they?
2016-11-08 14:23 GMT+01:00 David T. Lewis <lewis at mail.msen.com>:
>
> On Tue, Nov 08, 2016 at 03:29:05AM +0100, Nicolas Cellier wrote:
> >
> > In this code:
> >
> > static inline sqInt intAtPointer(char *ptr) { return
> > (sqInt)(*((unsigned int *)ptr)); }
> > static inline sqInt intAtPointerput(char *ptr, int val) { return
> > (sqInt)(*((unsigned int *)ptr)= val); }
> >
> > the usage of unsigned int is questionable...
> > It means that intAt and long32At won't perform sign extension in 64bits
> > Spur VM.
> > Since it returns a signed result (sqInt), that's troubling.
> >
> > The sole real sender of intAt seems to be primitiveIntegerAt: which fails
> > for this reason on 64bits spur when fed with negative integers.
>
>
> Can you give an example of the failure? I tried on 64-bit Spur and also
> on image format 68002 (64-bit V3) on Linux, and the accesses to elements
> of an IntegerArray work correctly in both cases.
>
>
> For example:
>
> { -1 . -2 . -3 . -4 . 7 . -9 } asIntegerArray ==> an IntegerArray(-1 -2 -3
> -4 7 -9)
>
> Accessing the elements of this IntegerArray works as expected, and I can
> see
> in an inspector that the first element is stored internally as 16rFFFFFFFF.
>
> Is there something else that is failing?
>
> Dave
>
>
> >
> > For long32At, it's difficult to analyze: many senders!
> >
> > IMO, if we don't want a signed quantity, we should better write
> > unsignedLong32At (or use lowcode uint32AtPointer which answers a 32 bits
> > result)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20161108/c5dcd590/attachment.html>
More information about the Vm-dev
mailing list