From 9c36499e311a62f92127829d96f7eb638b49001f Mon Sep 17 00:00:00 2001 From: dearblue Date: Thu, 28 Jan 2021 21:40:38 +0900 Subject: Fixed `String#unpack` to handle the highest range of integer values Previously, problems occurred when the `fixnum` was exceeded. - 32-bit cpu mode with `MRB_WORD_BOXING` and `MRB_INT32`: ```console % bin/mruby -e 'p [0x7fffffff].pack("N").unpack("N")' trace (most recent call last): -e:1: cannot unpack to Integer: 2147483647 (RangeError) ``` - 64-bit cpu mode with `MRB_WORD_BOXING` and `MRB_INT64`: ```console % bin/mruby -e 'p [0x7fffffff_ffffffff].pack("q").unpack("q")' trace (most recent call last): -e:1: cannot unpack to Integer: 9223372036854775807 (RangeError) ``` --- mrbgems/mruby-pack/src/pack.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/mrbgems/mruby-pack/src/pack.c b/mrbgems/mruby-pack/src/pack.c index 014679a5d..58c2a3be7 100644 --- a/mrbgems/mruby-pack/src/pack.c +++ b/mrbgems/mruby-pack/src/pack.c @@ -15,6 +15,9 @@ #include #include +#define INT_OVERFLOW_P(n) ((n) < MRB_INT_MIN || (n) > MRB_INT_MAX) +#define UINT_OVERFLOW_P(n) ((n) > MRB_INT_MAX) + struct tmpl { mrb_value str; int idx; @@ -258,7 +261,7 @@ unpack_l(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, un if (flags & PACK_FLAG_SIGNED) { int32_t sl = ul; #ifndef MRB_INT64 - if (!FIXABLE(sl)) { + if (INT_OVERFLOW_P(sl)) { i32tostr(msg, sizeof(msg), sl); mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); } @@ -266,14 +269,14 @@ unpack_l(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, un n = sl; } else { #ifndef MRB_INT64 - if (!POSFIXABLE(ul)) { + if (UINT_OVERFLOW_P(ul)) { u32tostr(msg, sizeof(msg), ul); mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); } #endif n = ul; } - mrb_ary_push(mrb, ary, mrb_fixnum_value(n)); + mrb_ary_push(mrb, ary, mrb_int_value(mrb, n)); return 4; } @@ -379,19 +382,19 @@ unpack_q(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, un } if (flags & PACK_FLAG_SIGNED) { int64_t sll = ull; - if (!FIXABLE(sll)) { + if (INT_OVERFLOW_P(sll)) { i64tostr(msg, sizeof(msg), sll); mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); } n = (mrb_int)sll; } else { - if (!POSFIXABLE(ull)) { + if (UINT_OVERFLOW_P(ull)) { u64tostr(msg, sizeof(msg), ull); mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); } n = (mrb_int)ull; } - mrb_ary_push(mrb, ary, mrb_fixnum_value(n)); + mrb_ary_push(mrb, ary, mrb_int_value(mrb, n)); return 8; } -- cgit v1.2.3 From b500e8295001849cf96e88d7f0a86d517299ff7d Mon Sep 17 00:00:00 2001 From: dearblue Date: Fri, 29 Jan 2021 22:23:22 +0900 Subject: Remove unnecessary range confirmation This is a correction based on the review by @matz. https://github.com/mruby/mruby/pull/5306#pullrequestreview-578378401 --- mrbgems/mruby-pack/src/pack.c | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/mrbgems/mruby-pack/src/pack.c b/mrbgems/mruby-pack/src/pack.c index 58c2a3be7..b0da7b06b 100644 --- a/mrbgems/mruby-pack/src/pack.c +++ b/mrbgems/mruby-pack/src/pack.c @@ -216,26 +216,6 @@ u32tostr(char *buf, size_t len, uint32_t n) snprintf(buf, len, "%" PRIu32, n); #endif /* MRB_NO_STDIO */ } - -static void -i32tostr(char *buf, size_t len, int32_t n) -{ -#ifdef MRB_NO_STDIO - if (len < 1) { - return; - } - - if (n < 0) { - *buf ++ = '-'; - len --; - n = -n; - } - - u32tostr(buf, len, (uint32_t)n); -#else - snprintf(buf, len, "%" PRId32, n); -#endif /* MRB_NO_STDIO */ -} #endif /* MRB_INT64 */ static int @@ -259,14 +239,7 @@ unpack_l(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, un ul += (uint32_t)src[3]; } if (flags & PACK_FLAG_SIGNED) { - int32_t sl = ul; -#ifndef MRB_INT64 - if (INT_OVERFLOW_P(sl)) { - i32tostr(msg, sizeof(msg), sl); - mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); - } -#endif - n = sl; + n = (int32_t)ul; } else { #ifndef MRB_INT64 if (UINT_OVERFLOW_P(ul)) { @@ -340,6 +313,7 @@ u64tostr(char *buf, size_t len, uint64_t n) #endif /* MRB_NO_STDIO */ } +#ifndef MRB_INT64 static void i64tostr(char *buf, size_t len, int64_t n) { @@ -359,6 +333,7 @@ i64tostr(char *buf, size_t len, int64_t n) snprintf(buf, len, "%" PRId64, n); #endif /* MRB_NO_STDIO */ } +#endif /* MRB_INT64 */ static int unpack_q(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, unsigned int flags) @@ -382,10 +357,12 @@ unpack_q(mrb_state *mrb, const unsigned char *src, int srclen, mrb_value ary, un } if (flags & PACK_FLAG_SIGNED) { int64_t sll = ull; +#ifndef MRB_INT64 if (INT_OVERFLOW_P(sll)) { i64tostr(msg, sizeof(msg), sll); mrb_raisef(mrb, E_RANGE_ERROR, "cannot unpack to Integer: %s", msg); } +#endif n = (mrb_int)sll; } else { if (UINT_OVERFLOW_P(ull)) { -- cgit v1.2.3