search

apache / tvm

Open deep learning compiler stack for cpu, gpu and specialized accelerators
https://tvm.apache.org/
Apache License 2.0
11.42k stars 3.4k forks source link

[Bugfix][Support] Fix copy constructor for support::OrderedSet #17044

Closed Lunderberg closed 1 month ago

Lunderberg commented 1 month ago

Prior to this commit, the support::OrderedSet<T> utility used the default copy constructor and copy assignment, which would copy both the OrderedSet::elements_ and OrderedSet::elem_to_iter_ members. While this is the correct behavior for elements_, the copy of elem_to_iter_ would contain references to the original's element_, rather than to its own.

While elem_to_iter_ is used in both OrderedSet::push_back and OrderedSet::erase, the implementation of OrderedSet::push_back only depends on the keys used in elem_to_iter_, and does not depend on the values stored. As a result, this bug could go undetected for append-only usage, which is the most frequent use of OrderedSet.

This commit updates support::OrderedSet to have an explicit copy constructor and copy assignment. Only the std::list<T> elements_ member may be copied, while the elem_to_iter_ must instead be rebuilt.

Lunderberg commented 1 month ago

While I'm unaware of any way to trigger this bug from an externally-visible API, I ran across this during implementation of a debug feature. This bug can result in segfaults when accessing elements of a support::OrderedSet.