Check for use of InstIds from the wrong SemIR::File (#5997)

Use the `CheckIRId` as a unique identifier for the scope of an `InstId`
- if an `InstId` is created within the scope of one `CheckIRId` it must
not be used in the scope of a different `CheckIRId`.

This is achieved without extra storage, but with false negatives for
large inputs.

When an `InstId` is created, the original index of the `Inst` is XORed
with a tag derived from the `CheckIRId` to produce the final `InstId`.
When the `InstId` is used, the expected tag is XORed with the `InstId`
to get back to the original index - if the tags don't match, the
resulting index will be corrupted, likely too large - resulting in an
out of bounds index CHECK-failure.

(the tag value is derived as such:
* take the CheckIRId
* left shift one bit (padding zero)
* left shift another bit (padding 1 - used to signify that the resulting
`InstId` has a tag combined into it)
* reverse the bits

In this way, the tag is unlikely to overlap with the index for small
test cases - making it possible to separate out the `CheckIRId` from the
index in these cases to provide more meaningful debugging/CHECK
messages, and more informative `SemIR` textual dumping that can now
include the `CheckIRId` along with the `Inst`'s index in the name of an
`inst`)

The test churn here is improved printing as tagged `InstId`s can now,
with best effort (more likely for small test cases where the `CheckIRId`
and the `Inst` index aren't at risk of overlapping from the high and low
bits), render the `CheckIRId` as part of the inst's name. Going from
`instNN` to `irMM.instNN`.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
This commit is contained in:
David Blaikie
2025-10-02 23:07:36 +00:00
committed by GitHub
co-authored by Richard Smith
parent ce6bf91a83
commit 12fa65e53c
31 changed files with 2193 additions and 2032 deletions
+4 -4
View File
@@ -559,8 +559,8 @@ class MultiUnitCache {
auto include_in_dumps() -> const IncludeInDumpsStore& {
if (!include_in_dumps_) {
include_in_dumps_.emplace(
IncludeInDumpsStore::MakeWithExplicitSize(units_.size(), false));
include_in_dumps_.emplace(IncludeInDumpsStore::MakeWithExplicitSize(
IdTag(), units_.size(), false));
for (const auto& [i, unit] : llvm::enumerate(units_)) {
// If this is first accessed after lexing is complete, we need to apply
// per-file includes. Otherwise, this is based only on the exclude
@@ -580,8 +580,8 @@ class MultiUnitCache {
auto tree_and_subtrees_getters() -> const TreeAndSubtreesGettersStore& {
if (!tree_and_subtrees_getters_) {
tree_and_subtrees_getters_.emplace(
TreeAndSubtreesGettersStore::MakeWithExplicitSize(units_.size(),
nullptr));
TreeAndSubtreesGettersStore::MakeWithExplicitSize(
IdTag(), units_.size(), nullptr));
for (const auto& [i, unit] : llvm::enumerate(units_)) {
if (unit->has_source()) {
tree_and_subtrees_getters_->Set(SemIR::CheckIRId(i),