[Vm-dev] Problem in newspeak cogit code generation

[David T. Lewis]

On Mon, Oct 28, 2019 at 10:52:38PM +0100, Nicolas Cellier wrote:
>  
> Hi David,
> OK, the variables are initialized conditionally in Cogit>>setInterpreter:
> 
>      BytecodeSetHasDirectedSuperSend ifTrue:
>          [directedSuperSendTrampolines := CArrayAccessor on: (Array new:
> NumSendTrampolines).
>          directedSuperBindingSendTrampolines := CArrayAccessor on: (Array
> new: NumSendTrampolines).
>          directedSendUsesBinding := false].
> 

There is a <doNotGenerate> directive for the method, so the initialization
that you see there is never part of the generated C code.


> So maybe it's not the case of Newspeak bytecode set, and maybe these
> methods won't be used, but it's not a nice way to do it.
> We shall not generate unused incorrect methods, it spoils compiler warning
> examination...
>

Actually, I think that it /is/ used (or at least it could be used if
#isDirected is true, which probably amounts to the same thing). All of
the unreferenced methods will be optimized away by the inliner, but there
is one remaining usage, and it does look like a possible source of trouble.

Dave

 
> Le dim. 27 oct. 2019 ?? 19:58, David T. Lewis <lewis at mail.msen.com> a ??crit :
> 
> >
> > On Sat, Oct 26, 2019 at 11:45:08PM +0200, Nicolas Cellier wrote:
> > >
> > > Hi all,
> > > I see what looks like a big problem in those files:
> > >
> > https://raw.githubusercontent.com/OpenSmalltalk/opensmalltalk-vm/Cog/nsspur64src/vm/cogitX64SysV.c
> > >
> > https://raw.githubusercontent.com/OpenSmalltalk/opensmalltalk-vm/Cog/nsspursrc/vm/cogitIA32.c
> > >
> > > The generated code for genSendDirectedSuper:numArgs: is bogus.
> > > the 3 instance variables
> > > - directedSendUsesBinding
> > > - directedSuperSendTrampolines
> > > - directedSuperBindingSendTrampolines
> > > have been translated into local (thus uninitialized) variables...
> > >
> > > How can this possibly work?
> > > Is it related to the CI failures?
> >
> > Very likely the variables are being generated as local because they are
> > not referenced elsewhere. For example, directedSuperSendTrampolines is
> > initialized in simulation only by Cogit>>setInterpreter: but apparently
> > is not set anywhere in the actual translated code.
> >
> > I don't think that uninitialized locals are set to null in C, so yes
> > this could be a source of random behavior in the CI tests.
> >
> > A solution would be to initialize the three variables, possibly just
> > by referencing them explicitly in one of the declareCVarsIn: methods.
> >
> > Dave
> >
> >