My experience using asmjit and bug report

[Eugene]

Hi all,

Thank you for your library. This semester we used it in our
virtualization classes.
Repository is here: http://code.google.com/p/mathvm/

I am student. My implementation of virtual machine is here:
http://code.google.com/p/mathvm/source/browse/#svn%2Ftrunk%2Fstudents%2F2011%2FEvgeny.Batalov%2Fvm

This vm can generate a lot of code so it can help you in testing your
library.
I can help you with testing using this vm because if we have a bug in
particular test it can be hard to understand who introduced it - vm or
compiler.

I've used compiler to generate optimized code and I think that I have
found some bugs or documentation inaccuracy in it. I could make
library usage mistakes too.

Here they are:
1. idiv_lo_hi

Definition from docs:
void AsmJit::CompilerIntrinsics::idiv_lo_hi
(const GPVar & dst_lo, const GPVar & dst_hi, const GPVar & src )

This instruction divides (signed) the value in the AL, AX, or EAX
register by the source operand and stores the result in the AX, DX:AX,
or EDX:EAX registers.

Definition from intel manual:
Signed divide RDX:RAX by r/m64, with result stored in RAX ← Quotient,
RDX ←Remainder.

So I thought that I can use it in following way (code from vm jit):
case BC_IDIV:      { AnyVar _tos1 = ssaStack.back();
ssaStack.pop_back();
                          AnyVar _tos2 = ssaStack.back();
ssaStack.pop_back();
                          GPVar _tmp(cc.newGP());
                          cc.mov(_tmp, imm(0));
                          cc.idiv_lo_hi(*_tos1.gp, _tmp, *_tos2.gp);
                          ssaStack.push_back(_tos1);
                          cc.unuse(*_tos2.gp); cc.unuse(_tmp);
                        }   break;


Sometimes it works, but the following simple example breaks with
SIGFPE, Arithmetic exception (because value if remainder is too long
to put into RDX):
int i;
int j;
int k;

i = 2;
j = 7;

k = j / 2 + 1;
print(k, '\n');

Generated assembler is:
mov rcx, 2
mov [rdi], rcx
mov rcx, 7
mov [rdi + 8], rcx
mov rcx, 1
mov rdx, 2
mov rax, [rdi + 8]
mov r8, 0
mov r8, rdx
idiv r8
add rcx, rax

on idiv registers' values are:
rax            0x7      7
rdx            0x2      2
r8             0x2      2

So zero is forgiven...
But if we compile
int i;
int j;
int k;

i = 2;
j = 7;

k = j / 2;// + 1;
print(k, '\n');

then register allocation works well:

mov rcx, 2
mov [rdi], rcx
mov rcx, 7
mov [rdi + 8], rcx
mov rcx, 2
mov rax, [rdi + 8]
mov rdx, 0
idiv rcx

2. cvtsi2sd - error in instruction generation and in asmjit
disassembler
Compiler generates instruction for 32 bit registers on my 64 bit
machine.

Usage example from my jit compiler:
case BC_I2D:      {AnyVar _int = ssaStack.back();
                         ssaStack.pop_back();
                         AnyVar _double = newSSAXMMVar(cc);
                         cc.cvtsi2sd(*_double.xmm, *_int.gp);
                         ssaStack.push_back(_double);
                         cc.unuse(*_int.gp);} break;

Source code example:
double d;
int i;
i = 4294967296;
d = i;
print(d);

AsmJit logger output:
; Prolog
sub rsp, 40
; Body
mov rcx, 4294967296
mov [rdi + 8], rcx
mov rcx, [rdi + 8]
cvtsi2sd xmm0, rcx

But in gdb diassembler we see other thing:
 x /10i $rip
=> 0x7ffff7fe8000:      sub    $0x28,%rsp
  0x7ffff7fe8004:      movabs $0x100000000,%rcx
  0x7ffff7fe800e:      mov    %rcx,0x8(%rdi)
  0x7ffff7fe8012:      mov    0x8(%rdi),%rcx
  0x7ffff7fe8016:      cvtsi2sd %ecx,%xmm0

So compiler produced invalid opcode and logger showed invalid opcode
too.

3. Also there are problems with labels. But my current understanding
of asmjit code doesn't allow to localize source of errors or context
where they are possible.

[Petr Kobalíček]

Hi Eugene,

Thank you for using AsmJit and welcome to the mailing list!

On Mon, Mar 12, 2012 at 8:49 PM, Eugene <eabat...@gmail.com> wrote:
> Hi all,
>
> Thank you for your library. This semester we used it in our
> virtualization classes.
> Repository is here: http://code.google.com/p/mathvm/

I just checked and I like that the project uses static typing, not so
usual when it comes to VMs.

> I am student. My implementation of virtual machine is here:
> http://code.google.com/p/mathvm/source/browse/#svn%2Ftrunk%2Fstudents%2F2011%2FEvgeny.Batalov%2Fvm
>
> This vm can generate a lot of code so it can help you in testing your
> library.
> I can help you with testing using this vm because if we have a bug in
> particular test it can be hard to understand who introduced it - vm or
> compiler.

I'm interested about any bugs generated by AsmJit, don't hasitate to
report them!

> I've used compiler to generate optimized code and I think that I have
> found some bugs or documentation inaccuracy in it. I could make
> library usage mistakes too.
>
> Here they are:
> 1. idiv_lo_hi
>
> Definition from docs:
> void AsmJit::CompilerIntrinsics::idiv_lo_hi
> (const GPVar & dst_lo, const GPVar & dst_hi, const GPVar & src )
>
> This instruction divides (signed) the value in the AL, AX, or EAX
> register by the source operand and stores the result in the AX, DX:AX,
> or EDX:EAX registers.
>
> Definition from intel manual:
> Signed divide RDX:RAX by r/m64, with result stored in RAX ← Quotient,
> RDX ←Remainder.

Okay, I think that dst_lo and dst_hi should be renamed to dst_rem and
dst_quot, do you agree?

> So I thought that I can use it in following way (code from vm jit):
> case BC_IDIV:      { AnyVar _tos1 = ssaStack.back();
> ssaStack.pop_back();
>                           AnyVar _tos2 = ssaStack.back();
> ssaStack.pop_back();
>                           GPVar _tmp(cc.newGP());
>                           cc.mov(_tmp, imm(0));
>                           cc.idiv_lo_hi(*_tos1.gp, _tmp, *_tos2.gp);
>                           ssaStack.push_back(_tos1);
>                           cc.unuse(*_tos2.gp); cc.unuse(_tmp);
>                         }   break;

I will check this out, I think that maybe the problem is wrong
argument position generated by AsmJit.

It's hard to me to check for error only using this log, I think that
the best is to create small example which uses only AsmJit::Compiler
and generates the exception.

Agreed, I filled issue #48.

> 3. Also there are problems with labels. But my current understanding
> of asmjit code doesn't allow to localize source of errors or context
> where they are possible.

I think that you should make sure you created a label using
newLabel(). Labels contain ID which is used by assembler/compiler and
this ID is only generated by calling newLabel(). I hope there is no
other problem here. But anyway, if you encounter an issue here, don't
hesitate to report it. You can also put asserts into AsmJit and send
me patch if you think that there is something wrong.

I will look at the issues tonight.

Thanks
Petr

[Petr Kobalíček]

Just to report some progress,

I fixed issue #48, but there is some SVN problem at code.google.com,
so I will try to do a commit tomorrow.

I also reviewed the idiv issue and it seems that you are using
AsmJit-1.0-beta3. There is newer version in SVN however. I'm going to
make a new beta4 release with all fixes applied this weekend.

Best regards
Petr Kobalicek

[Евгений Баталов]

Petr, thank you very much for your work. I'll support this thread when
I have time. I have to much work now.

16 марта 2012 г. 3:31 пользователь Petr Kobalíček
<kobalic...@gmail.com> написал:

[Eugene]

Hi all once again and sorry for my delay.

Thank you Petr for new release.

Recently I tried beta 4 to check whether old errors reported by me are fixed. Also I've run all tests for my VM to check for other errors.

Here is my experience:

1. We compile our VM with -Wall -Werror. External libraries are not compiled with such options. But compiling beta4 I got warning in asmjit header file (which is treated as error).

It is not very critical for your lib as I think. But maybe for some users it can be inconvenient a little.

Here is g++ output:

g++ -c  -D_POSIX_SOURCE -DMATHVM_POSIX -DMATHVM_WITH_SDL -O2 -Wall -Werror -D_REENTRANT -fPIC -ggdb3  -I../../../../include -I../../../../libs -I../../../../vm main.cpp -o build/opt/main.o

cc1plus: warnings being treated as errors

In file included from ../../../../libs/asmjit/DefsX86X64.h:17,

                 from ../../../../libs/asmjit/Defs.h:412,

                 from ../../../../libs/asmjit/Compiler.h:13,

                 from JITCompiler.h:3,

                 from ExecutionEnv.h:6,

                 from main.cpp:5:

../../../../libs/asmjit/Util.h: In function ‘uint32_t AsmJit::Util::bitCount(uint32_t)’:

../../../../libs/asmjit/Util.h:93: error: suggest parentheses around ‘+’ in operand of ‘&’

make: *** [build/opt/main.o] Error 1

Following this article: http://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B#Operator_precedence

Line 93 can be fixed with following code: return (((x + (x >> 4)) & 0xF0F0F0F) * 0x1010101) >> 24; //because binary '+' has higher priority then bitwise '&'

2. It seems like you added management of exported symbols onto asmjit. So my types which include AsmJit types were had to be fixed with pragma (one of some ways to do it).

#pragma GCC visibility push(hidden)

typedef union {

  AsmJit::GPVar  *gp;

  AsmJit::XMMVar *xmm;

} AnyVar;

typedef std::vector<AnyVar> SSAStack;

#pragma GCC visibility pop

3. About the bug with idiv_lo_hi. As I see it was removed in beta4 and there is no method for division with register allocation feature now?

4. About issue #48. Yes it is fixed. Error isn't reproducible now.

5. About labels. In previous post I mentioned that there are problems with them but without details. I found my old code which I used to study how to use asmjit. 

And I am still having problems with this code...

Here is it:

 //labels

  typedef void (*Func9)();

  Compiler c9;

  c9.setLogger(&logger1);

  c9.newFunction(CALL_CONV_DEFAULT, FunctionBuilder0<void>());

  c9.getFunction()->setHint(FUNCTION_HINT_NAKED, true);

  

  Label l91 = c9.newLabel();

  Label l92 = c9.newLabel();

  Label l93 = c9.newLabel();

  Label l94 = c9.newLabel();

  Label l95 = c9.newLabel();

  Label l96 = c9.newLabel();

  Label l97 = c9.newLabel();

  c9.bind(l92);

  GPVar _var91(c9.newGP());

  GPVar _var92(c9.newGP());

  

  c9.bind(l93);

  c9.jmp(l91);

  c9.bind(l95);

  c9.mov(_var91, imm(0));

  c9.bind(l96);

  c9.jmp(l93);

  c9.mov(_var92, imm(1));

  c9.jmp(l91);

  c9.bind(l94);

  c9.jmp(l92);

  c9.bind(l97);

  c9.add(_var91, _var92);

  c9.bind(l91);

  c9.ret();

  c9.endFunction();

  

  Func9 func9 = function_cast<Func9>(c9.make()); //line 390

  MemoryManager::getGlobal()->free((void*)func9);

GDB says:

Program received signal SIGSEGV, Segmentation fault.

memcpy () at ../sysdeps/x86_64/memcpy.S:191

191     ../sysdeps/x86_64/memcpy.S: No such file or directory.

        in ../sysdeps/x86_64/memcpy.S

(gdb) bt

#0  memcpy () at ../sysdeps/x86_64/memcpy.S:191

#1  0x0000000000447e8c in AsmJit::CompilerContext::_assignState (

    this=0x7fffffffced0, state=0x0)

    at ../../../../libs/asmjit/CompilerX86X64.cpp:6487

#2  0x0000000000437b72 in AsmJit::ETarget::translate (this=0x6bad30, cc=...)

    at ../../../../libs/asmjit/Compiler.cpp:252

#3  0x000000000044abfd in AsmJit::CompilerCore::serialize (

    this=0x7fffffffd280, a=...)

    at ../../../../libs/asmjit/CompilerX86X64.cpp:7729

#4  0x000000000044a86c in AsmJit::CompilerCore::make (this=0x7fffffffd280)

    at ../../../../libs/asmjit/CompilerX86X64.cpp:7615

#5  0x000000000046b139 in main () at main.cpp:390

Am I doing something worng with labels. All labels are binded once.

Also I have a complicated source code (Newton function optimization) for my VM which fails due to label problems.

6. I tested my VM with test set which was passed with beta3. 

Test set is quite big and good news! Beta4 passes VM test set too! :)

[Petr Kobalíček]

Hi Eugene

On Mon, Apr 16, 2012 at 10:33 PM, Eugene <eabat...@gmail.com> wrote:
> Hi all once again and sorry for my delay.
> Thank you Petr for new release.

No problem and thanks:)

Thanks, this should be fixed now by #402.

> 2. It seems like you added management of exported symbols onto asmjit. So my
> types which include AsmJit types were had to be fixed with pragma (one of
> some ways to do it).
> #pragma GCC visibility push(hidden)
> typedef union {
>   AsmJit::GPVar  *gp;
>   AsmJit::XMMVar *xmm;
> } AnyVar;
> typedef std::vector<AnyVar> SSAStack;
> #pragma GCC visibility pop

I think this have been added a whole time ago, however, I'm planning
to remove that feature, because source code is usually compiled with
hidden visibility. It was critical some time ago.

But on the other hand, I don't like the union. If you need union-like
type, you can use "AsmJit::BaseVar"

AsmJit::BaseVar var = c.newGP();

var.isGP();
var.isXMM();

But the casting is a bit verbose:

static_cast<AsmJit::GPVar&>(var);
static_cast<AsmJit::XMMVar&>(var);

IF your union works then there is no problem, but I wonder about your
memory management to manage the pointers.

> 3. About the bug with idiv_lo_hi. As I see it was removed in beta4 and there
> is no method for division with register allocation feature now?

Please use plain div/idiv, it's the same:

inline void idiv(const GPVar& dst_rem, const GPVar& dst_quot, const
GPVar& src)
inline void idiv(const GPVar& dst_rem, const GPVar& dst_quot, const Mem& src)

> 4. About issue #48. Yes it is fixed. Error isn't reproducible now.

OK, thanks:)

I will look into this issue, thanks for this code.

> 6. I tested my VM with test set which was passed with beta3.
> Test set is quite big and good news! Beta4 passes VM test set too! :)

Thanks, good to know.

I'm going to reply here after I found solutions to fix those problems.
To fix visibility issue you can use -DASMJIT_HIDDEN compiler option
(AsmJit won't redeclare it if defined).

[Petr Kobalíček]

Hi Eugene,

I filled issues #55 and #56.

Best regards
Petr Kobalicek

On Mon, Apr 16, 2012 at 11:00 PM, Petr Kobalíček

[Eugene]

Hi once again,

very quick response so I decided to answer quickly too :)

I like your point about using BaseVar instead of union. In my case I need to use pointers cause I need to store vars in a single stack data structure. 

Pointers allows not to copy variables when poping them and pushing. I think it can be unsafe to copy them. Maybe asmjit allows to copy vars by copy constructor and copied var is still the same as original. But it is simply my "protective programming" :) Generally it is better to understand library you are using better and don't do things by yourself :)

And I tried AsmJit::CompilerIntrinsics::idiv. All the stuff including generated machine code is the same as I reported earlier with idiv_lo_hi.

I tried to reproduce this behavior in C code. I imitated behavior of my JIT compiler which uses stack machine bytecode to generate instruction for asmjit compiler. But my C code works well. Here it is:

/*source code:

int i;

int j;

int k;

i = 2;

j = 7;

k = j / 2 + 1;

*/

  //Equivalent C code:

  typedef int (*Func10)();

  Compiler c10;

  c10.setLogger(&logger1);

  c10.newFunction(CALL_CONV_DEFAULT, FunctionBuilder0<int>());

  c10.getFunction()->setHint(FUNCTION_HINT_NAKED, true);

  

  GPVar _10tos1(c10.newGP());

  GPVar _10tos2(c10.newGP());

  GPVar _10tos3(c10.newGP());

  GPVar _10tos4(c10.newGP());

  GPVar _10tos5(c10.newGP());

  GPVar _10tos6(c10.newGP());

  GPVar _10tos7(c10.newGP());

  GPVar _10tos8(c10.newGP());

  GPVar _10tos9(c10.newGP());

  GPVar _10i(c10.newGP());

  GPVar _10j(c10.newGP());

  GPVar _10k(c10.newGP());

  GPVar _10tmp(c10.newGP());

 c10.mov(_10tos1, imm(2));

  c10.mov(_10i, _10tos1); // i = 2

  c10.unuse(_10tos1);

  c10.mov(_10tos2, imm(7)); 

  c10.mov(_10j, _10tos2);// j = 7

  c10.unuse(_10tos2);

  c10.mov(_10tos3, imm(1));

  c10.mov(_10tos4, imm(2));

  c10.mov(_10tos5, _10j);

  c10.mov(_10tmp, imm(0));

  //j / 2 + 1

  c10.idiv(_10tmp, _10tos5, _10tos4); //remainder, quotient/devidend, devider

  c10.unuse(_10tmp);

  c10.unuse(_10tos4);

  c10.add(_10tos5, _10tos3);

  c10.unuse(_10tos3);

  c10.mov(_10k, _10tos5);

  c10.unuse(_10tos5);

  c10.ret(_10k);

  c10.endFunction();

  

  Func10 func10 = function_cast<Func10>(c10.make());

  std::cout << func10() << std::endl;

  MemoryManager::getGlobal()->free((void*)func10);

Maybe I can generate log of instructions which my JIT compiler gives to asmjit compiler, can I? I think it will reproduce JIT compiler  behavior very well.

Also I think it is a good idea to fix documentation for idiv. Names of parameters are very nice but comments to function are written in terms of registers. We don't use them in compiler.