ir3/ra: add helper for getting a dst interval

There have been multiple issues related to accessing intervals through
invalid register names. This usually results in a (difficult to
diagnose) out-of-bounds access. Wrap all the interval accesses in a
helper where we can assert that the name is in-bounds.

Signed-off-by: Job Noorman <jnoorman@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34503>
This commit is contained in:
Job Noorman 2025-04-14 16:00:13 +02:00 committed by Marge Bot
parent 384bf8e58e
commit d8033ba173
2 changed files with 62 additions and 48 deletions

View file

@ -386,6 +386,13 @@ rb_node_to_interval_const(const struct rb_node *node)
return rb_node_data(struct ra_interval, node, physreg_node);
}
static struct ra_interval *
ra_interval_get(struct ra_ctx *ctx, struct ir3_register *dst)
{
assert(dst->name != 0 && dst->name < ctx->live->definitions_count);
return &ctx->intervals[dst->name];
}
static struct ra_interval *
ra_interval_next(struct ra_interval *interval)
{
@ -755,7 +762,7 @@ check_dst_overlap(struct ra_ctx *ctx, struct ra_file *file,
if (ra_get_file(ctx, other_dst) != file)
continue;
struct ra_interval *other_interval = &ctx->intervals[other_dst->name];
struct ra_interval *other_interval = ra_interval_get(ctx, other_dst);
assert(!other_interval->interval.parent);
physreg_t other_start = other_interval->physreg_start;
physreg_t other_end = other_interval->physreg_end;
@ -1075,7 +1082,7 @@ compress_regs_left(struct ra_ctx *ctx, struct ra_file *file,
if (dst_inserted[n])
continue;
struct ra_interval *other_interval = &ctx->intervals[other_dst->name];
struct ra_interval *other_interval = ra_interval_get(ctx, other_dst);
/* if the destination partially overlaps this interval, we need to
* extend candidate_start to the end.
*/
@ -1093,7 +1100,7 @@ compress_regs_left(struct ra_ctx *ctx, struct ra_file *file,
*/
if (other_dst->tied) {
struct ra_interval *tied_interval =
&ctx->intervals[other_dst->tied->def->name];
ra_interval_get(ctx, other_dst->tied->def);
if (tied_interval->is_killed)
continue;
}
@ -1247,7 +1254,7 @@ compress_regs_left(struct ra_ctx *ctx, struct ra_file *file,
if (cur_reg == reg) {
ret_reg = physreg;
} else {
struct ra_interval *interval = &ctx->intervals[cur_reg->name];
struct ra_interval *interval = ra_interval_get(ctx, cur_reg);
interval->physreg_start = physreg;
interval->physreg_end = physreg + interval_size;
}
@ -1277,11 +1284,11 @@ compress_regs_left(struct ra_ctx *ctx, struct ra_file *file,
if (!tied)
continue;
struct ra_interval *tied_interval = &ctx->intervals[tied->def->name];
struct ra_interval *tied_interval = ra_interval_get(ctx, tied->def);
if (!tied_interval->is_killed)
continue;
struct ra_interval *dst_interval = &ctx->intervals[dst->name];
struct ra_interval *dst_interval = ra_interval_get(ctx, dst);
unsigned dst_size = reg_size(dst);
dst_interval->physreg_start = ra_interval_get_physreg(tied_interval);
dst_interval->physreg_end = dst_interval->physreg_start + dst_size;
@ -1363,7 +1370,7 @@ try_allocate_src(struct ra_ctx *ctx, struct ra_file *file,
if (!ra_reg_is_src(src))
continue;
if (ra_get_file(ctx, src) == file && reg_size(src) >= size) {
struct ra_interval *src_interval = &ctx->intervals[src->def->name];
struct ra_interval *src_interval = ra_interval_get(ctx, src->def);
physreg_t src_physreg = ra_interval_get_physreg(src_interval);
if (src_physreg % reg_elem_size(reg) == 0 &&
src_physreg + size <= file_size &&
@ -1512,7 +1519,7 @@ assign_reg(struct ir3_instruction *instr, struct ir3_register *reg,
static void
mark_src_killed(struct ra_ctx *ctx, struct ir3_register *src)
{
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
if (!(src->flags & IR3_REG_FIRST_KILL) || interval->is_killed ||
interval->interval.parent ||
@ -1526,7 +1533,7 @@ static void
insert_dst(struct ra_ctx *ctx, struct ir3_register *dst)
{
struct ra_file *file = ra_get_file(ctx, dst);
struct ra_interval *interval = &ctx->intervals[dst->name];
struct ra_interval *interval = ra_interval_get(ctx, dst);
d("insert dst %u physreg %u", dst->name, ra_interval_get_physreg(interval));
@ -1541,7 +1548,7 @@ allocate_dst_fixed(struct ra_ctx *ctx, struct ir3_register *dst,
physreg_t physreg)
{
struct ra_file *file = ra_get_file(ctx, dst);
struct ra_interval *interval = &ctx->intervals[dst->name];
struct ra_interval *interval = ra_interval_get(ctx, dst);
ra_update_affinity(file->size, dst, physreg);
ra_interval_init(interval, dst);
@ -1566,8 +1573,8 @@ insert_tied_dst_copy(struct ra_ctx *ctx, struct ir3_register *dst)
if (!tied)
return;
struct ra_interval *tied_interval = &ctx->intervals[tied->def->name];
struct ra_interval *dst_interval = &ctx->intervals[dst->name];
struct ra_interval *tied_interval = ra_interval_get(ctx, tied->def);
struct ra_interval *dst_interval = ra_interval_get(ctx, dst);
if (tied_interval->is_killed)
return;
@ -1588,7 +1595,7 @@ allocate_dst(struct ra_ctx *ctx, struct ir3_register *dst)
struct ir3_register *tied = dst->tied;
if (tied) {
struct ra_interval *tied_interval = &ctx->intervals[tied->def->name];
struct ra_interval *tied_interval = ra_interval_get(ctx, tied->def);
if (tied_interval->is_killed) {
/* The easy case: the source is killed, so we can just reuse it
* for the destination.
@ -1608,13 +1615,13 @@ static void
assign_src(struct ra_ctx *ctx, struct ir3_instruction *instr,
struct ir3_register *src)
{
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
struct ra_file *file = ra_get_file(ctx, src);
struct ir3_register *tied = src->tied;
physreg_t physreg;
if (tied) {
struct ra_interval *tied_interval = &ctx->intervals[tied->name];
struct ra_interval *tied_interval = ra_interval_get(ctx, tied);
physreg = ra_interval_get_physreg(tied_interval);
} else {
physreg = ra_interval_get_physreg(interval);
@ -1712,7 +1719,7 @@ handle_split(struct ra_ctx *ctx, struct ir3_instruction *instr)
return;
}
struct ra_interval *src_interval = &ctx->intervals[src->def->name];
struct ra_interval *src_interval = ra_interval_get(ctx, src->def);
physreg_t physreg = ra_interval_get_physreg(src_interval);
assign_src(ctx, instr, src);
@ -1758,7 +1765,7 @@ handle_collect(struct ra_ctx *ctx, struct ir3_instruction *instr)
mark_src_killed(ctx, src);
}
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
/* We only need special handling if the source's interval overlaps with
* the destination's interval.
@ -1792,13 +1799,13 @@ handle_collect(struct ra_ctx *ctx, struct ir3_instruction *instr)
/* Remove the temporary is_killed we added */
ra_foreach_src (src, instr) {
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
while (interval->interval.parent != NULL) {
interval = ir3_reg_interval_to_ra_interval(interval->interval.parent);
}
/* Filter out cases where it actually should be killed */
if (interval != &ctx->intervals[src->def->name] ||
if (interval != ra_interval_get(ctx, src->def) ||
!(src->flags & IR3_REG_KILL)) {
ra_file_unmark_killed(ra_get_file(ctx, src), interval);
}
@ -1854,7 +1861,7 @@ handle_precolored_input(struct ra_ctx *ctx, struct ir3_instruction *instr)
return;
struct ra_file *file = ra_get_file(ctx, instr->dsts[0]);
struct ra_interval *interval = &ctx->intervals[instr->dsts[0]->name];
struct ra_interval *interval = ra_interval_get(ctx, instr->dsts[0]);
physreg_t physreg = ra_reg_get_physreg(instr->dsts[0]);
allocate_dst_fixed(ctx, instr->dsts[0], physreg);
@ -1874,7 +1881,7 @@ handle_input(struct ra_ctx *ctx, struct ir3_instruction *instr)
allocate_dst(ctx, instr->dsts[0]);
struct ra_file *file = ra_get_file(ctx, instr->dsts[0]);
struct ra_interval *interval = &ctx->intervals[instr->dsts[0]->name];
struct ra_interval *interval = ra_interval_get(ctx, instr->dsts[0]);
ra_file_insert(file, interval);
}
@ -1884,7 +1891,7 @@ assign_input(struct ra_ctx *ctx, struct ir3_instruction *instr)
if (!(instr->dsts[0]->flags & IR3_REG_SSA))
return;
struct ra_interval *interval = &ctx->intervals[instr->dsts[0]->name];
struct ra_interval *interval = ra_interval_get(ctx, instr->dsts[0]);
struct ra_file *file = ra_get_file(ctx, instr->dsts[0]);
if (instr->dsts[0]->num == INVALID_REG) {
@ -1920,7 +1927,7 @@ static void
handle_precolored_source(struct ra_ctx *ctx, struct ir3_register *src)
{
struct ra_file *file = ra_get_file(ctx, src);
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
physreg_t physreg = ra_reg_get_physreg(src);
if (ra_interval_get_num(interval) == src->num)
@ -1957,7 +1964,7 @@ handle_chmask(struct ra_ctx *ctx, struct ir3_instruction *instr)
ra_foreach_src (src, instr) {
struct ra_file *file = ra_get_file(ctx, src);
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
if (src->flags & IR3_REG_FIRST_KILL)
ra_file_remove(file, interval);
}
@ -1997,7 +2004,7 @@ handle_live_in(struct ra_ctx *ctx, struct ir3_register *def)
assert(physreg != (physreg_t)~0);
struct ra_interval *interval = &ctx->intervals[def->name];
struct ra_interval *interval = ra_interval_get(ctx, def);
struct ra_file *file = ra_get_file(ctx, def);
ra_interval_init(interval, def);
interval->physreg_start = physreg;
@ -2016,7 +2023,7 @@ handle_live_out(struct ra_ctx *ctx, struct ir3_register *def)
return;
struct ra_block_state *state = &ctx->blocks[ctx->block->index];
struct ra_interval *interval = &ctx->intervals[def->name];
struct ra_interval *interval = ra_interval_get(ctx, def);
physreg_t physreg = ra_interval_get_physreg(interval);
if (physreg != ra_reg_get_physreg(def)) {
if (!state->renames)
@ -2032,7 +2039,7 @@ handle_phi(struct ra_ctx *ctx, struct ir3_register *def)
return;
struct ra_file *file = ra_get_file(ctx, def);
struct ra_interval *interval = &ctx->intervals[def->name];
struct ra_interval *interval = ra_interval_get(ctx, def);
/* phis are always scalar, so they should already be the smallest possible
* size. However they may be coalesced with other live-in values/phi
@ -2061,7 +2068,7 @@ assign_phi(struct ra_ctx *ctx, struct ir3_instruction *phi)
return;
struct ra_file *file = ra_get_file(ctx, phi->dsts[0]);
struct ra_interval *interval = &ctx->intervals[phi->dsts[0]->name];
struct ra_interval *interval = ra_interval_get(ctx, phi->dsts[0]);
assert(!interval->interval.parent);
unsigned num = ra_interval_get_num(interval);
assign_reg(phi, phi->dsts[0], num);
@ -2322,7 +2329,7 @@ handle_block(struct ra_ctx *ctx, struct ir3_block *block)
if (instr->opc == OPC_META_PHI) {
struct ir3_register *dst = instr->dsts[0];
if (!ctx->intervals[dst->name].interval.inserted) {
if (!ra_interval_get(ctx, dst)->interval.inserted) {
handle_phi(ctx, dst);
}
} else {

View file

@ -140,6 +140,13 @@ ra_interval_search_sloppy(struct rb_tree *tree, physreg_t reg)
return node ? rb_node_to_interval(node) : NULL;
}
static struct ra_interval *
ra_interval_get(struct ra_ctx *ctx, struct ir3_register *dst)
{
assert(dst->name != 0 && dst->name < ctx->live->definitions_count);
return &ctx->intervals[dst->name];
}
/* Get the interval covering the reg, or the closest to the right if it
* doesn't exist.
*/
@ -556,9 +563,9 @@ try_demote_instruction(struct ra_ctx *ctx, struct ir3_instruction *instr)
/* We need one source to either be demotable or an immediate. */
if (instr->srcs_count > 1) {
struct ra_interval *src0_interval =
(instr->srcs[0]->flags & IR3_REG_SSA) ? &ctx->intervals[instr->srcs[0]->def->name] : NULL;
(instr->srcs[0]->flags & IR3_REG_SSA) ? ra_interval_get(ctx, instr->srcs[0]->def) : NULL;
struct ra_interval *src1_interval =
(instr->srcs[0]->flags & IR3_REG_SSA) ? &ctx->intervals[instr->srcs[0]->def->name] : NULL;
(instr->srcs[0]->flags & IR3_REG_SSA) ? ra_interval_get(ctx, instr->srcs[0]->def) : NULL;
if (!(src0_interval && src0_interval->spill_def) &&
!(src1_interval && src1_interval->spill_def) &&
!(instr->srcs[0]->flags & IR3_REG_IMMED) &&
@ -569,9 +576,9 @@ try_demote_instruction(struct ra_ctx *ctx, struct ir3_instruction *instr)
}
case 3: {
struct ra_interval *src0_interval =
(instr->srcs[0]->flags & IR3_REG_SSA) ? &ctx->intervals[instr->srcs[0]->def->name] : NULL;
(instr->srcs[0]->flags & IR3_REG_SSA) ? ra_interval_get(ctx, instr->srcs[0]->def) : NULL;
struct ra_interval *src1_interval =
(instr->srcs[1]->flags & IR3_REG_SSA) ? &ctx->intervals[instr->srcs[1]->def->name] : NULL;
(instr->srcs[1]->flags & IR3_REG_SSA) ? ra_interval_get(ctx, instr->srcs[1]->def) : NULL;
/* src1 cannot be shared */
if (src1_interval && !src1_interval->spill_def) {
@ -592,7 +599,7 @@ try_demote_instruction(struct ra_ctx *ctx, struct ir3_instruction *instr)
}
case 4: {
assert(instr->srcs[0]->flags & IR3_REG_SSA);
struct ra_interval *src_interval = &ctx->intervals[instr->srcs[0]->def->name];
struct ra_interval *src_interval = ra_interval_get(ctx, instr->srcs[0]->def);
if (!src_interval->spill_def)
return false;
break;
@ -613,7 +620,7 @@ try_demote_instruction(struct ra_ctx *ctx, struct ir3_instruction *instr)
/* Now we actually demote the instruction */
ra_foreach_src (src, instr) {
assert(src->flags & IR3_REG_SHARED);
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
if (interval->spill_def) {
src->def = interval->spill_def;
src->flags &= ~IR3_REG_SHARED;
@ -626,7 +633,7 @@ try_demote_instruction(struct ra_ctx *ctx, struct ir3_instruction *instr)
}
}
struct ra_interval *dst_interval = &ctx->intervals[instr->dsts[0]->name];
struct ra_interval *dst_interval = ra_interval_get(ctx, instr->dsts[0]);
instr->dsts[0]->flags &= ~IR3_REG_SHARED;
ra_interval_init(dst_interval, instr->dsts[0]);
dst_interval->spill_def = instr->dsts[0];
@ -687,7 +694,7 @@ get_reg(struct ra_ctx *ctx, struct ir3_register *reg, bool src)
if (!ra_reg_is_src(src))
continue;
if ((src->flags & IR3_REG_SHARED) && reg_size(src) >= size) {
struct ra_interval *src_interval = &ctx->intervals[src->def->name];
struct ra_interval *src_interval = ra_interval_get(ctx, src->def);
physreg_t src_physreg = ra_interval_get_physreg(src_interval);
if (src_physreg % reg_elem_size(reg) == 0 &&
src_physreg + size <= reg_file_size(reg) &&
@ -714,7 +721,7 @@ reload_src(struct ra_ctx *ctx, struct ir3_instruction *instr,
struct ir3_register *src)
{
struct ir3_register *reg = src->def;
struct ra_interval *interval = &ctx->intervals[reg->name];
struct ra_interval *interval = ra_interval_get(ctx, reg);
unsigned size = reg_size(reg);
physreg_t best_reg = get_reg(ctx, reg, true);
@ -767,7 +774,7 @@ reload_src_finalize(struct ra_ctx *ctx, struct ir3_instruction *instr,
struct ir3_register *src)
{
struct ir3_register *reg = src->def;
struct ra_interval *interval = &ctx->intervals[reg->name];
struct ra_interval *interval = ra_interval_get(ctx, reg);
if (!interval->needs_reload)
return;
@ -807,7 +814,7 @@ mark_src(struct ra_ctx *ctx, struct ir3_register *src)
if (!(src->flags & IR3_REG_SHARED))
return;
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
if (interval->interval.inserted) {
while (interval->interval.parent)
@ -824,7 +831,7 @@ ensure_src_live(struct ra_ctx *ctx, struct ir3_instruction *instr,
if (!(src->flags & IR3_REG_SHARED))
return;
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
if (!interval->interval.inserted) {
/* In some cases we cannot demote shared reg sources to non-shared regs,
@ -852,7 +859,7 @@ assign_src(struct ra_ctx *ctx, struct ir3_register *src)
if (!(src->flags & IR3_REG_SHARED))
return;
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
assert(interval->interval.inserted);
src->num = ra_physreg_to_num(ra_interval_get_physreg(interval), src->flags);
@ -874,13 +881,13 @@ handle_dst(struct ra_ctx *ctx, struct ir3_instruction *instr,
if (!(dst->flags & IR3_REG_SHARED))
return;
struct ra_interval *interval = &ctx->intervals[dst->name];
struct ra_interval *interval = ra_interval_get(ctx, dst);
ra_interval_init(interval, dst);
interval->spill_def = NULL;
if (dst->tied) {
struct ir3_register *tied_def = dst->tied->def;
struct ra_interval *tied_interval = &ctx->intervals[tied_def->name];
struct ra_interval *tied_interval = ra_interval_get(ctx, tied_def);
if ((dst->tied->flags & IR3_REG_KILL) &&
!tied_interval->interval.parent &&
rb_tree_is_empty(&tied_interval->interval.children)) {
@ -949,7 +956,7 @@ handle_src_late(struct ra_ctx *ctx, struct ir3_instruction *instr,
if (!(src->flags & IR3_REG_SHARED))
return;
struct ra_interval *interval = &ctx->intervals[src->def->name];
struct ra_interval *interval = ra_interval_get(ctx, src->def);
reload_src_finalize(ctx, instr, src);
/* Remove killed sources that have to be killed late due to being merged with
@ -995,8 +1002,8 @@ handle_split(struct ra_ctx *ctx, struct ir3_instruction *split)
return;
}
struct ra_interval *src_interval = &ctx->intervals[src->def->name];
struct ra_interval *dst_interval = &ctx->intervals[dst->name];
struct ra_interval *src_interval = ra_interval_get(ctx, src->def);
struct ra_interval *dst_interval = ra_interval_get(ctx, dst);
ra_interval_init(dst_interval, dst);
dst_interval->spill_def = NULL;
@ -1036,7 +1043,7 @@ handle_phi(struct ra_ctx *ctx, struct ir3_instruction *phi)
if (!(dst->flags & IR3_REG_SHARED))
return;
struct ra_interval *dst_interval = &ctx->intervals[dst->name];
struct ra_interval *dst_interval = ra_interval_get(ctx, dst);
ra_interval_init(dst_interval, dst);
/* In some rare cases, it's possible to have a phi node with a physical-only