[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