[Vm-dev] Hypothetical (VMBIGENDIAN) problem in ceSendMustBeBooleanTointerpretingAtDelta

[Nicolas Cellier]

C compiler warnings are powerful when an IDE is availlable for navigating
thru declarations, otherwise the price of deciphering the warnings and
rejecting false alarms is really too high, especially in inlined VM code!
Here I used MSVC and visual studio 2019 preview

Avertissement    C4293    '<<' : compteur de décalage négatif ou trop
important, comportement non défini    SqueakCogSpur
X:\Smalltalk\opensmalltalk-vm\spur64src\vm\cointerp.c    16102

Normally, I would report such problem directly in code annotation (review)
on github.
But this is generated code, and the problem is at line 16092 (16102 is the
end of line), so nobody is going to read it.

    longAtput(GIV(framePointer) + FoxIFrameFlags, (VMBIGENDIAN
        ? ((1 + (((cogMethod->cmNumArgs)) << ((BytesPerWord * 8) - 8))) +
((((longAt(GIV(framePointer) + FoxMethod)) & MFMethodFlagHasContextFlag) !=
0
    ? 1ULL << ((BytesPerWord * 8) - 16)
    : 0))) + ((((longAt(GIV(framePointer) + FoxMethod)) &
MFMethodFlagIsBlockFlag) != 0
    ? 1ULL << ((BytesPerWord * 8) - 24)
    : 0))
        : ((1 + (((cogMethod->cmNumArgs)) << 8)) +
((((longAt(GIV(framePointer) + FoxMethod)) & MFMethodFlagHasContextFlag) !=
0
    ? 1U << 16
    : 0))) + ((((longAt(GIV(framePointer) + FoxMethod)) &
MFMethodFlagIsBlockFlag) != 0
    ? 1U << 24
    : 0))));

if VMBIGENDIAN is true (that's currently NEVER the case, but see below),
then
cogMethod->cmNumArgs is an unsigned int (32 bit) that we shift by
(BytesPerWord * 8) - 8) = 56 bits... Undefined Behavior as the warning
tells.

Note that the VM simulator currently hides the problem because it uses the
unlimited integer model of Squeak rather than the bounded integer model of
target architecture.

Corrections may vary
- do nothing and remember that we must ignore this specific warning? NO,
this not sustainable!
- insert specific workaround (cast...) in
ceSendMustBeBooleanTointerpretingAtDelta... until we repeat the same
mistake somewhere else;
- craft more generic patch in code generation (with better type inferencing
we can know that left member is 32 bits long, and with aggressive constant
substitution that we already use, we also know that shift is larger than 31
bits...);
- eradicate VMBIGENDIAN support.

I'm wondering about last solution. This supports costs (like time to review
warnings that we should not care so much about), and rots (code is never
executed, not even compiled if compilers are clever enough). It's becoming
more a flag and a list of recipes for future support than real support.
Whenever a new BIGENDIAN machine will reappear, the VMBIGENDIAN support
will be so rotten that we'll have to rewrite every old place that we
flagged and review every potential new place that we forgot to flag... The
recipes could go somewhere else (some wiki or a dead branch appropriately
traced in the wiki for example).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20181225/616d61f3/attachment-0001.html>