aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Zhu <peter@peterzhu.ca>2023-04-04 09:27:45 -0400
committerPeter Zhu <peter@peterzhu.ca>2023-04-04 11:12:07 -0400
commita84c99468f26a9f79fec57926d561ed906505eac (patch)
tree86c94feef11c4682f4330ed67dc3bc2be15a09d5
parent06da0d1456f52a69ad19a0a8cc35e3359d2d144c (diff)
downloadruby-a84c99468f26a9f79fec57926d561ed906505eac.tar.gz
Fix crash in Time on 32-bit systems
[Bug #19575] struct vtm is packed causing it to have a size that is not aligned on 32-bit systems. When allocating it on the stack, it will have unaligned addresses which means that the fields won't be marked by the GC when scanning the stack (since the GC only marks aligned addresses). This can cause crashes when the fields are heap allocated objects like Bignums. This commit moves the flags in struct time_object into struct vtm for space efficiency and removes the need for packing. This is an example of a crash: ruby(rb_print_backtrace+0xd) [0x56848945] ../src/vm_dump.c:785 ruby(rb_vm_bugreport) ../src/vm_dump.c:1101 ruby(rb_assert_failure+0x7a) [0x56671857] ../src/error.c:878 ruby(vm_search_cc+0x0) [0x56666e47] ../src/vm_method.c:1366 ruby(rb_vm_search_method_slowpath) ../src/vm_insnhelper.c:2090 ruby(callable_method_entry+0x5) [0x568232d3] ../src/vm_method.c:1406 ruby(rb_callable_method_entry) ../src/vm_method.c:1413 ruby(gccct_method_search_slowpath) ../src/vm_eval.c:427 ruby(gccct_method_search+0x20f) [0x568237ef] ../src/vm_eval.c:476 ruby(opt_equality_by_mid_slowpath+0x2c) [0x5682388c] ../src/vm_insnhelper.c:2338 ruby(rb_equal+0x37) [0x566fe577] ../src/object.c:133 ruby(rb_big_eq+0x34) [0x56876ee4] ../src/bignum.c:5554 ruby(rb_int_equal+0x14) [0x566f3ed4] ../src/numeric.c:4640 ruby(rb_int_equal) ../src/numeric.c:4634 ruby(vm_call0_cfunc_with_frame+0x6d) [0x568303c2] ../src/vm_eval.c:148 ruby(vm_call0_cfunc) ../src/vm_eval.c:162 ruby(vm_call0_body) ../src/vm_eval.c:208 ruby(rb_funcallv_scope+0xd1) [0x56833971] ../src/vm_eval.c:85 ruby(RB_TEST+0x0) [0x567e8488] ../src/time.c:78 ruby(eq) ../src/time.c:78 ruby(small_vtm_sub) ../src/time.c:1523 ruby(timelocalw+0x23b) [0x567f3e9b] ../src/time.c:1593 ruby(time_s_alloc+0x0) [0x567f536b] ../src/time.c:3698 ruby(time_new_timew) ../src/time.c:2694 ruby(time_s_mktime) ../src/time.c:3698
-rw-r--r--test/ruby/test_time.rb7
-rw-r--r--time.c57
-rw-r--r--timev.h7
3 files changed, 34 insertions, 37 deletions
diff --git a/test/ruby/test_time.rb b/test/ruby/test_time.rb
index 2bf6056a49..f47a49d3e0 100644
--- a/test/ruby/test_time.rb
+++ b/test/ruby/test_time.rb
@@ -1403,11 +1403,8 @@ class TestTime < Test::Unit::TestCase
else
RbConfig::SIZEOF["void*"] # Same size as VALUE
end
- expect =
- GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] +
- sizeof_timew +
- RbConfig::SIZEOF["void*"] * 4 + 5 + # vtm
- 1 # tzmode, tm_got
+ sizeof_vtm = RbConfig::SIZEOF["void*"] * 4 + 8
+ expect = GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] + sizeof_timew + sizeof_vtm
assert_equal expect, ObjectSpace.memsize_of(t)
rescue LoadError => e
omit "failed to load objspace: #{e.message}"
diff --git a/time.c b/time.c
index 343da2a52b..4dbdd3cf7d 100644
--- a/time.c
+++ b/time.c
@@ -1754,34 +1754,31 @@ localtimew(wideval_t timew, struct vtm *result)
#define TIME_TZMODE_FIXOFF 2
#define TIME_TZMODE_UNINITIALIZED 3
-RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_BEGIN()
struct time_object {
wideval_t timew; /* time_t value * TIME_SCALE. possibly Rational. */
struct vtm vtm;
- unsigned int tzmode:3; /* 0:localtime 1:utc 2:fixoff 3:uninitialized */
- unsigned int tm_got:1;
-} RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_END();
+};
#define GetTimeval(obj, tobj) ((tobj) = get_timeval(obj))
#define GetNewTimeval(obj, tobj) ((tobj) = get_new_timeval(obj))
#define IsTimeval(obj) rb_typeddata_is_kind_of((obj), &time_data_type)
-#define TIME_INIT_P(tobj) ((tobj)->tzmode != TIME_TZMODE_UNINITIALIZED)
+#define TIME_INIT_P(tobj) ((tobj)->vtm.tzmode != TIME_TZMODE_UNINITIALIZED)
-#define TZMODE_UTC_P(tobj) ((tobj)->tzmode == TIME_TZMODE_UTC)
-#define TZMODE_SET_UTC(tobj) ((tobj)->tzmode = TIME_TZMODE_UTC)
+#define TZMODE_UTC_P(tobj) ((tobj)->vtm.tzmode == TIME_TZMODE_UTC)
+#define TZMODE_SET_UTC(tobj) ((tobj)->vtm.tzmode = TIME_TZMODE_UTC)
-#define TZMODE_LOCALTIME_P(tobj) ((tobj)->tzmode == TIME_TZMODE_LOCALTIME)
-#define TZMODE_SET_LOCALTIME(tobj) ((tobj)->tzmode = TIME_TZMODE_LOCALTIME)
+#define TZMODE_LOCALTIME_P(tobj) ((tobj)->vtm.tzmode == TIME_TZMODE_LOCALTIME)
+#define TZMODE_SET_LOCALTIME(tobj) ((tobj)->vtm.tzmode = TIME_TZMODE_LOCALTIME)
-#define TZMODE_FIXOFF_P(tobj) ((tobj)->tzmode == TIME_TZMODE_FIXOFF)
+#define TZMODE_FIXOFF_P(tobj) ((tobj)->vtm.tzmode == TIME_TZMODE_FIXOFF)
#define TZMODE_SET_FIXOFF(time, tobj, off) do { \
- (tobj)->tzmode = TIME_TZMODE_FIXOFF; \
+ (tobj)->vtm.tzmode = TIME_TZMODE_FIXOFF; \
RB_OBJ_WRITE_UNALIGNED(time, &(tobj)->vtm.utc_offset, off); \
} while (0)
#define TZMODE_COPY(tobj1, tobj2) \
- ((tobj1)->tzmode = (tobj2)->tzmode, \
+ ((tobj1)->vtm.tzmode = (tobj2)->vtm.tzmode, \
(tobj1)->vtm.utc_offset = (tobj2)->vtm.utc_offset, \
(tobj1)->vtm.zone = (tobj2)->vtm.zone)
@@ -1789,7 +1786,7 @@ static int zone_localtime(VALUE zone, VALUE time);
static VALUE time_get_tm(VALUE, struct time_object *);
#define MAKE_TM(time, tobj) \
do { \
- if ((tobj)->tm_got == 0) { \
+ if ((tobj)->vtm.tm_got == 0) { \
time_get_tm((time), (tobj)); \
} \
} while (0)
@@ -1828,7 +1825,7 @@ force_make_tm(VALUE time, struct time_object *tobj)
if (!NIL_P(zone) && zone != str_empty && zone != str_utc) {
if (zone_localtime(zone, time)) return;
}
- tobj->tm_got = 0;
+ tobj->vtm.tm_got = 0;
time_get_tm(time, tobj);
}
@@ -1864,8 +1861,8 @@ time_s_alloc(VALUE klass)
struct time_object *tobj;
obj = TypedData_Make_Struct(klass, struct time_object, &time_data_type, tobj);
- tobj->tzmode = TIME_TZMODE_UNINITIALIZED;
- tobj->tm_got = 0;
+ tobj->vtm.tzmode = TIME_TZMODE_UNINITIALIZED;
+ tobj->vtm.tm_got = 0;
time_set_timew(obj, tobj, WINT2FIXWV(0));
tobj->vtm.zone = Qnil;
@@ -1972,7 +1969,7 @@ time_init_now(rb_execution_context_t *ec, VALUE time, VALUE zone)
time_modify(time);
GetNewTimeval(time, tobj);
TZMODE_SET_LOCALTIME(tobj);
- tobj->tm_got=0;
+ tobj->vtm.tm_got=0;
rb_timespec_now(&ts);
time_set_timew(time, tobj, timenano2timew(ts.tv_sec, ts.tv_nsec));
@@ -1998,7 +1995,7 @@ time_set_utc_offset(VALUE time, VALUE off)
time_modify(time);
GetTimeval(time, tobj);
- tobj->tm_got = 0;
+ tobj->vtm.tm_got = 0;
tobj->vtm.zone = Qnil;
TZMODE_SET_FIXOFF(time, tobj, off);
@@ -2374,7 +2371,7 @@ zone_localtime(VALUE zone, VALUE time)
if (UNDEF_P(local)) return 0;
s = extract_vtm(local, time, tobj, subsecx);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
zone_set_offset(zone, tobj, s, t);
zone_set_dst(zone, tobj, tm);
return 1;
@@ -2468,7 +2465,7 @@ time_init_vtm(VALUE time, struct vtm vtm, VALUE zone)
time_set_timew(time, tobj, timegmw(&vtm));
vtm_day_wraparound(&vtm);
time_set_vtm(time, tobj, vtm);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
TZMODE_SET_LOCALTIME(tobj);
if (zone_timelocal(zone, time)) {
return time;
@@ -2484,13 +2481,13 @@ time_init_vtm(VALUE time, struct vtm vtm, VALUE zone)
vtm.isdst = 0; /* No DST in UTC */
vtm_day_wraparound(&vtm);
time_set_vtm(time, tobj, vtm);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
TZMODE_SET_UTC(tobj);
return time;
}
TZMODE_SET_LOCALTIME(tobj);
- tobj->tm_got=0;
+ tobj->vtm.tm_got=0;
if (!NIL_P(vtm.utc_offset)) {
VALUE off = vtm.utc_offset;
@@ -3996,7 +3993,7 @@ time_localtime(VALUE time)
GetTimeval(time, tobj);
if (TZMODE_LOCALTIME_P(tobj)) {
- if (tobj->tm_got)
+ if (tobj->vtm.tm_got)
return time;
}
else {
@@ -4012,7 +4009,7 @@ time_localtime(VALUE time)
rb_raise(rb_eArgError, "localtime error");
time_set_vtm(time, tobj, vtm);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
TZMODE_SET_LOCALTIME(tobj);
return time;
}
@@ -4097,7 +4094,7 @@ time_gmtime(VALUE time)
GetTimeval(time, tobj);
if (TZMODE_UTC_P(tobj)) {
- if (tobj->tm_got)
+ if (tobj->vtm.tm_got)
return time;
}
else {
@@ -4108,7 +4105,7 @@ time_gmtime(VALUE time)
GMTIMEW(tobj->timew, &vtm);
time_set_vtm(time, tobj, vtm);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
TZMODE_SET_UTC(tobj);
return time;
}
@@ -4122,7 +4119,7 @@ time_fixoff(VALUE time)
GetTimeval(time, tobj);
if (TZMODE_FIXOFF_P(tobj)) {
- if (tobj->tm_got)
+ if (tobj->vtm.tm_got)
return time;
}
else {
@@ -4142,7 +4139,7 @@ time_fixoff(VALUE time)
time_set_vtm(time, tobj, vtm);
RB_OBJ_WRITE_UNALIGNED(time, &tobj->vtm.zone, zone);
- tobj->tm_got = 1;
+ tobj->vtm.tm_got = 1;
TZMODE_SET_FIXOFF(time, tobj, off);
return time;
}
@@ -5496,7 +5493,7 @@ end_submicro: ;
GetNewTimeval(time, tobj);
TZMODE_SET_LOCALTIME(tobj);
- tobj->tm_got = 0;
+ tobj->vtm.tm_got = 0;
time_set_timew(time, tobj, timew);
if (gmt) {
@@ -5561,7 +5558,7 @@ tm_from_time(VALUE klass, VALUE time)
v->zone = Qnil;
time_set_vtm(tm, ttm, *v);
- ttm->tm_got = 1;
+ ttm->vtm.tm_got = 1;
TZMODE_SET_UTC(ttm);
return tm;
}
diff --git a/timev.h b/timev.h
index d667b60b14..d350a170de 100644
--- a/timev.h
+++ b/timev.h
@@ -2,7 +2,6 @@
#define RUBY_TIMEV_H
#include "ruby/ruby.h"
-RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_BEGIN()
struct vtm {
VALUE year; /* 2000 for example. Integer. */
VALUE subsecx; /* 0 <= subsecx < TIME_SCALE. possibly Rational. */
@@ -16,7 +15,11 @@ struct vtm {
unsigned int sec:6; /* 0..60 */
unsigned int wday:3; /* 0:Sunday, 1:Monday, ..., 6:Saturday 7:init */
unsigned int isdst:2; /* 0:StandardTime 1:DayLightSavingTime 3:init */
-} RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_END();
+
+ /* Flags for struct time_object */
+ unsigned int tzmode:3; /* 0:localtime 1:utc 2:fixoff 3:uninitialized */
+ unsigned int tm_got:1;
+};
#define TIME_SCALE 1000000000