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
There was a problem hiding this comment.
No longer using unreachable in favour of abort() - see below.
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++.
There was a problem hiding this comment.
Done. Error message sent to the logger.
isaacault
force-pushed
the
INVALIDATE_ITERATOR
branch
2 times, most recently
from
9c4696f to
fc8431d
Compare
isaacault
force-pushed
the
INVALIDATE_ITERATOR
branch
from
fc8431d to
69f9eb2
Compare
|
This file will be removed by #2622 |
Unified Runtime -> intel/llvm Repo Move NoticeInformationThe source code of Unified Runtime has been moved to intel/llvm under the unified-runtime top-level directory, The code will be mirrored to oneapi-src/unified-runtime and the specification will continue to be hosted at oneapi-src.github.io/unified-runtime. The contribution guide has been updated with new instructions for contributing to Unified Runtime. PR MigrationAll open PRs including this one will be labelled auto-close and shall be automatically closed after 30 days. Should you wish to continue with your PR you will need to migrate it to intel/llvm. This is an automated comment. |
Unified Runtime -> intel/llvm Repo Move NoticeFollowing on from the previous notice, we have now enabled workflows to automatically label and close PRs because the Unified Runtime source code has moved to intel/llvm. This PR has now been marked with the Please review the previous notice for more information, including assistance with migrating your PR to intel/llvm. Should there be a reason for this PR to remain open, manually remove the This is an automated comment. |
No description provided.