-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
While investigating #7606 I noticed this code in the Anderson2021 codebase:
std::unique_ptr<ThreadInfo> GPULoopInfo::create_thread_info() {
internal_assert(at_or_inside_block());
internal_assert(at_or_inside_thread());
auto max_thread_counts = current_block_loop->get_union_thread_counts(nullptr);
std::unique_ptr<ThreadInfo> new_thread_info = std::make_unique<ThreadInfo>(
current_thread_loop->vectorized_loop_index,
current_thread_loop->size,
current_thread_loop->stage->loop,
max_thread_counts);
thread_info = new_thread_info.get();
return new_thread_info;
}
since thread_info
is a member of this
, and declared as a plain const ThreadInfo*
, after this call, it refers to an unowned pointer... the ownership is in the returned unique_ptr. Since the only two call sites immediately assign the result to a local unique_ptr, this member var is guaranteed to be stale
at the end of the calling routine.
I'm not sure what the right fix is here because I don't know the intended semantics -- shared_ptr
might be appropriate, but it's not clear to me whether the returned result is intended to be mutable or not, or what constraints there are on mutability.
The current code looks absolutely unsafe, however, and needs fixing; if there are similar code patterns elsewhere in this codebase, they need examination as well.