Skip to content

Commit 526e73d

Browse files
committed
make shape code use the tydesc found in the box, not the shape str
1 parent 3a1e33e commit 526e73d

File tree

3 files changed

+88
-46
lines changed

3 files changed

+88
-46
lines changed

src/rt/rust_cc.cpp

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ class irc : public shape::data<irc,shape::ptr> {
9393
}
9494

9595
void walk_box2() {
96-
shape::data<irc,shape::ptr>::walk_box_contents1();
96+
// the box ptr can be NULL for env ptrs in closures and data
97+
// not fully initialized
98+
rust_opaque_box *box = *(rust_opaque_box**)dp;
99+
if (box)
100+
shape::data<irc,shape::ptr>::walk_box_contents1();
97101
}
98102

99103
void walk_uniq2() {
@@ -103,8 +107,6 @@ class irc : public shape::data<irc,shape::ptr> {
103107
void walk_fn2(char code) {
104108
switch (code) {
105109
case shape::SHAPE_BOX_FN: {
106-
// Record an irc for the environment box, but don't descend
107-
// into it since it will be walked via the box's allocation
108110
shape::bump_dp<void*>(dp); // skip over the code ptr
109111
walk_box2(); // walk over the environment ptr
110112
break;
@@ -137,19 +139,19 @@ class irc : public shape::data<irc,shape::ptr> {
137139

138140
void walk_uniq_contents2(irc &sub) { sub.walk(); }
139141

140-
void walk_box_contents2(irc &sub, shape::ptr &box_dp) {
141-
maybe_record_irc(box_dp);
142+
void walk_box_contents2(irc &sub) {
143+
maybe_record_irc();
142144

143145
// Do not traverse the contents of this box; it's in the allocation
144146
// somewhere, so we're guaranteed to come back to it (if we haven't
145147
// traversed it already).
146148
}
147149

148-
void maybe_record_irc(shape::ptr &box_dp) {
149-
if (!box_dp)
150-
return;
150+
void maybe_record_irc() {
151+
rust_opaque_box *box_ptr = *(rust_opaque_box **) dp;
151152

152-
rust_opaque_box *box_ptr = (rust_opaque_box *) box_dp;
153+
if (!box_ptr)
154+
return;
153155

154156
// Bump the internal reference count of the box.
155157
if (ircs.find(box_ptr) == ircs.end()) {
@@ -326,7 +328,11 @@ class mark : public shape::data<mark,shape::ptr> {
326328
}
327329

328330
void walk_box2() {
329-
shape::data<mark,shape::ptr>::walk_box_contents1();
331+
// the box ptr can be NULL for env ptrs in closures and data
332+
// not fully initialized
333+
rust_opaque_box *box = *(rust_opaque_box**)dp;
334+
if (box)
335+
shape::data<mark,shape::ptr>::walk_box_contents1();
330336
}
331337

332338
void walk_uniq2() {
@@ -336,8 +342,6 @@ class mark : public shape::data<mark,shape::ptr> {
336342
void walk_fn2(char code) {
337343
switch (code) {
338344
case shape::SHAPE_BOX_FN: {
339-
// Record an irc for the environment box, but don't descend
340-
// into it since it will be walked via the box's allocation
341345
shape::data<mark,shape::ptr>::walk_fn_contents1();
342346
break;
343347
}
@@ -369,11 +373,11 @@ class mark : public shape::data<mark,shape::ptr> {
369373

370374
void walk_uniq_contents2(mark &sub) { sub.walk(); }
371375

372-
void walk_box_contents2(mark &sub, shape::ptr &box_dp) {
373-
if (!box_dp)
374-
return;
376+
void walk_box_contents2(mark &sub) {
377+
rust_opaque_box *box_ptr = *(rust_opaque_box **) dp;
375378

376-
rust_opaque_box *box_ptr = (rust_opaque_box *) box_dp;
379+
if (!box_ptr)
380+
return;
377381

378382
if (marked.find(box_ptr) != marked.end())
379383
return; // Skip to avoid chasing cycles.
@@ -516,22 +520,26 @@ class sweep : public shape::data<sweep,shape::ptr> {
516520
}
517521

518522
void walk_box2() {
519-
shape::data<sweep,shape::ptr>::walk_box_contents1();
523+
// In sweep phase, do not walk the box contents. There is an
524+
// outer loop walking all remaining boxes, and this box may well
525+
// have been freed already!
520526
}
521527

522528
void walk_fn2(char code) {
523529
switch (code) {
524530
case shape::SHAPE_UNIQ_FN: {
525531
fn_env_pair pair = *(fn_env_pair*)dp;
526532

527-
// free closed over data:
528-
shape::data<sweep,shape::ptr>::walk_fn_contents1();
529-
530-
// now free the embedded type descr:
531-
upcall_s_free_shared_type_desc((type_desc*)pair.env->td);
532-
533-
// now free the ptr:
534-
task->kernel->free(pair.env);
533+
if (pair.env) {
534+
// free closed over data:
535+
shape::data<sweep,shape::ptr>::walk_fn_contents1();
536+
537+
// now free the embedded type descr:
538+
upcall_s_free_shared_type_desc((type_desc*)pair.env->td);
539+
540+
// now free the ptr:
541+
task->kernel->free(pair.env);
542+
}
535543
break;
536544
}
537545
case shape::SHAPE_BOX_FN: {
@@ -579,10 +587,6 @@ class sweep : public shape::data<sweep,shape::ptr> {
579587

580588
void walk_uniq_contents2(sweep &sub) { sub.walk(); }
581589

582-
void walk_box_contents2(sweep &sub, shape::ptr &box_dp) {
583-
return;
584-
}
585-
586590
void walk_struct2(const uint8_t *end_sp) {
587591
while (this->sp != end_sp) {
588592
this->walk();

src/rt/rust_shape.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ class cmp : public data<cmp,ptr_pair> {
264264
result = sub.result;
265265
}
266266

267-
inline void walk_box_contents2(cmp &sub, ptr_pair &box_dp) {
267+
inline void walk_box_contents2(cmp &sub) {
268268
sub.align = true;
269269
sub.walk();
270270
result = sub.result;
@@ -309,6 +309,15 @@ class cmp : public data<cmp,ptr_pair> {
309309
ptr_pair::make(in_data_0, in_data_1)),
310310
result(0) {}
311311

312+
cmp(const cmp &other,
313+
const uint8_t *in_sp,
314+
const type_param *in_params,
315+
const rust_shape_tables *in_tables,
316+
ptr_pair &in_dp)
317+
: data<cmp,ptr_pair>(other.task, other.align, in_sp, in_params, in_tables,
318+
in_dp),
319+
result(0) {}
320+
312321
cmp(const cmp &other,
313322
const uint8_t *in_sp = NULL,
314323
const type_param *in_params = NULL,

src/rt/rust_shape.h

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,17 @@ class ptr {
712712
inline operator bool() const { return p != NULL; }
713713
inline operator uintptr_t() const { return (uintptr_t)p; }
714714

715+
inline const type_desc *box_body_td() const {
716+
rust_opaque_box *box = *reinterpret_cast<rust_opaque_box**>(p);
717+
assert(box->ref_count >= 1);
718+
return box->td;
719+
}
720+
721+
inline ptr box_body() const {
722+
rust_opaque_box *box = *reinterpret_cast<rust_opaque_box**>(p);
723+
return make((uint8_t*)::box_body(box));
724+
}
725+
715726
static inline ptr make(uint8_t *in_p) {
716727
ptr self(in_p);
717728
return self;
@@ -746,7 +757,7 @@ class data_pair {
746757
inline void operator=(const T rhs) { fst = snd = rhs; }
747758

748759
static data_pair<T> make(T &fst, T &snd) {
749-
data_pair<T> data(fst, snd);
760+
data_pair<T> data(fst, snd);
750761
return data;
751762
}
752763
};
@@ -792,6 +803,25 @@ class ptr_pair {
792803
ptr_pair self(pair.fst, pair.snd);
793804
return self;
794805
}
806+
807+
inline const type_desc *box_body_td() const {
808+
// Here we assume that the two ptrs are both boxes with
809+
// equivalent type descriptors. This is safe because we only
810+
// use ptr_pair in the cmp glue, and we only use the cmp glue
811+
// when rust guarantees us that the boxes are of the same
812+
// type. As box types are not opaque to Rust, it is in a
813+
// position to make this determination.
814+
rust_opaque_box *box_fst = *reinterpret_cast<rust_opaque_box**>(fst);
815+
assert(box_fst->ref_count >= 1);
816+
return box_fst->td;
817+
}
818+
819+
inline ptr_pair box_body() const {
820+
rust_opaque_box *box_fst = *reinterpret_cast<rust_opaque_box**>(fst);
821+
rust_opaque_box *box_snd = *reinterpret_cast<rust_opaque_box**>(snd);
822+
return make((uint8_t*)::box_body(box_fst),
823+
(uint8_t*)::box_body(box_snd));
824+
}
795825
};
796826

797827
// NB: This function does not align.
@@ -933,18 +963,16 @@ class data : public ctxt< data<T,U> > {
933963
template<typename T,typename U>
934964
void
935965
data<T,U>::walk_box_contents1() {
936-
typename U::template data<uint8_t *>::t box_ptr = bump_dp<uint8_t *>(dp);
937-
U box_dp(box_ptr);
938-
939-
// No need to worry about alignment so long as the box header is
940-
// a multiple of 16 bytes. We can just find the body by adding
941-
// the size of header to box_dp.
942-
assert ((sizeof(rust_opaque_box) % 16) == 0 ||
943-
!"Must align to find the box body");
944-
945-
U body_dp = box_dp + sizeof(rust_opaque_box);
946-
T sub(*static_cast<T *>(this), body_dp);
947-
static_cast<T *>(this)->walk_box_contents2(sub, box_dp);
966+
const type_desc *body_td = dp.box_body_td();
967+
if (body_td) {
968+
U body_dp(dp.box_body());
969+
arena arena;
970+
type_param *params = type_param::from_tydesc(body_td, arena);
971+
T sub(*static_cast<T *>(this), body_td->shape, params,
972+
body_td->shape_tables, body_dp);
973+
sub.align = true;
974+
static_cast<T *>(this)->walk_box_contents2(sub);
975+
}
948976
}
949977

950978
template<typename T,typename U>
@@ -1006,7 +1034,7 @@ data<T,U>::walk_tag1(tag_info &tinfo) {
10061034

10071035
template<typename T,typename U>
10081036
void
1009-
data<T,U>::walk_fn_contents1() {
1037+
data<T,U>::walk_fn_contents1() {
10101038
fn_env_pair pair = bump_dp<fn_env_pair>(dp);
10111039
if (!pair.env)
10121040
return;
@@ -1119,9 +1147,10 @@ class log : public data<log,ptr> {
11191147

11201148
void walk_subcontext2(log &sub) { sub.walk(); }
11211149

1122-
void walk_box_contents2(log &sub, ptr &ref_count_dp) {
1150+
void walk_box_contents2(log &sub) {
11231151
out << prefix;
1124-
if (!ref_count_dp) {
1152+
rust_opaque_box *box_ptr = *(rust_opaque_box **) dp;
1153+
if (!box_ptr) {
11251154
out << "(null)";
11261155
} else {
11271156
sub.align = true;

0 commit comments

Comments
 (0)