Fix Coverity INVALIDATE_ITERATOR defects. #2584
Conversation
source/common/ur_singleton.hpp
Outdated
| if (auto iter = map.find(getKey(key)); iter != map.end()) { | ||
| iter->second.ref_count++; | ||
| } else { | ||
| __builtin_unreachable(); |
There was a problem hiding this comment.
there's a function:
unified-runtime/source/common/ur_util.hpp
Line 339 in 8b7a995
source/common/ur_singleton.hpp
Outdated
| if (auto iter = map.find(getKey(key)); iter != map.end()) { | ||
| iter->second.ref_count++; | ||
| } else { | ||
| __builtin_unreachable(); |
There was a problem hiding this comment.
I'm not sure removing the assert (what this is functionally equivalent to) is the correct resolution here - it's possible for an error in UR code to retain an error value, and I think flagging that issue in a debug build is useful. I don't think this also resolves the Coverity error either.
If we do want to improve the error checking and have it work in release mode, I think calling abort() with an error message is the best move.
Edit: To explain why it is equivalent, consider that __builtin_unreachable has undefined behaviour. Since undefined behaviour must Never Happen, the compiler may reason that the true branch of the conditional is always taken, and replace it with just auto iter = map.find(getKey(key)); iter->second.ref_count++.
isaacault
force-pushed
the
INVALIDATE_ITERATOR
branch
from
8e5f8d0 to
9c4696f
Compare
No description provided.