nasty bug

[Palo Marton]

Hi,

I have hit a really nasty bug that seems to happen only with very complex function. Here is part of my source:

{

AsmJit::GPVar p(c.newGP(AsmJit::VARIABLE_TYPE_GPD));

AsmJit::GPVar r(c.newGP(AsmJit::VARIABLE_TYPE_GPD));

...some code that coputes p and r is here

if (m_iTag==1234) {

c.cmp(p,p); <--- THIS LINE SHOULD HAVE NO EFFECT

c.cmp(r,r); <--- reset CPU flags, just to ensure that previous line has no effect

}

Jit_S0(target, c, p, r);

if (m_iTag==1234) {

CSInteger::Jit_assert_eq(c, AsmJit::dword_ptr_abs(m_pData), 0);

}

... p and r are not used anymore here

}

c - compiler

p,r - distinct GPVars

Jit_S0 - complex function that adds big bunch of code to compiler (inlines many functions). It also writes value r to m_pData array on some position that depends on value of p, but it should never write to m_pData[0] if m_iTag==1234.

target - some C++ object, not important

m_iTag - variable used to tag objects where m_pData[0] should always be 0.

Jit_assert_eq - runtime assertion that checks whether m_pData[0]==0

Strange about this code is that its behaviour depends on line c.cmp(p,p). 

With this:

c.cmp(p,p); <--- THIS LINE SHOULD HAVE NO EFFECT

It works OK.

With this:

//c.cmp(p,p); 

or this:

c.mov(p,p);

or this:

c.nop();

It sometimes fails on assertion.

This line should have no effect whatsoever on the runtime behaviour of the code, but apparently it has. It probably affects "liveness" of p variable during register assignment or something like that.

I think that this is some complicated bug in AsmJit. Can it have something to do with your recent fix to esp register bug?

I will investigate this further to get more info...

Palo

[Palo Marton]

I have found the place where AsmJit emmits wrong code. Here is my source:

c.nop();

c.nop();

c.add(AsmJit::dword_ptr_abs(this0->m_pData, moznost, 2),i);

c.nop();

c.nop();

and this is what gets emitted:

09BCC078 90                   nop  

09BCC079 90                   nop  

09BCC07A 8B 44 24 1C          mov         eax,dword ptr [esp+1Ch]  

09BCC07E 8B 44 24 20          mov         eax,dword ptr [esp+20h]  

09BCC082 01 05 18 79 08 08    add         dword ptr ds:[8087918h],eax  

09BCC088 90                   nop  

09BCC089 90                   nop  

(nops are there just to tag the place in emitted code).

Pretty strange output. The code should write i to m_pData[moznost], but instead of it it moves 2 times something to eax and then it writes eax to m_pData[0]. I think that the bug here is that GPVar "moznost" is not in register but on stack and AsmJit has some bug in handling this situation. What it should have done is to free some register, move "moznost" into that register and then use it for proper addressing like:

add         dword ptr ds:[ebx*4+8087918h],eax 

Is there some quick fix for this, or can you point me to the place in AsmJit source code that handles this, so I can fix it by myself?

Palo

[Petr Kobalíček]

Hi Palo,

Thanks for the report.

I will check this issue by today's evening. It looks like the compiler doesn't know about the index register at ptr_abs.

Best,
Petr 

--
 
---
You received this message because you are subscribed to the Google Groups "asmjit-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to asmjit-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Palo Marton]

More details:

I have added one assert on which it fails and probably should not:

void CompilerContext::translateOperands(Operand* operands, uint32_t count) ASMJIT_NOTHROW

{

...

    else if (o.isMem())

    {

      if ((o.getId() & OPERAND_ID_TYPE_MASK) == OPERAND_ID_TYPE_VAR)

      {

        // Memory access. We just increment here actual displacement.

        VarData* vdata = _compiler->_getVarData(o.getId());

        ASMJIT_ASSERT(vdata != NULL);

        o._mem.displacement += vdata->isMemArgument

          ? _argumentsActualDisp

          : _variablesActualDisp;

        // NOTE: This is not enough, variable position will be patched later

        // by CompilerContext::_patchMemoryOperands().

      }

      else if ((o._mem.base & OPERAND_ID_TYPE_MASK) == OPERAND_ID_TYPE_VAR)

      {

        VarData* vdata = _compiler->_getVarData(o._mem.base);

        ASMJIT_ASSERT(vdata != NULL);

        o._mem.base = vdata->registerIndex;

      }

      if ((o._mem.index & OPERAND_ID_TYPE_MASK) == OPERAND_ID_TYPE_VAR)

      {

        VarData* vdata = _compiler->_getVarData(o._mem.index);

        ASMJIT_ASSERT(vdata != NULL);

ASMJIT_ASSERT(vdata->registerIndex!=INVALID_VALUE); <!---- ADDED LINE, FAILS ON THIS ASSERT

        o._mem.index = vdata->registerIndex;

      }

    }

  }

}

p.

Dňa piatok, 20. decembra 2013 10:08:13 UTC+1 petr kobalicek napísal(-a):

[Palo Marton]

I have tried to dig into this further but without any success.

It seems that EInstruction::prepare should somehow ensure that index var is in register by this part of code:

      if ((o._mem.index & OPERAND_ID_TYPE_MASK) == OPERAND_ID_TYPE_VAR)

      {

        VarData* vdata = _compiler->_getVarData(o._mem.index);

        ASMJIT_ASSERT(vdata != NULL);

        if (vdata->workOffset != _offset)

        {

          if (!cc._isActive(vdata)) cc._addActive(vdata);

          vdata->workOffset = _offset;

          variablesCount++;

        }

      }

But I don't know how this works. It should probably add this variable to _active list. But I have not found where this list is processed in this case. It seems to be accessed only during jump and call instructions, but nowhere else.

Luckily I have some workaround that seems to fix this:

c.alloc(moznost); // ASMJIT BUG: https://groups.google.com/forum/#!topic/asmjit-dev/7xoH7Y-AvK4

c.add(AsmJit::dword_ptr_abs(this0->m_pData, moznost, 2),i);

This is a bit better solution than my previous workaround, which was to add dummy c.cmp(p,p); instruction on some completely unrelated place in my code.

p.

[Palo Marton]

Finally I have tracked down this nasty bug and fixed it.

Problem was in function EInstruction::translate. If instruction had 2 operands that are spilled (on stack), than there was a bug in how they are allocated back to registers.

First AsmJit assigned workOffset=cc._currentOffset to both variables. Then it tried to allocate register for the first operand. For that it needed to emit one instruction. This resulted in incrementing of cc._currentOffset. Then it tried to allocate register for the second operand. But because the _currentOffset was incremented, there was nothing to prevent it from allocating the same register also to this second operand. During that it spilled the first operand, which ended with vdata->registerIndex==INVALID_VALUE.

My fix to this is to update workOffset of all operands also during allocation loop. See code below.

p.

Emittable* EInstruction::translate(CompilerContext& cc) ASMJIT_NOTHROW

{

  uint32_t i;

  uint32_t variablesCount = _variablesCount;

  if (variablesCount > 0)

  {

    // These variables are used by the instruction and we set current offset

    // to their work offsets -> getSpillCandidate never return the variable

    // used this instruction.

    for (i = 0; i < variablesCount; i++)

    {

      _variables->vdata->workOffset = cc._currentOffset;

    }

    // Alloc variables used by the instruction (special first).

    for (i = 0; i < variablesCount; i++)

    {

for(int j=0; j<variablesCount; j++) {

VarAllocRecord& r2 = _variables[j];

r2.vdata->workOffset=cc._currentOffset;

}

      VarAllocRecord& r = _variables[i];

      // Alloc variables with specific register first.

      if ((r.vflags & VARIABLE_ALLOC_SPECIAL) != 0)

        cc.allocVar(r.vdata, r.regMask, r.vflags);

    }

    for (i = 0; i < variablesCount; i++)

    {

 for(int j=0; j<variablesCount; j++) {

 VarAllocRecord& r2 = _variables[j];

 r2.vdata->workOffset=cc._currentOffset;

 }

      VarAllocRecord& r = _variables[i];

      // Alloc variables without specific register last.

      if ((r.vflags & VARIABLE_ALLOC_SPECIAL) == 0) {

        cc.allocVar(r.vdata, r.regMask, r.vflags);

 }

 ASMJIT_ASSERT(r.vdata->registerIndex!=INVALID_VALUE);

 for(int j=0; j<=i; j++) {

 VarAllocRecord& r2 = _variables[j];

 ASMJIT_ASSERT(r2.vdata->registerIndex!=INVALID_VALUE);

 }

    }