search

jinganix / enif_protobuf

A Google Protobuf implementation with enif (Erlang nif)
38 stars 21 forks source link

uint64 decode fail #19

Closed DominicGame closed 3 years ago

DominicGame commented 3 years ago 
static inline ERL_NIF_TERM
unpack_uint64(ErlNifEnv *env, ep_dec_t *dec, ERL_NIF_TERM *term)
{
    uint32_t    shift = 0, left = 10;
    uint64_t    val = 0;
    printf("uint64_t %d, \r\n", sizeof(val));

    while (left && dec->p < dec->end) {

        val |= (((uint64_t) (*(dec->p) & 0x7f)) << shift);
        printf("unpack_uint64 v:%d, %d, %lu, \r\n", (*(dec->p) & 0x7f), shift, (unsigned long) val);
        if ((*(dec->p)++ & 0x80) == 0) {

            *term = enif_make_ulong(env, (ErlNifUInt64) val);
            return RET_OK;
        }
        shift += 7;
        left--;
    }

    return_error(env, dec->term);
}

1> enif_protobuf:decode(<<8,181,207,209,168,154,47>>, gateway_s_heart). uint64_t 8, unpack_uint64 v:53, 0, 53, unpack_uint64 v:79, 7, 10165, unpack_uint64 v:81, 14, 1337269, unpack_uint64 v:40, 21, 85223349, unpack_uint64 v:26, 28, 2769577909, unpack_uint64 v:47, 35, 2769577909, {gateway_s_heart,2769577909}

2>gateway_12_pb:decode_msg(<<8,181,207,209,168,154,47>>, gateway_s_heart). ====={53,0,53} ====={79,7,10165} ====={81,14,1337269} ====={40,21,85223349} ====={26,28,7064545205} ====={47,35,1621972248501} {gateway_s_heart,1621972248501}

enif decode , ((uint64_t) (*(dec->p) & 0x7f)) seem work like: int32 << n but I print 'sizeof(uint64_t)' get 8 when I use gpb(gateway_12_pb) to decode, I can get the right result I have no idea about it.....

some info: Window 10 vcvarsall.bat x64, vs set compile env rebar3_gpb_plugin ~ "2.15.0" gpb ~ "4.17.3" =================== gateway_12.proto ================== syntax = "proto3";

message gateway_s_heart { uint64 time = 1; }

jinganix commented 3 years ago

I have no idea with windows. And add a test for this issue. Could you test it.

DominicGame commented 3 years ago

thanks for your reply! test result:

======================== EUnit ========================
module 'ep_tests'
  ep_tests: load_cache_test...[0.016 s] ok
  ep_tests: load_oneof_test...ok
  ep_tests: nested_oneof_test...ok
  ep_tests:130: smp_cache_encoding_test_...[16.000 s] ok
  ep_tests:160: smp_cache_decoding_test_...[13.188 s] ok
  ep_tests: decode_uint64_issue_19_test...*failed*
in function ep_tests:decode_uint64_issue_19_test/0 (d:/tmp/enif_protobuf_git/test/ep_tests.erl, line 178)
in call from eunit_test:'-mf_wrapper/2-fun-0-'/2 (eunit_test.erl, line 273)
in call from eunit_test:run_testfun/1 (eunit_test.erl, line 71)
in call from eunit_proc:run_test/1 (eunit_proc.erl, line 522)
in call from eunit_proc:with_timeout/3 (eunit_proc.erl, line 347)
in call from eunit_proc:handle_test/2 (eunit_proc.erl, line 505)
in call from eunit_proc:tests_inorder/3 (eunit_proc.erl, line 447)
in call from eunit_proc:with_timeout/3 (eunit_proc.erl, line 337)
**error:{badmatch,{m1,2769577909}}
  output:<<"">>

  [done in 29.297 s]
=======================================================
  Failed: 1.  Skipped: 0.  Passed: 5.
===> Error running tests

I use old rebar.config to compile it, will this cause some problem?

{provider_hooks, [{pre, [{compile, {pc, compile}}, {clean, {pc, clean}}]}]}.

{port_specs, [
    {"priv/enif_protobuf.so", [
        "c_src/enif_protobuf.c",
        "c_src/ep_cache.c",
        "c_src/ep_decoder.c",
        "c_src/ep_encoder.c",
        "c_src/ep_node.c"
    ]}
]}.

{port_env, [
    {"(linux|solaris|freebsd|netbsd|openbsd|dragonfly|darwin|gnu)",
        "CFLAGS", "$CFLAGS -Ic_src/ -g -std=c99 -Wall -Werror -O3 -fno-strict-aliasing"},
    {"(linux|solaris|freebsd|netbsd|openbsd|dragonfly|darwin|gnu)",
        "CXXFLAGS", "$CXXFLAGS -Ic_src/ -g -Wall -Werror -O3"},
    {"(linux|solaris|freebsd|netbsd|openbsd|dragonfly|darwin|gnu)",
        "LDFLAGS", "$LDFLAGS -lstdc++ -lm"},
    %% OS X Leopard flags for 64-bit
    {"darwin9.*-64$", "CXXFLAGS", "-m64"},
    {"darwin9.*-64$", "LDFLAGS", "-arch x86_64"},
    %% OS X Snow Leopard flags for 32-bit
    {"darwin9.*-64$", "CXXFLAGS", "-m64"},
    {"darwin9.*-64$", "LDFLAGS", "-arch x86_64"},
    %% OS X Snow Leopard flags for 32-bit
    {"darwin10.*-32$", "CXXFLAGS", "-m32"},
    {"darwin10.*-32$", "LDFLAGS", "-arch i386"},
    %% This will merge into basho/rebar/rebar.config eventually
    {"win32", "CFLAGS", "/DWIN32 /D_WINDOWS /D_WIN32 /DWINDOWS"},
    {"win32", "CXXFLAGS", "-Ic_src/ -g -Wall -O3"}
]}.
{eunit_opts, [
    verbose,
    {report, {
        eunit_surefire, [{dir,"."}]
    }}
]}.
jinganix commented 3 years ago

Can you get the assembly code of function unpack_uint64

DominicGame commented 3 years ago 
_TEXT   SEGMENT
shift$ = 32
left$ = 36
tv83 = 40
val$ = 48
env$ = 80
dec$ = 88
term$ = 96
unpack_uint64 PROC

; 137  : {

  00000 4c 89 44 24 18   mov     QWORD PTR [rsp+24], r8
  00005 48 89 54 24 10   mov     QWORD PTR [rsp+16], rdx
  0000a 48 89 4c 24 08   mov     QWORD PTR [rsp+8], rcx
  0000f 48 83 ec 48  sub     rsp, 72            ; 00000048H

; 138  :     uint32_t    shift = 0, left = 10;

  00013 c7 44 24 20 00
    00 00 00     mov     DWORD PTR shift$[rsp], 0
  0001b c7 44 24 24 0a
    00 00 00     mov     DWORD PTR left$[rsp], 10

; 139  :     uint64_t    val = 0;

  00023 48 c7 44 24 30
    00 00 00 00  mov     QWORD PTR val$[rsp], 0
$LN2@unpack_uin:

; 140  : 
; 141  :     while (left && dec->p < dec->end) {

  0002c 83 7c 24 24 00   cmp     DWORD PTR left$[rsp], 0
  00031 0f 84 a1 00 00
    00       je  $LN3@unpack_uin
  00037 48 8b 44 24 58   mov     rax, QWORD PTR dec$[rsp]
  0003c 48 8b 4c 24 58   mov     rcx, QWORD PTR dec$[rsp]
  00041 48 8b 49 10  mov     rcx, QWORD PTR [rcx+16]
  00045 48 39 08     cmp     QWORD PTR [rax], rcx
  00048 0f 83 8a 00 00
    00       jae     $LN3@unpack_uin

; 142  : 
; 143  :         val |= ((uint64_t) (*(dec->p) & 0x7f) << shift);

  0004e 48 8b 44 24 58   mov     rax, QWORD PTR dec$[rsp]
  00053 48 8b 00     mov     rax, QWORD PTR [rax]
  00056 0f be 00     movsx   eax, BYTE PTR [rax]
  00059 83 e0 7f     and     eax, 127       ; 0000007fH
  0005c 48 98        cdqe
  0005e 8b 4c 24 20  mov     ecx, DWORD PTR shift$[rsp]
  00062 48 d3 e0     shl     rax, cl
  00065 48 8b 4c 24 30   mov     rcx, QWORD PTR val$[rsp]
  0006a 48 0b c8     or  rcx, rax
  0006d 48 8b c1     mov     rax, rcx
  00070 48 89 44 24 30   mov     QWORD PTR val$[rsp], rax

; 144  :         if ((*(dec->p)++ & 0x80) == 0) {

  00075 48 8b 44 24 58   mov     rax, QWORD PTR dec$[rsp]
  0007a 48 8b 00     mov     rax, QWORD PTR [rax]
  0007d 0f be 00     movsx   eax, BYTE PTR [rax]
  00080 25 80 00 00 00   and     eax, 128       ; 00000080H
  00085 89 44 24 28  mov     DWORD PTR tv83[rsp], eax
  00089 48 8b 44 24 58   mov     rax, QWORD PTR dec$[rsp]
  0008e 48 8b 00     mov     rax, QWORD PTR [rax]
  00091 48 ff c0     inc     rax
  00094 48 8b 4c 24 58   mov     rcx, QWORD PTR dec$[rsp]
  00099 48 89 01     mov     QWORD PTR [rcx], rax
  0009c 83 7c 24 28 00   cmp     DWORD PTR tv83[rsp], 0
  000a1 75 1b        jne     SHORT $LN4@unpack_uin

; 145  : 
; 146  :             *term = enif_make_ulong(env, (ErlNifUInt64) val);

  000a3 8b 54 24 30  mov     edx, DWORD PTR val$[rsp]
  000a7 48 8b 4c 24 50   mov     rcx, QWORD PTR env$[rsp]
  000ac ff 15 a0 00 00
    00       call    QWORD PTR WinDynNifCallbacks+160
  000b2 48 8b 4c 24 60   mov     rcx, QWORD PTR term$[rsp]
  000b7 48 89 01     mov     QWORD PTR [rcx], rax

; 147  :             return RET_OK;

  000ba 33 c0        xor     eax, eax
  000bc eb 42        jmp     SHORT $LN1@unpack_uin
$LN4@unpack_uin:

; 148  :         }
; 149  :         shift += 7;

  000be 8b 44 24 20  mov     eax, DWORD PTR shift$[rsp]
  000c2 83 c0 07     add     eax, 7
  000c5 89 44 24 20  mov     DWORD PTR shift$[rsp], eax

; 150  :         left--;

  000c9 8b 44 24 24  mov     eax, DWORD PTR left$[rsp]
  000cd ff c8        dec     eax
  000cf 89 44 24 24  mov     DWORD PTR left$[rsp], eax

; 151  :     }

  000d3 e9 54 ff ff ff   jmp     $LN2@unpack_uin
$LN3@unpack_uin:

; 152  : 
; 153  :     return_error(env, dec->term);

  000d8 48 8b 4c 24 50   mov     rcx, QWORD PTR env$[rsp]
  000dd ff 15 00 00 00
    00       call    QWORD PTR WinDynNifCallbacks
  000e3 48 8b 4c 24 58   mov     rcx, QWORD PTR dec$[rsp]
  000e8 4c 8b 49 40  mov     r9, QWORD PTR [rcx+64]
  000ec 4c 8b 40 60  mov     r8, QWORD PTR [rax+96]
  000f0 ba 02 00 00 00   mov     edx, 2
  000f5 48 8b 4c 24 50   mov     rcx, QWORD PTR env$[rsp]
  000fa ff 15 c0 00 00
    00       call    QWORD PTR WinDynNifCallbacks+192
$LN1@unpack_uin:

; 154  : }

  00100 48 83 c4 48  add     rsp, 72            ; 00000048H
  00104 c3       ret     0
unpack_uint64 ENDP

this code?

edit: I try

printf("sizeof %d %d %ul %ul\n", sizeof(unsigned long long), sizeof(uint64_t), (uint64_t)47 << 35, (uint64_t)47 << 1);
sizeof 8 8 0l 94l

(uint64_t)47 << 35 get 0 unbelievable!!!

jinganix commented 3 years ago

Could you try ((uint64_t) 47) << 35 ?

DominicGame commented 3 years ago

I ask my friend about this, he let me try use '%lld' to printf

        val |= ((uint64_t) (*(dec->p) & 0x7f) << shift);
        printf("%lld %lld %d\n", val, ((uint64_t) (*(dec->p) & 0x7f) << shift), shift);

out: 53 53 0 10165 10112 7 1337269 1327104 14 85223349 83886080 21 7064545205 6979321856 28 1621972248501 1614907703296 35 in this case, test pass!!!!!

I check the warning

*term = enif_make_ulong(env, (ErlNifUInt64) val);

c_src/ep_decoder.c(147): warning C4242: conversion from “uint64_t” to “unsigned long”, possible loss of data then printf("sizeof %d %d %d %d\n", sizeof(uint64_t), sizeof(ErlNifUInt64), sizeof(unsigned long), sizeof(unsigned long long)); sizeof 8 8 4 8

and here is the compile cmd: cl.exe /c /DWIN32 /D_WINDOWS /D_WIN32 /DWINDOWS /FAcs /Zi /Wall -I"c:/Program Files/erl-23.0/lib/erl_interface-4.0/include" -I"c:/Program Files/erl-23.0/erts-11.0/include" c_src/ep_decoder.c /Foc_src/ep_decoder.obj

It's strange, I'm sorry that I don't know C well

jinganix commented 3 years ago

How about pass the big number as const:

*term = enif_make_ulong(env, 1621972248501L);
DominicGame commented 3 years ago 
*term = enif_make_ulong(env, 1621972248501L);

it's cuted and return 2769577909

I check erl doc:

ERL_NIF_TERM enif_make_uint64(ErlNifEnv* env, ErlNifUInt64 i)

Creates an integer term from an unsigned 64-bit integer.

ERL_NIF_TERM enif_make_ulong(ErlNifEnv* env, unsigned long i)

Creates an integer term from an unsigned long int.

c_src/ep_decoder.c(147): warning C4242: conversion from “uint64_t” to “unsigned long”, possible loss of data then printf("sizeof %d %d %d %d\n", sizeof(uint64_t), sizeof(ErlNifUInt64), sizeof(unsigned long), sizeof(unsigned long long)); sizeof 8 8 4 8

'unsigned long int' maybe work as uint32 on windows so, I replace 'enif_make_ulong' with 'enif_make_uint64' and I get the right value 🎉

by the way, should I close issue right now?

jinganix commented 3 years ago

Great, would you like to create a PR?