From e5d41874ec0d2aaf2037d10dd92091edd2405924 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Wed, 16 Jun 2010 17:58:35 +0200 Subject: term_to_binary use all 32 bits of INTEGER_EXT Earlier, external format INTEGER_EXT was only produced for 28-bit signed integers. Now full 32-bit signed integers are produced as INTEGER_EXT to avoid the more costly SMALL_BIG_EXT format. Both old and new code can read 32-bit INTEGER_EXT. Also fixed integer encoding bugs in erl_interface erl_encode/erl_decode. (Thanks to Alexander Demidenko for reporting) --- lib/erl_interface/src/legacy/erl_marshal.c | 93 ++++++++----------- lib/erl_interface/test/ei_decode_SUITE.erl | 34 ++++--- .../test/ei_decode_SUITE_data/ei_decode_test.c | 50 ++++++---- .../test/erl_eterm_SUITE_data/eterm_test.c | 102 ++++++++++++++++++--- 4 files changed, 177 insertions(+), 102 deletions(-) (limited to 'lib') diff --git a/lib/erl_interface/src/legacy/erl_marshal.c b/lib/erl_interface/src/legacy/erl_marshal.c index c57c552b90..18315bfbd3 100644 --- a/lib/erl_interface/src/legacy/erl_marshal.c +++ b/lib/erl_interface/src/legacy/erl_marshal.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "erl_interface.h" #include "erl_marshal.h" @@ -178,26 +179,14 @@ int erl_encode_it(ETERM *ep, unsigned char **ext, int dist) return 0; case ERL_INTEGER: - i = ep->uval.ival.i; - /* ERL_SMALL_BIG */ - if ((i > ERL_MAX) || (i < ERL_MIN)) { - *(*ext)++ = ERL_SMALL_BIG_EXT; - *(*ext)++ = 4; /* four bytes */ - if ((*(*ext)++ = ((i>>31) & 0x01))) /* sign byte */ - i = -i; - *(*ext)++ = i & 0xff; /* LSB first */ - *(*ext)++ = (i >> 8) & 0xff; - *(*ext)++ = (i >> 16) & 0xff; - *(*ext)++ = (i >> 24) & 0x7f; /* Don't include the sign bit */ - return 0; - } + i = ep->uval.ival.i; /* SMALL_INTEGER */ if ((i < 256) && (i >= 0)) { *(*ext)++ = ERL_SMALL_INTEGER_EXT; *(*ext)++ = i & 0xff; return 0; } - /* INTEGER */ + /* R14B: Use all 32 bits of INTEGER_EXT */ *(*ext)++ = ERL_INTEGER_EXT; *(*ext)++ = (i >> 24) & 0xff; *(*ext)++ = (i >> 16) & 0xff; @@ -208,23 +197,23 @@ int erl_encode_it(ETERM *ep, unsigned char **ext, int dist) case ERL_U_INTEGER: u = ep->uval.uival.u; /* ERL_U_SMALL_BIG */ - if (u > ERL_MAX) { - *(*ext)++ = ERL_SMALL_BIG_EXT; - *(*ext)++ = 4; /* four bytes */ - *(*ext)++ = 0; /* sign byte */ - *(*ext)++ = u & 0xff; /* LSB first */ - *(*ext)++ = (u >> 8) & 0xff; - *(*ext)++ = (u >> 16) & 0xff; - *(*ext)++ = (u >> 24) & 0xff; - return 0; + if ((int)u < 0) { + *(*ext)++ = ERL_SMALL_BIG_EXT; + *(*ext)++ = 4; /* four bytes */ + *(*ext)++ = 0; /* sign byte */ + *(*ext)++ = u & 0xff; /* LSB first */ + *(*ext)++ = (u >> 8) & 0xff; + *(*ext)++ = (u >> 16) & 0xff; + *(*ext)++ = (u >> 24) & 0xff; + return 0; } /* SMALL_INTEGER */ - if ((u < 256) && (u >= 0)) { + if (u < 256) { *(*ext)++ = ERL_SMALL_INTEGER_EXT; *(*ext)++ = u & 0xff; return 0; } - /* INTEGER */ + /* R14B: Use all 32 bits of INTEGER_EXT */ *(*ext)++ = ERL_INTEGER_EXT; *(*ext)++ = (u >> 24) & 0xff; *(*ext)++ = (u >> 16) & 0xff; @@ -234,29 +223,28 @@ int erl_encode_it(ETERM *ep, unsigned char **ext, int dist) case ERL_LONGLONG: l = ep->uval.llval.i; /* ERL_SMALL_BIG */ - if ((l > ((long long) ERL_MAX)) || - (l < ((long long) ERL_MIN))) { - *(*ext)++ = ERL_SMALL_BIG_EXT; - *(*ext)++ = 8; /* eight bytes */ - if ((*(*ext)++ = ((l>>63) & 0x01))) /* sign byte */ + if (l > ((long long) INT_MAX) || l < ((long long) INT_MIN)) { + *(*ext)++ = ERL_SMALL_BIG_EXT; + *(*ext)++ = 8; + if ((*(*ext)++ = (l<0))) /* sign byte */ l = -l; - *(*ext)++ = l & 0xff; /* LSB first */ + *(*ext)++ = l & 0xff; /* LSB first */ *(*ext)++ = (l >> 8) & 0xff; *(*ext)++ = (l >> 16) & 0xff; - *(*ext)++ = (l >> 24) & 0xff; - *(*ext)++ = (l >> 32) & 0xff; - *(*ext)++ = (l >> 40) & 0xff; - *(*ext)++ = (l >> 48) & 0xff; - *(*ext)++ = (l >> 56) & 0x7f; /* Don't include the sign bit */ + *(*ext)++ = (l >> 24) & 0xff; + *(*ext)++ = (l >> 32) & 0xff; + *(*ext)++ = (l >> 40) & 0xff; + *(*ext)++ = (l >> 48) & 0xff; + *(*ext)++ = (l >> 56) & 0xff; return 0; } /* SMALL_INTEGER */ if ((l < 256) && (l >= 0)) { - *(*ext)++ = ERL_SMALL_INTEGER_EXT; - *(*ext)++ = l & 0xff; - return 0; + *(*ext)++ = ERL_SMALL_INTEGER_EXT; + *(*ext)++ = l & 0xff; + return 0; } - /* INTEGER */ + /* R14B: Use all 32 bits of INTEGER_EXT */ *(*ext)++ = ERL_INTEGER_EXT; *(*ext)++ = (l >> 24) & 0xff; *(*ext)++ = (l >> 16) & 0xff; @@ -267,7 +255,7 @@ int erl_encode_it(ETERM *ep, unsigned char **ext, int dist) case ERL_U_LONGLONG: ul = ep->uval.ullval.u; /* ERL_U_SMALL_BIG */ - if (ul > ((unsigned long long) ERL_MAX)) { + if (ul > ((unsigned long long) INT_MAX)) { *(*ext)++ = ERL_SMALL_BIG_EXT; *(*ext)++ = 8; /* eight bytes */ *(*ext)++ = 0; /* sign byte */ @@ -287,7 +275,7 @@ int erl_encode_it(ETERM *ep, unsigned char **ext, int dist) *(*ext)++ = ul & 0xff; return 0; } - /* INTEGER */ + /* R14B: Use all 32 bits of INTEGER_EXT */ *(*ext)++ = ERL_INTEGER_EXT; *(*ext)++ = (ul >> 24) & 0xff; *(*ext)++ = (ul >> 16) & 0xff; @@ -732,11 +720,6 @@ static ETERM *erl_decode_it(unsigned char **ext) if (arity > 8) goto big_truncate; - if (arity == 8 && ((*ext)[7] & 0x80) && sign) { - /* MSB already occupied ! */ - goto big_truncate; - } - if (arity == 4 && ((*ext)[3] & 0x80) && !sign) { /* It will fit into an unsigned int !! */ u = (((*ext)[3] << 24)|((*ext)[2])<< 16|((*ext)[1]) << 8 |(**ext)); @@ -747,14 +730,10 @@ static ETERM *erl_decode_it(unsigned char **ext) return ep; } else if (arity == 4 && !((*ext)[3] & 0x80)) { /* It will fit into an int !! - * Note: It comes in "one's-complement notation" */ - if (sign) - i = (int) (~(((*ext)[3] << 24) | ((*ext)[2])<< 16 | - ((*ext)[1]) << 8 | (**ext)) | (unsigned int) sign); - else - i = (int) (((*ext)[3] << 24) | ((*ext)[2])<< 16 | - ((*ext)[1]) << 8 | (**ext)); + i = (int) (((*ext)[3] << 24) | ((*ext)[2])<< 16 | + ((*ext)[1]) << 8 | (**ext)); + if (sign) i = -i; ERL_TYPE(ep) = ERL_INTEGER; ep->uval.ival.i = i; *ext += arity; @@ -780,8 +759,10 @@ static ETERM *erl_decode_it(unsigned char **ext) for(x = 0 ; x < arity ; x++) { l |= ((long long)(*ext)[x]) << ((long long)(8*x)); } - - if (sign) l = (long long) (~l | (unsigned long long) sign); + if (sign) { + l = -l; + if (l > 0) goto big_truncate; + } ERL_TYPE(ep) = ERL_LONGLONG; ep->uval.llval.i = l; diff --git a/lib/erl_interface/test/ei_decode_SUITE.erl b/lib/erl_interface/test/ei_decode_SUITE.erl index c6858b45ad..09a37409f2 100644 --- a/lib/erl_interface/test/ei_decode_SUITE.erl +++ b/lib/erl_interface/test/ei_decode_SUITE.erl @@ -222,14 +222,16 @@ send_integers(P) -> ?line send_term_as_binary(P,256), % INTEGER_EXT smallest pos (*) ?line send_term_as_binary(P,-1), % INTEGER_EXT largest neg - ?line send_term_as_binary(P, 16#07ffffff), % INTEGER_EXT largest (28 bits) - ?line send_term_as_binary(P,-16#08000000), % INTEGER_EXT smallest - ?line send_term_as_binary(P, 16#08000000), % SMALL_BIG_EXT smallest pos(*) - ?line send_term_as_binary(P,-16#08000001), % SMALL_BIG_EXT largest neg (*) - - ?line send_term_as_binary(P, 16#7fffffff), % SMALL_BIG_EXT largest i32 - ?line send_term_as_binary(P,-16#80000000), % SMALL_BIG_EXT smallest i32 - + ?line send_term_as_binary(P, 16#07ffffff), % INTEGER_EXT old largest (28 bits) + ?line send_term_as_binary(P,-16#08000000), % INTEGER_EXT old smallest + ?line send_term_as_binary(P, 16#08000000), % SMALL_BIG_EXT old smallest pos(*) + ?line send_term_as_binary(P,-16#08000001), % SMALL_BIG_EXT old largest neg (*) + + ?line send_term_as_binary(P, 16#7fffffff), % INTEGER_EXT new largest (32 bits) + ?line send_term_as_binary(P,-16#80000000), % INTEGER_EXT new smallest (32 bis) + ?line send_term_as_binary(P, 16#80000000), % SMALL_BIG_EXT new smallest pos(*) + ?line send_term_as_binary(P,-16#80000001), % SMALL_BIG_EXT new largest neg (*) + case erlang:system_info(wordsize) of 4 -> ?line send_term_as_binary(P, 16#80000000),% SMALL_BIG_EXT u32 @@ -266,15 +268,17 @@ send_integers2(P) -> ?line send_term_as_binary(P,255), % SMALL_INTEGER_EXT largest ?line send_term_as_binary(P,256), % INTEGER_EXT smallest pos (*) ?line send_term_as_binary(P,-1), % INTEGER_EXT largest neg + + ?line send_term_as_binary(P, 16#07ffffff), % INTEGER_EXT old largest (28 bits) + ?line send_term_as_binary(P,-16#08000000), % INTEGER_EXT old smallest + ?line send_term_as_binary(P, 16#08000000), % SMALL_BIG_EXT old smallest pos(*) + ?line send_term_as_binary(P,-16#08000001), % SMALL_BIG_EXT old largest neg (*) - ?line send_term_as_binary(P, 16#07ffffff), % INTEGER_EXT largest (28 bits) - ?line send_term_as_binary(P,-16#08000000), % INTEGER_EXT smallest - ?line send_term_as_binary(P, 16#08000000), % SMALL_BIG_EXT smallest pos(*) - ?line send_term_as_binary(P,-16#08000001), % SMALL_BIG_EXT largest neg (*) + ?line send_term_as_binary(P, 16#7fffffff), % INTEGER_EXT new largest (32 bits) + ?line send_term_as_binary(P,-16#80000000), % INTEGER_EXT new smallest + ?line send_term_as_binary(P, 16#80000000), % SMALL_BIG_EXT new smallest pos(*) + ?line send_term_as_binary(P,-16#80000001), % SMALL_BIG_EXT new largest neg (*) - ?line send_term_as_binary(P, 16#7fffffff), % SMALL_BIG_EXT largest i32 - ?line send_term_as_binary(P,-16#80000000), % SMALL_BIG_EXT smallest i32 - ?line send_term_as_binary(P, 16#80000000),% SMALL_BIG_EXT u32 ?line send_term_as_binary(P, 16#ffffffff),% SMALL_BIG_EXT largest u32 ?line send_term_as_binary(P, 16#7fffffffffff), % largest i48 diff --git a/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c b/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c index 5447e2deb3..b349138ae9 100644 --- a/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c +++ b/lib/erl_interface/test/ei_decode_SUITE_data/ei_decode_test.c @@ -246,13 +246,23 @@ TESTCASE(test_ei_decode_long) EI_DECODE_2 (decode_long, 5, long, 256); EI_DECODE_2 (decode_long, 5, long, -1); + /* Old 28 bit limits for INTEGER_EXT */ EI_DECODE_2 (decode_long, 5, long, 0x07ffffff); EI_DECODE_2 (decode_long, 5, long, -0x08000000); - EI_DECODE_2 (decode_long, 7, long, 0x08000000); - EI_DECODE_2 (decode_long, 7, long, -0x08000001); + EI_DECODE_2 (decode_long, 5, long, 0x08000000); + EI_DECODE_2 (decode_long, 5, long, -0x08000001); - EI_DECODE_2 (decode_long, 7, long, 0x7fffffff); - EI_DECODE_2 (decode_long, 7, long, -ll(0x80000000)); /* Strange :-( */ + /* New 32 bit limits for INTEGER_EXT */ + EI_DECODE_2 (decode_long, 5, long, 0x7fffffff); + EI_DECODE_2 (decode_long, 5, long, -ll(0x80000000)); /* Strange :-( */ + if (sizeof(long) > 4) { + EI_DECODE_2(decode_long, 7, long, 0x80000000); + EI_DECODE_2(decode_long, 7, long, -ll(0x80000001)); + } + else { + EI_DECODE_2_FAIL(decode_long, 7, long, 0x80000000); + EI_DECODE_2_FAIL(decode_long, 7, long, -ll(0x80000001)); + } EI_DECODE_2_FAIL(decode_long, 7, long, 0x80000000); EI_DECODE_2_FAIL(decode_long, 7, long, 0xffffffff); @@ -280,11 +290,13 @@ TESTCASE(test_ei_decode_ulong) EI_DECODE_2 (decode_ulong, 5, unsigned long, 0x07ffffff); EI_DECODE_2_FAIL(decode_ulong, 5, unsigned long, -0x08000000); - EI_DECODE_2 (decode_ulong, 7, unsigned long, 0x08000000); - EI_DECODE_2_FAIL(decode_ulong, 7, unsigned long, -0x08000001); + EI_DECODE_2 (decode_ulong, 5, unsigned long, 0x08000000); + EI_DECODE_2_FAIL(decode_ulong, 5, unsigned long, -0x08000001); - EI_DECODE_2 (decode_ulong, 7, unsigned long, 0x7fffffff); - EI_DECODE_2_FAIL(decode_ulong, 7, unsigned long, -ll(0x80000000)); + EI_DECODE_2 (decode_ulong, 5, unsigned long, 0x7fffffff); + EI_DECODE_2_FAIL(decode_ulong, 5, unsigned long, -ll(0x80000000)); + EI_DECODE_2 (decode_ulong, 7, unsigned long, 0x80000000); + EI_DECODE_2_FAIL(decode_ulong, 7, unsigned long, -ll(0x80000001)); if (sizeof(long) > 4) { EI_DECODE_2 (decode_ulong, 11, unsigned long, ll(0x8000000000000000)); @@ -319,13 +331,14 @@ TESTCASE(test_ei_decode_longlong) EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, 0x07ffffff); EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, -0x08000000); - EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, 0x08000000); - EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, -0x08000001); - - EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, 0x7fffffff); - EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, -ll(0x80000000)); + EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, 0x08000000); + EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, -0x08000001); + EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, 0x7fffffff); + EI_DECODE_2 (decode_longlong, 5, EI_LONGLONG, -ll(0x80000000)); EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, 0x80000000); + EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, -ll(0x80000001)); + EI_DECODE_2 (decode_longlong, 7, EI_LONGLONG, 0xffffffff); EI_DECODE_2 (decode_longlong, 9, EI_LONGLONG, ll(0x7fffffffffff)); @@ -352,13 +365,14 @@ TESTCASE(test_ei_decode_ulonglong) EI_DECODE_2 (decode_ulonglong, 5, EI_ULONGLONG, 0x07ffffff); EI_DECODE_2_FAIL(decode_ulonglong, 5, EI_ULONGLONG, -0x08000000); - EI_DECODE_2 (decode_ulonglong, 7, EI_ULONGLONG, 0x08000000); - EI_DECODE_2_FAIL(decode_ulonglong, 7, EI_ULONGLONG, -0x08000001); - - EI_DECODE_2 (decode_ulonglong, 7, EI_ULONGLONG, 0x7fffffff); - EI_DECODE_2_FAIL(decode_ulonglong, 7, EI_ULONGLONG, -ll(0x80000000)); + EI_DECODE_2 (decode_ulonglong, 5, EI_ULONGLONG, 0x08000000); + EI_DECODE_2_FAIL(decode_ulonglong, 5, EI_ULONGLONG, -0x08000001); + EI_DECODE_2 (decode_ulonglong, 5, EI_ULONGLONG, 0x7fffffff); + EI_DECODE_2_FAIL(decode_ulonglong, 5, EI_ULONGLONG, -ll(0x80000000)); EI_DECODE_2 (decode_ulonglong, 7, EI_ULONGLONG, 0x80000000); + EI_DECODE_2_FAIL(decode_ulonglong, 7, EI_ULONGLONG, -0x80000001); + EI_DECODE_2 (decode_ulonglong, 7, EI_ULONGLONG, 0xffffffff); EI_DECODE_2 (decode_ulonglong, 9, EI_ULONGLONG, ll(0x7fffffffffff)); diff --git a/lib/erl_interface/test/erl_eterm_SUITE_data/eterm_test.c b/lib/erl_interface/test/erl_eterm_SUITE_data/eterm_test.c index 6b2ec8f766..f273efd532 100644 --- a/lib/erl_interface/test/erl_eterm_SUITE_data/eterm_test.c +++ b/lib/erl_interface/test/erl_eterm_SUITE_data/eterm_test.c @@ -63,32 +63,108 @@ TESTCASE(build_terms) report(1); } +static int abs_and_sign(ETERM* v, unsigned long long* av, int* sign) +{ + long long sv; + switch (ERL_TYPE(v)) { + case ERL_INTEGER: sv = ERL_INT_VALUE(v); break; + case ERL_U_INTEGER: *av = ERL_INT_UVALUE(v); *sign = 0; return 1; + case ERL_LONGLONG: sv = ERL_LL_VALUE(v); break; + case ERL_U_LONGLONG: *av = ERL_LL_UVALUE(v); *sign = 0; return 1; + default: return 0; + } + if (sv < 0) { + *av = -sv; + *sign = 1; + } + else { + *av = sv; + *sign = 0; + } + return 1; +} + +/* Shouldn't erl_match() cope with this? +*/ +static int eq_ints(ETERM* a, ETERM* b) +{ + unsigned long long a_abs, b_abs; + int a_sign, b_sign; + return abs_and_sign(a, &a_abs, &a_sign) && abs_and_sign(b, &b_abs, &b_sign) + && (a_abs == b_abs) && (a_sign == b_sign); +} + +static void encode_decode(ETERM* original, const char* text) +{ + static unsigned char encoded[16*1024]; + ETERM* new_terms; + int bytes = erl_encode(original, encoded); + + if (bytes == 0) { + fail("failed to encode terms"); + } + else if (bytes > sizeof(encoded)) { + fail("encoded terms buffer overflow"); + } + else if ((new_terms = erl_decode(encoded)) == NULL) { + fail("failed to decode terms"); + } + else if (!erl_match(original, new_terms) && !eq_ints(original, new_terms)) { + erl_print_term(stderr, original); + fprintf(stderr, "(%i) != (%i)", ERL_TYPE(original), ERL_TYPE(new_terms)); + erl_print_term(stderr, new_terms); + fprintf(stderr, " [%s]\r\n", text); + fail("decoded terms didn't match original"); + } + erl_free_term(original); + erl_free_term(new_terms); +} /* * Converts an Erlang term to the external term format and back again. */ TESTCASE(round_trip_conversion) { - ETERM* original; - ETERM* new_terms; - char encoded[16*1024]; - int n; + int n, i; erl_init(NULL, 0); - original = all_types(); - if (erl_encode(original, encoded) == 0) + encode_decode(all_types(), "ALL"); + { - fail("failed to encode terms"); - } else if ((new_terms = erl_decode(encoded)) == NULL) + int v; + for (v = 8; v; v <<= 1) { + for (i=-4; i<4; i++) { + encode_decode(erl_mk_int(v+i), "INT"); + encode_decode(erl_mk_int(-(v+i)), "NEG INT"); + } + } + } { - fail("failed to decode terms"); - } else if (!erl_match(original, new_terms)) + unsigned int v; + for (v = 8; v; v <<= 1) { + for (i=-4; i<4; i++) { + encode_decode(erl_mk_uint(v+i), "UINT"); + } + } + } { - fail("decoded terms didn't match original"); + long long v; + for (v = 8; v; v <<= 1) { + for (i=-4; i<4; i++) { + encode_decode(erl_mk_longlong(v+i), "LONGLONG"); + encode_decode(erl_mk_longlong(-(v+i)), "NEG LONGLONG"); + } + } + } + { + unsigned long long v; + for (v = 8; v; v <<= 1) { + for (i=-4; i<4; i++) { + encode_decode(erl_mk_ulonglong(v+i), "ULONGLONG"); + } + } } - erl_free_term(original); - erl_free_term(new_terms); report(1); } -- cgit v1.2.3