1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix crash in Time on 32-bit systems

Browse Source 
[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
This commit is contained in:
Peter Zhu 2023-04-04 09:27:45 -04:00
parent 06da0d1456
commit a84c99468f
Notes: git 2023-04-04 15:12:30 +00:00 
Merged: https://github.com/ruby/ruby/pull/7650
3 changed files with 34 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
test/ruby/test_time.rb
View File

@ -1403,11 +1403,8 @@ class TestTime < Test::Unit::TestCase
else else
RbConfig::SIZEOF["void*"] # Same size as VALUE RbConfig::SIZEOF["void*"] # Same size as VALUE
end end
expect = sizeof_vtm = RbConfig::SIZEOF["void*"] * 4 + 8
GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] + expect = GC::INTERNAL_CONSTANTS[:BASE_SLOT_SIZE] + sizeof_timew + sizeof_vtm
sizeof_timew +
RbConfig::SIZEOF["void*"] * 4 + 5 + # vtm
1 # tzmode, tm_got
assert_equal expect, ObjectSpace.memsize_of(t) assert_equal expect, ObjectSpace.memsize_of(t)
rescue LoadError => e rescue LoadError => e
omit "failed to load objspace: #{e.message}" omit "failed to load objspace: #{e.message}"

57
time.c
View File

@ -1754,34 +1754,31 @@ localtimew(wideval_t timew, struct vtm *result)
#define TIME_TZMODE_FIXOFF 2 #define TIME_TZMODE_FIXOFF 2
#define TIME_TZMODE_UNINITIALIZED 3 #define TIME_TZMODE_UNINITIALIZED 3
RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_BEGIN()
struct time_object { struct time_object {
wideval_t timew; /* time_t value * TIME_SCALE. possibly Rational. */ wideval_t timew; /* time_t value * TIME_SCALE. possibly Rational. */
struct vtm vtm; 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 GetTimeval(obj, tobj) ((tobj) = get_timeval(obj))
#define GetNewTimeval(obj, tobj) ((tobj) = get_new_timeval(obj)) #define GetNewTimeval(obj, tobj) ((tobj) = get_new_timeval(obj))
#define IsTimeval(obj) rb_typeddata_is_kind_of((obj), &time_data_type) #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_UTC_P(tobj) ((tobj)->vtm.tzmode == TIME_TZMODE_UTC)
#define TZMODE_SET_UTC(tobj) ((tobj)->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_LOCALTIME_P(tobj) ((tobj)->vtm.tzmode == TIME_TZMODE_LOCALTIME)
#define TZMODE_SET_LOCALTIME(tobj) ((tobj)->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 { \ #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); \ RB_OBJ_WRITE_UNALIGNED(time, &(tobj)->vtm.utc_offset, off); \
} while (0) } while (0)
#define TZMODE_COPY(tobj1, tobj2) \ #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.utc_offset = (tobj2)->vtm.utc_offset, \
(tobj1)->vtm.zone = (tobj2)->vtm.zone) (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 *); static VALUE time_get_tm(VALUE, struct time_object *);
#define MAKE_TM(time, tobj) \ #define MAKE_TM(time, tobj) \
do { \ do { \
if ((tobj)->tm_got == 0) { \ if ((tobj)->vtm.tm_got == 0) { \
time_get_tm((time), (tobj)); \ time_get_tm((time), (tobj)); \
} \ } \
} while (0) } 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 (!NIL_P(zone) && zone != str_empty && zone != str_utc) {
if (zone_localtime(zone, time)) return; if (zone_localtime(zone, time)) return;
} }
tobj->tm_got = 0; tobj->vtm.tm_got = 0;
time_get_tm(time, tobj); time_get_tm(time, tobj);
} }
@ -1864,8 +1861,8 @@ time_s_alloc(VALUE klass)
struct time_object *tobj; struct time_object *tobj;
obj = TypedData_Make_Struct(klass, struct time_object, &time_data_type, tobj); obj = TypedData_Make_Struct(klass, struct time_object, &time_data_type, tobj);
tobj->tzmode = TIME_TZMODE_UNINITIALIZED; tobj->vtm.tzmode = TIME_TZMODE_UNINITIALIZED;
tobj->tm_got = 0; tobj->vtm.tm_got = 0;
time_set_timew(obj, tobj, WINT2FIXWV(0)); time_set_timew(obj, tobj, WINT2FIXWV(0));
tobj->vtm.zone = Qnil; tobj->vtm.zone = Qnil;
@ -1972,7 +1969,7 @@ time_init_now(rb_execution_context_t *ec, VALUE time, VALUE zone)
time_modify(time); time_modify(time);
GetNewTimeval(time, tobj); GetNewTimeval(time, tobj);
TZMODE_SET_LOCALTIME(tobj); TZMODE_SET_LOCALTIME(tobj);
tobj->tm_got=0; tobj->vtm.tm_got=0;
rb_timespec_now(&ts); rb_timespec_now(&ts);
time_set_timew(time, tobj, timenano2timew(ts.tv_sec, ts.tv_nsec)); 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); time_modify(time);
GetTimeval(time, tobj); GetTimeval(time, tobj);
tobj->tm_got = 0; tobj->vtm.tm_got = 0;
tobj->vtm.zone = Qnil; tobj->vtm.zone = Qnil;
TZMODE_SET_FIXOFF(time, tobj, off); TZMODE_SET_FIXOFF(time, tobj, off);
@ -2374,7 +2371,7 @@ zone_localtime(VALUE zone, VALUE time)
if (UNDEF_P(local)) return 0; if (UNDEF_P(local)) return 0;
s = extract_vtm(local, time, tobj, subsecx); 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_offset(zone, tobj, s, t);
zone_set_dst(zone, tobj, tm); zone_set_dst(zone, tobj, tm);
return 1; return 1;
@ -2468,7 +2465,7 @@ time_init_vtm(VALUE time, struct vtm vtm, VALUE zone)
time_set_timew(time, tobj, timegmw(&vtm)); time_set_timew(time, tobj, timegmw(&vtm));
vtm_day_wraparound(&vtm); vtm_day_wraparound(&vtm);
time_set_vtm(time, tobj, vtm); time_set_vtm(time, tobj, vtm);
tobj->tm_got = 1; tobj->vtm.tm_got = 1;
TZMODE_SET_LOCALTIME(tobj); TZMODE_SET_LOCALTIME(tobj);
if (zone_timelocal(zone, time)) { if (zone_timelocal(zone, time)) {
return 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.isdst = 0; /* No DST in UTC */
vtm_day_wraparound(&vtm); vtm_day_wraparound(&vtm);
time_set_vtm(time, tobj, vtm); time_set_vtm(time, tobj, vtm);
tobj->tm_got = 1; tobj->vtm.tm_got = 1;
TZMODE_SET_UTC(tobj); TZMODE_SET_UTC(tobj);
return time; return time;
} }
TZMODE_SET_LOCALTIME(tobj); TZMODE_SET_LOCALTIME(tobj);
tobj->tm_got=0; tobj->vtm.tm_got=0;
if (!NIL_P(vtm.utc_offset)) { if (!NIL_P(vtm.utc_offset)) {
VALUE off = vtm.utc_offset; VALUE off = vtm.utc_offset;
@ -3996,7 +3993,7 @@ time_localtime(VALUE time)
GetTimeval(time, tobj); GetTimeval(time, tobj);
if (TZMODE_LOCALTIME_P(tobj)) { if (TZMODE_LOCALTIME_P(tobj)) {
if (tobj->tm_got) if (tobj->vtm.tm_got)
return time; return time;
} }
else { else {
@ -4012,7 +4009,7 @@ time_localtime(VALUE time)
rb_raise(rb_eArgError, "localtime error"); rb_raise(rb_eArgError, "localtime error");
time_set_vtm(time, tobj, vtm); time_set_vtm(time, tobj, vtm);
tobj->tm_got = 1; tobj->vtm.tm_got = 1;
TZMODE_SET_LOCALTIME(tobj); TZMODE_SET_LOCALTIME(tobj);
return time; return time;
} }
@ -4097,7 +4094,7 @@ time_gmtime(VALUE time)
GetTimeval(time, tobj); GetTimeval(time, tobj);
if (TZMODE_UTC_P(tobj)) { if (TZMODE_UTC_P(tobj)) {
if (tobj->tm_got) if (tobj->vtm.tm_got)
return time; return time;
} }
else { else {
@ -4108,7 +4105,7 @@ time_gmtime(VALUE time)
GMTIMEW(tobj->timew, &vtm); GMTIMEW(tobj->timew, &vtm);
time_set_vtm(time, tobj, vtm); time_set_vtm(time, tobj, vtm);
tobj->tm_got = 1; tobj->vtm.tm_got = 1;
TZMODE_SET_UTC(tobj); TZMODE_SET_UTC(tobj);
return time; return time;
} }
@ -4122,7 +4119,7 @@ time_fixoff(VALUE time)
GetTimeval(time, tobj); GetTimeval(time, tobj);
if (TZMODE_FIXOFF_P(tobj)) { if (TZMODE_FIXOFF_P(tobj)) {
if (tobj->tm_got) if (tobj->vtm.tm_got)
return time; return time;
} }
else { else {
@ -4142,7 +4139,7 @@ time_fixoff(VALUE time)
time_set_vtm(time, tobj, vtm); time_set_vtm(time, tobj, vtm);
RB_OBJ_WRITE_UNALIGNED(time, &tobj->vtm.zone, zone); RB_OBJ_WRITE_UNALIGNED(time, &tobj->vtm.zone, zone);
tobj->tm_got = 1; tobj->vtm.tm_got = 1;
TZMODE_SET_FIXOFF(time, tobj, off); TZMODE_SET_FIXOFF(time, tobj, off);
return time; return time;
} }
@ -5496,7 +5493,7 @@ end_submicro: ;
GetNewTimeval(time, tobj); GetNewTimeval(time, tobj);
TZMODE_SET_LOCALTIME(tobj); TZMODE_SET_LOCALTIME(tobj);
tobj->tm_got = 0; tobj->vtm.tm_got = 0;
time_set_timew(time, tobj, timew); time_set_timew(time, tobj, timew);
if (gmt) { if (gmt) {
@ -5561,7 +5558,7 @@ tm_from_time(VALUE klass, VALUE time)
v->zone = Qnil; v->zone = Qnil;
time_set_vtm(tm, ttm, *v); time_set_vtm(tm, ttm, *v);
ttm->tm_got = 1; ttm->vtm.tm_got = 1;
TZMODE_SET_UTC(ttm); TZMODE_SET_UTC(ttm);
return tm; return tm;
} }

7
timev.h
View File

@ -2,7 +2,6 @@
#define RUBY_TIMEV_H #define RUBY_TIMEV_H
#include "ruby/ruby.h" #include "ruby/ruby.h"
RBIMPL_ATTR_PACKED_STRUCT_UNALIGNED_BEGIN()
struct vtm { struct vtm {
VALUE year; /* 2000 for example. Integer. */ VALUE year; /* 2000 for example. Integer. */
VALUE subsecx; /* 0 <= subsecx < TIME_SCALE. possibly Rational. */ VALUE subsecx; /* 0 <= subsecx < TIME_SCALE. possibly Rational. */
@ -16,7 +15,11 @@ struct vtm {
unsigned int sec:6; /* 0..60 */ unsigned int sec:6; /* 0..60 */
unsigned int wday:3; /* 0:Sunday, 1:Monday, ..., 6:Saturday 7:init */ unsigned int wday:3; /* 0:Sunday, 1:Monday, ..., 6:Saturday 7:init */
unsigned int isdst:2; /* 0:StandardTime 1:DayLightSavingTime 3: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 #define TIME_SCALE 1000000000