From 737fc5d3c184fb9693e14e1642d678573ed7c94b Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Fri, 20 Jun 2025 14:28:06 +0200 Subject: [PATCH] pan/genxml: Make sure we catch overflows on 32-bit integer fields If we store the temporary value in a [u]int32_t, the result might be silently truncated, making the overflow check in the u_pack helpers useless. Make sure we use 64-bit fields as soon as the size is greater than 31 bits to prevent that. It forces us to fix some types in the xml and helper arguments (sint wrongly defined as uint) and add explicit casts on u32 subtraction whose result is stored in an s32. Signed-off-by: Boris Brezillon Reviewed-by: Lars-Ivar Hesselberg Simonsen Part-of: --- src/gallium/drivers/panfrost/pan_jm.c | 2 +- src/panfrost/genxml/cs_builder.h | 4 ++-- src/panfrost/genxml/decode_csf.c | 26 +++++++++++----------- src/panfrost/genxml/gen_pack.py | 8 +++---- src/panfrost/genxml/v4.xml | 4 ++-- src/panfrost/genxml/v5.xml | 4 ++-- src/panfrost/genxml/v6.xml | 4 ++-- src/panfrost/genxml/v7.xml | 4 ++-- src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c | 2 +- 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_jm.c b/src/gallium/drivers/panfrost/pan_jm.c index 878b7428f34..af01674d454 100644 --- a/src/gallium/drivers/panfrost/pan_jm.c +++ b/src/gallium/drivers/panfrost/pan_jm.c @@ -737,7 +737,7 @@ jm_emit_primitive(struct panfrost_batch *batch, * on Valhall, so we don't need to set that here */ } else if (cfg.index_type) { - cfg.base_vertex_offset = draw->index_bias - ctx->offset_start; + cfg.base_vertex_offset = (int64_t)draw->index_bias - ctx->offset_start; #if PAN_ARCH <= 7 cfg.indices = batch->indices; diff --git a/src/panfrost/genxml/cs_builder.h b/src/panfrost/genxml/cs_builder.h index 89b63c14db9..e177e72c912 100644 --- a/src/panfrost/genxml/cs_builder.h +++ b/src/panfrost/genxml/cs_builder.h @@ -1621,7 +1621,7 @@ cs_finish_fragment(struct cs_builder *b, bool increment_frag_completed, static inline void cs_add32(struct cs_builder *b, struct cs_index dest, struct cs_index src, - unsigned imm) + int32_t imm) { cs_emit(b, ADD_IMM32, I) { I.destination = cs_dst32(b, dest); @@ -1632,7 +1632,7 @@ cs_add32(struct cs_builder *b, struct cs_index dest, struct cs_index src, static inline void cs_add64(struct cs_builder *b, struct cs_index dest, struct cs_index src, - unsigned imm) + int32_t imm) { cs_emit(b, ADD_IMM64, I) { I.destination = cs_dst64(b, dest); diff --git a/src/panfrost/genxml/decode_csf.c b/src/panfrost/genxml/decode_csf.c index 35fc223eaaa..d5cee354fcf 100644 --- a/src/panfrost/genxml/decode_csf.c +++ b/src/panfrost/genxml/decode_csf.c @@ -116,7 +116,7 @@ print_cs_instr(FILE *fp, const uint64_t *instr) case MALI_CS_OPCODE_MOVE32: { cs_unpack(instr, CS_MOVE32, I); - fprintf(fp, "MOVE32 r%u, #0x%X", I.destination, I.immediate); + fprintf(fp, "MOVE32 r%u, #0x%" PRIX64, I.destination, I.immediate); break; } @@ -156,7 +156,7 @@ print_cs_instr(FILE *fp, const uint64_t *instr) cs_unpack(instr, CS_RUN_IDVS, I); fprintf( fp, - "RUN_IDVS%s%s%s.varying_srt%d.varying_fau%d.varying_tsd%d.frag_srt%d.frag_tsd%d r%u, #%x", + "RUN_IDVS%s%s%s.varying_srt%d.varying_fau%d.varying_tsd%d.frag_srt%d.frag_tsd%d r%u, #%" PRIx64, I.progress_increment ? ".progress_inc" : "", I.malloc_enable ? "" : ".no_malloc", I.draw_id_register_enable ? ".draw_id_enable" : "", @@ -176,7 +176,7 @@ print_cs_instr(FILE *fp, const uint64_t *instr) ".INVALID", }; - fprintf(fp, "RUN_IDVS2%s%s%s%s r%u, #%x", + fprintf(fp, "RUN_IDVS2%s%s%s%s r%u, #%" PRIx64, I.progress_increment ? ".progress_inc" : "", I.malloc_enable ? "" : ".no_malloc", I.draw_id_register_enable ? ".draw_id_enable" : "", @@ -195,8 +195,8 @@ print_cs_instr(FILE *fp, const uint64_t *instr) "BFEXT_U32", "BFEXT_S32", "BFINS_IMM32", "UMIN_IMM32", }; - fprintf(fp, "%s r%u, r%u, #%d", instr_name[I.sub_opcode], I.destination, - I.source, I.immediate); + fprintf(fp, "%s r%u, r%u, #%" PRId64, instr_name[I.sub_opcode], + I.destination, I.source, I.immediate); break; } @@ -208,8 +208,8 @@ print_cs_instr(FILE *fp, const uint64_t *instr) "BFEXT_U64", "BFEXT_S64", "BFINS_IMM64", "UMIN_IMM64", }; - fprintf(fp, "%s d%u, d%u, #%d", instr_name[I.sub_opcode], I.destination, - I.source, I.immediate); + fprintf(fp, "%s d%u, d%u, #%" PRId64, instr_name[I.sub_opcode], + I.destination, I.source, I.immediate); break; } @@ -309,7 +309,7 @@ print_cs_instr(FILE *fp, const uint64_t *instr) const char *state = I.state <= sizeof(state_name) ? state_name[I.state] : ".INVALID"; - fprintf(fp, "SET_STATE_IMM32%s #%u", state, I.value); + fprintf(fp, "SET_STATE_IMM32%s #%" PRIu64, state, I.value); break; } @@ -364,7 +364,7 @@ print_cs_instr(FILE *fp, const uint64_t *instr) case MALI_CS_OPCODE_RUN_FULLSCREEN: { cs_unpack(instr, CS_RUN_FULLSCREEN, I); - fprintf(fp, "RUN_FULLSCREEN%s r%u, #%x", + fprintf(fp, "RUN_FULLSCREEN%s r%u, #%" PRIx64, I.progress_increment ? ".progress_inc" : "", I.dcd, I.flags_override); break; @@ -390,16 +390,16 @@ print_cs_instr(FILE *fp, const uint64_t *instr) case MALI_CS_OPCODE_ADD_IMMEDIATE32: { cs_unpack(instr, CS_ADD_IMM32, I); - fprintf(fp, "ADD_IMMEDIATE32 r%u, r%u, #%d", I.destination, I.source, - I.immediate); + fprintf(fp, "ADD_IMMEDIATE32 r%u, r%u, #%" PRId64, I.destination, + I.source, I.immediate); break; } case MALI_CS_OPCODE_ADD_IMMEDIATE64: { cs_unpack(instr, CS_ADD_IMM64, I); - fprintf(fp, "ADD_IMMEDIATE64 d%u, d%u, #%d", I.destination, I.source, - I.immediate); + fprintf(fp, "ADD_IMMEDIATE64 d%u, d%u, #%" PRId64, I.destination, + I.source, I.immediate); break; } diff --git a/src/panfrost/genxml/gen_pack.py b/src/panfrost/genxml/gen_pack.py index 233dd6463bd..39406b6f25e 100644 --- a/src/panfrost/genxml/gen_pack.py +++ b/src/panfrost/genxml/gen_pack.py @@ -196,10 +196,10 @@ class Field(object): type = 'bool' elif self.type in ['float', 'ulod', 'slod']: type = 'float' - elif self.type in ['uint', 'hex'] and self.end - self.start > 32: + elif self.type in ['uint', 'hex'] and self.end - self.start >= 31: type = 'uint64_t' elif self.type == 'int': - type = 'int32_t' + type = 'int64_t' if self.end - self.start >= 31 else 'int32_t' elif self.type in ['uint', 'hex', 'uint/float', 'padded', 'Pixel Format', 'Component Swizzle']: type = 'uint32_t' elif self.type in self.parser.structs: @@ -488,12 +488,12 @@ class Group(object): elif field.type in self.parser.enums: print(' fprintf(fp, "%*s{}: %s\\n", indent, "", {}_as_str({}));'.format(name, enum_name(field.type), val)) elif field.type == "int": - print(' fprintf(fp, "%*s{}: %d\\n", indent, "", {});'.format(name, val)) + print(' fprintf(fp, "%*s{}: %" PRId64 "\\n", indent, "", (int64_t){});'.format(name, val)) elif field.type == "bool": print(' fprintf(fp, "%*s{}: %s\\n", indent, "", {} ? "true" : "false");'.format(name, val)) elif field.type in ["float", "ulod", "slod"]: print(' fprintf(fp, "%*s{}: %f\\n", indent, "", {});'.format(name, val)) - elif field.type in ["uint", "hex"] and (field.end - field.start) >= 32: + elif field.type in ["uint", "hex"] and (field.end - field.start) >= 31: print(' fprintf(fp, "%*s{}: 0x%" PRIx64 "\\n", indent, "", {});'.format(name, val)) elif field.type == "hex": print(' fprintf(fp, "%*s{}: 0x%x\\n", indent, "", {});'.format(name, val)) diff --git a/src/panfrost/genxml/v4.xml b/src/panfrost/genxml/v4.xml index 476830ee90f..9ce4c7b9c9f 100644 --- a/src/panfrost/genxml/v4.xml +++ b/src/panfrost/genxml/v4.xml @@ -524,8 +524,8 @@ - - + + diff --git a/src/panfrost/genxml/v5.xml b/src/panfrost/genxml/v5.xml index 9c790dd0059..8c93016810f 100644 --- a/src/panfrost/genxml/v5.xml +++ b/src/panfrost/genxml/v5.xml @@ -559,8 +559,8 @@ - - + + diff --git a/src/panfrost/genxml/v6.xml b/src/panfrost/genxml/v6.xml index 6d7eb31b081..559628cbfdc 100644 --- a/src/panfrost/genxml/v6.xml +++ b/src/panfrost/genxml/v6.xml @@ -600,8 +600,8 @@ - - + + diff --git a/src/panfrost/genxml/v7.xml b/src/panfrost/genxml/v7.xml index 1f13939e09e..d5a9ceb2daa 100644 --- a/src/panfrost/genxml/v7.xml +++ b/src/panfrost/genxml/v7.xml @@ -656,8 +656,8 @@ - - + + diff --git a/src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c b/src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c index 285a124dcca..c1df1c0549c 100644 --- a/src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c +++ b/src/panfrost/vulkan/jm/panvk_vX_cmd_draw.c @@ -856,7 +856,7 @@ panvk_emit_tiler_primitive(struct panvk_cmd_buffer *cmdbuf, cfg.index_count = draw->info.vertex.count; cfg.indices = draw->indices; cfg.base_vertex_offset = - draw->info.vertex.base - draw->info.vertex.raw_offset; + (int64_t)draw->info.vertex.base - draw->info.vertex.raw_offset; switch (draw->info.index.size) { case 4: