@samitouri / QOSamiQemu / commits / b4bbd68882

target/hexagon: Change DisasContext packet type

The pkt variable inside DisasContext is of type Packet * and gets assigned to a local variable in decode_and_translate_packet. Right now there seems to be no problem with it but future changes to e.g. hexagon_tr_transalte_packet are potentially dangerous if pkt is accessed after the local variable goes out of scope. Since packets are being translated one at a time, the type of pkt can be changed to just Packet to avoid risk of having a dangling pointer. Signed-off-by: Marco Liebel <marco.liebel@oss.qualcomm.com> Reviewed-by: Brian Cain <brian.cain@oss.qualcomm.com> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>

Marco Liebel committed Jan 22, 2026 at 14:34 UTC b4bbd68882f24b0f99593b48fc50122e5dac5c41
8 files changed +66 -85
target/hexagon/decode.c
+3 -5
@@ -834,15 +834,13 @@ int disassemble_hexagon(uint32_t *words, int nwords, bfd_vma pc,
834 .hex_version = HEX_VER_ANY, /* Allow decode to accept anything */
835 };
836 DisasContext ctx;
837 - Packet pkt;
837
838 memset(&ctx, 0, sizeof(DisasContext));
839 ctx.hex_def = &any_def;
841 - ctx.pkt = &pkt;
840
843 - if (decode_packet(&ctx, nwords, words, &pkt, true) > 0) {
844 - snprint_a_pkt_disas(buf, &pkt, words, pc, hex_def);
845 - return pkt.encod_pkt_size_in_bytes;
841 + if (decode_packet(&ctx, nwords, words, &ctx.pkt, true) > 0) {
842 + snprint_a_pkt_disas(buf, &ctx.pkt, words, pc, hex_def);
843 + return ctx.pkt.encod_pkt_size_in_bytes;
844 } else {
845 for (int i = 0; i < nwords; i++) {
846 g_string_append_printf(buf, "0x" TARGET_FMT_lx "\t", words[i]);
target/hexagon/gen_tcg.h
+1 -1
@@ -1343,7 +1343,7 @@
1343 #define fGEN_TCG_J2_trap0(SHORTCODE) \
1344 do { \
1345 uiV = uiV; \
1346 - tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], ctx->pkt->pc); \
1346 + tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], ctx->pkt.pc); \
1347 TCGv excp = tcg_constant_tl(HEX_EVENT_TRAP0); \
1348 gen_helper_raise_exception(tcg_env, excp); \
1349 } while (0)
target/hexagon/gen_tcg_funcs.py
+1 -1
@@ -72,7 +72,7 @@ def gen_tcg_func(f, tag, regs, imms):
72 for immlett, bits, immshift in imms:
73 declared.append(hex_common.imm_name(immlett))
74
75 - arguments = ", ".join(["ctx", "ctx->insn", "ctx->pkt"] + declared)
75 + arguments = ", ".join(["ctx", "ctx->insn", "&ctx->pkt"] + declared)
76 f.write(f" emit_{tag}({arguments});\n")
77
78 elif hex_common.skip_qemu_helper(tag):
target/hexagon/genptr.c
+7 -7
@@ -382,7 +382,7 @@ static inline void gen_store_conditional8(DisasContext *ctx,
382 static TCGv gen_slotval(DisasContext *ctx)
383 {
384 int slotval =
385 - (ctx->pkt->pkt_has_scalar_store_s1 & 1) | (ctx->insn->slot << 1);
385 + (ctx->pkt.pkt_has_scalar_store_s1 & 1) | (ctx->insn->slot << 1);
386 return tcg_constant_tl(slotval);
387 }
388 #endif
@@ -458,7 +458,7 @@ static void gen_write_new_pc_addr(DisasContext *ctx, TCGv addr,
458 tcg_gen_brcondi_tl(cond, pred, 1, pred_false);
459 }
460
461 - if (ctx->pkt->pkt_has_multi_cof) {
461 + if (ctx->pkt.pkt_has_multi_cof) {
462 /* If there are multiple branches in a packet, ignore the second one */
463 tcg_gen_movcond_tl(TCG_COND_NE, hex_gpr[HEX_REG_PC],
464 ctx->branch_taken, tcg_constant_tl(0),
@@ -476,8 +476,8 @@ static void gen_write_new_pc_addr(DisasContext *ctx, TCGv addr,
476 static void gen_write_new_pc_pcrel(DisasContext *ctx, int pc_off,
477 TCGCond cond, TCGv pred)
478 {
479 - target_ulong dest = ctx->pkt->pc + pc_off;
480 - if (ctx->pkt->pkt_has_multi_cof) {
479 + target_ulong dest = ctx->pkt.pc + pc_off;
480 + if (ctx->pkt.pkt_has_multi_cof) {
481 gen_write_new_pc_addr(ctx, tcg_constant_tl(dest), cond, pred);
482 } else {
483 /* Defer this jump to the end of the TB */
@@ -528,7 +528,7 @@ static inline void gen_loop0r(DisasContext *ctx, TCGv RsV, int riV)
528 fIMMEXT(riV);
529 fPCALIGN(riV);
530 tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC0), RsV);
531 - tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt->pc + riV);
531 + tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt.pc + riV);
532 gen_set_usr_fieldi(ctx, USR_LPCFG, 0);
533 }
534
@@ -542,7 +542,7 @@ static inline void gen_loop1r(DisasContext *ctx, TCGv RsV, int riV)
542 fIMMEXT(riV);
543 fPCALIGN(riV);
544 tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC1), RsV);
545 - tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA1), ctx->pkt->pc + riV);
545 + tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA1), ctx->pkt.pc + riV);
546 }
547
548 static void gen_loop1i(DisasContext *ctx, int count, int riV)
@@ -555,7 +555,7 @@ static void gen_ploopNsr(DisasContext *ctx, int N, TCGv RsV, int riV)
555 fIMMEXT(riV);
556 fPCALIGN(riV);
557 tcg_gen_mov_tl(get_result_gpr(ctx, HEX_REG_LC0), RsV);
558 - tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt->pc + riV);
558 + tcg_gen_movi_tl(get_result_gpr(ctx, HEX_REG_SA0), ctx->pkt.pc + riV);
559 gen_set_usr_fieldi(ctx, USR_LPCFG, N);
560 gen_pred_write(ctx, 3, tcg_constant_tl(0));
561 }
target/hexagon/hex_common.py
+2 -2
@@ -1144,7 +1144,7 @@ def helper_args(tag, regs, imms):
1144 if need_pkt_has_multi_cof(tag):
1145 args.append(HelperArg(
1146 "i32",
1147 - "tcg_constant_tl(ctx->pkt->pkt_has_multi_cof)",
1147 + "tcg_constant_tl(ctx->pkt.pkt_has_multi_cof)",
1148 "uint32_t pkt_has_multi_cof"
1149 ))
1150 if need_pkt_need_commit(tag):
@@ -1156,7 +1156,7 @@ def helper_args(tag, regs, imms):
1156 if need_PC(tag):
1157 args.append(HelperArg(
1158 "i32",
1159 - "tcg_constant_tl(ctx->pkt->pc)",
1159 + "tcg_constant_tl(ctx->pkt.pc)",
1160 "target_ulong PC"
1161 ))
1162 if need_next_PC(tag):
target/hexagon/macros.h
+3 -3
@@ -83,7 +83,7 @@
83 */
84 #define CHECK_NOSHUF(VA, SIZE) \
85 do { \
86 - if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
86 + if (insn->slot == 0 && ctx->pkt.pkt_has_scalar_store_s1) { \
87 probe_noshuf_load(VA, SIZE, ctx->mem_idx); \
88 process_store(ctx, 1); \
89 } \
@@ -94,11 +94,11 @@
94 TCGLabel *noshuf_label = gen_new_label(); \
95 tcg_gen_brcondi_tl(TCG_COND_EQ, PRED, 0, noshuf_label); \
96 GET_EA; \
97 - if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
97 + if (insn->slot == 0 && ctx->pkt.pkt_has_scalar_store_s1) { \
98 probe_noshuf_load(EA, SIZE, ctx->mem_idx); \
99 } \
100 gen_set_label(noshuf_label); \
101 - if (insn->slot == 0 && ctx->pkt->pkt_has_scalar_store_s1) { \
101 + if (insn->slot == 0 && ctx->pkt.pkt_has_scalar_store_s1) { \
102 process_store(ctx, 1); \
103 } \
104 } while (0)
target/hexagon/translate.c
+48 -65
@@ -156,8 +156,6 @@ static void gen_goto_tb(DisasContext *ctx, unsigned tb_slot_idx,
156
157 static void gen_end_tb(DisasContext *ctx)
158 {
159 - Packet *pkt = ctx->pkt;
160 -
159 gen_exec_counters(ctx);
160
161 if (ctx->branch_cond != TCG_COND_NEVER) {
@@ -171,7 +169,7 @@ static void gen_end_tb(DisasContext *ctx)
169 gen_goto_tb(ctx, 0, ctx->branch_dest, true);
170 }
171 } else if (ctx->is_tight_loop &&
174 - pkt->insn[pkt->num_insns - 1].opcode == J2_endloop0) {
172 + ctx->pkt.insn[ctx->pkt.num_insns - 1].opcode == J2_endloop0) {
173 /*
174 * When we're in a tight loop, we defer the endloop0 processing
175 * to take advantage of direct block chaining
@@ -266,11 +264,9 @@ static bool need_slot_cancelled(Packet *pkt)
264
265 static bool need_next_PC(DisasContext *ctx)
266 {
269 - Packet *pkt = ctx->pkt;
270 -
267 /* Check for conditional control flow or HW loop end */
272 - for (int i = 0; i < pkt->num_insns; i++) {
273 - uint16_t opcode = pkt->insn[i].opcode;
268 + for (int i = 0; i < ctx->pkt.num_insns; i++) {
269 + uint16_t opcode = ctx->pkt.insn[i].opcode;
270 if (GET_ATTRIB(opcode, A_CONDEXEC) && GET_ATTRIB(opcode, A_COF)) {
271 return true;
272 }
@@ -353,8 +349,6 @@ static bool pkt_raises_exception(Packet *pkt)
349
350 static bool need_commit(DisasContext *ctx)
351 {
356 - Packet *pkt = ctx->pkt;
357 -
352 /*
353 * If the short-circuit property is set to false, we'll always do the commit
354 */
@@ -362,7 +356,7 @@ static bool need_commit(DisasContext *ctx)
356 return true;
357 }
358
365 - if (pkt_raises_exception(pkt)) {
359 + if (pkt_raises_exception(&ctx->pkt)) {
360 return true;
361 }
362
@@ -409,11 +403,10 @@ static void mark_implicit_writes(DisasContext *ctx)
403
404 static void analyze_packet(DisasContext *ctx)
405 {
412 - Packet *pkt = ctx->pkt;
406 ctx->read_after_write = false;
407 ctx->has_hvx_overlap = false;
415 - for (int i = 0; i < pkt->num_insns; i++) {
416 - Insn *insn = &pkt->insn[i];
408 + for (int i = 0; i < ctx->pkt.num_insns; i++) {
409 + Insn *insn = &ctx->pkt.insn[i];
410 ctx->insn = insn;
411 if (opcode_analyze[insn->opcode]) {
412 opcode_analyze[insn->opcode](ctx);
@@ -425,8 +418,7 @@ static void analyze_packet(DisasContext *ctx)
418
419 static void gen_start_packet(DisasContext *ctx)
420 {
428 - Packet *pkt = ctx->pkt;
429 - target_ulong next_PC = ctx->base.pc_next + pkt->encod_pkt_size_in_bytes;
421 + target_ulong next_PC = ctx->base.pc_next + ctx->pkt.encod_pkt_size_in_bytes;
422 int i;
423
424 /* Clear out the disassembly context */
@@ -468,13 +460,13 @@ static void gen_start_packet(DisasContext *ctx)
460 bitmap_zero(ctx->pregs_written, NUM_PREGS);
461
462 /* Initialize the runtime state for packet semantics */
471 - if (need_slot_cancelled(pkt)) {
463 + if (need_slot_cancelled(&ctx->pkt)) {
464 tcg_gen_movi_tl(hex_slot_cancelled, 0);
465 }
466 ctx->branch_taken = NULL;
475 - if (pkt->pkt_has_cof) {
467 + if (ctx->pkt.pkt_has_cof) {
468 ctx->branch_taken = tcg_temp_new();
477 - if (pkt->pkt_has_multi_cof) {
469 + if (ctx->pkt.pkt_has_multi_cof) {
470 tcg_gen_movi_tl(ctx->branch_taken, 0);
471 }
472 if (need_next_PC(ctx)) {
@@ -503,7 +495,7 @@ static void gen_start_packet(DisasContext *ctx)
495 * Preload the predicated pred registers into ctx->new_pred_value[pred_num]
496 * Only endloop instructions conditionally write to pred registers
497 */
506 - if (ctx->need_commit && pkt->pkt_has_endloop) {
498 + if (ctx->need_commit && ctx->pkt.pkt_has_endloop) {
499 for (i = 0; i < ctx->preg_log_idx; i++) {
500 int pred_num = ctx->preg_log[i];
501 ctx->new_pred_value[pred_num] = tcg_temp_new();
@@ -542,13 +534,11 @@ static void gen_start_packet(DisasContext *ctx)
534
535 bool is_gather_store_insn(DisasContext *ctx)
536 {
545 - Packet *pkt = ctx->pkt;
546 - Insn *insn = ctx->insn;
547 - if (GET_ATTRIB(insn->opcode, A_CVI_NEW) &&
548 - insn->new_value_producer_slot == 1) {
537 + if (GET_ATTRIB(ctx->insn->opcode, A_CVI_NEW) &&
538 + ctx->insn->new_value_producer_slot == 1) {
539 /* Look for gather instruction */
550 - for (int i = 0; i < pkt->num_insns; i++) {
551 - Insn *in = &pkt->insn[i];
540 + for (int i = 0; i < ctx->pkt.num_insns; i++) {
541 + Insn *in = &ctx->pkt.insn[i];
542 if (GET_ATTRIB(in->opcode, A_CVI_GATHER) && in->slot == 1) {
543 return true;
544 }
@@ -651,7 +641,7 @@ static bool slot_is_predicated(Packet *pkt, int slot_num)
641
642 void process_store(DisasContext *ctx, int slot_num)
643 {
654 - bool is_predicated = slot_is_predicated(ctx->pkt, slot_num);
644 + bool is_predicated = slot_is_predicated(&ctx->pkt, slot_num);
645 TCGLabel *label_end = NULL;
646
647 /*
@@ -728,13 +718,12 @@ static void process_store_log(DisasContext *ctx)
718 * slot 1 and then slot 0. This will be important when
719 * the memory accesses overlap.
720 */
731 - Packet *pkt = ctx->pkt;
732 - if (pkt->pkt_has_scalar_store_s1) {
733 - g_assert(!pkt->pkt_has_dczeroa);
721 + if (ctx->pkt.pkt_has_scalar_store_s1) {
722 + g_assert(!ctx->pkt.pkt_has_dczeroa);
723 process_store(ctx, 1);
724 }
736 - if (pkt->pkt_has_scalar_store_s0) {
737 - g_assert(!pkt->pkt_has_dczeroa);
725 + if (ctx->pkt.pkt_has_scalar_store_s0) {
726 + g_assert(!ctx->pkt.pkt_has_dczeroa);
727 process_store(ctx, 0);
728 }
729 }
@@ -742,7 +731,7 @@ static void process_store_log(DisasContext *ctx)
731 /* Zero out a 32-bit cache line */
732 static void process_dczeroa(DisasContext *ctx)
733 {
745 - if (ctx->pkt->pkt_has_dczeroa) {
734 + if (ctx->pkt.pkt_has_dczeroa) {
735 /* Store 32 bytes of zero starting at (addr & ~0x1f) */
736 TCGv addr = tcg_temp_new();
737 TCGv_i64 zero = tcg_constant_i64(0);
@@ -776,7 +765,7 @@ static void gen_commit_hvx(DisasContext *ctx)
765
766 /* Early exit if not needed */
767 if (!ctx->need_commit) {
779 - g_assert(!pkt_has_hvx_store(ctx->pkt));
768 + g_assert(!pkt_has_hvx_store(&ctx->pkt));
769 return;
770 }
771
@@ -810,25 +799,23 @@ static void gen_commit_hvx(DisasContext *ctx)
799 tcg_gen_gvec_mov(MO_64, dstoff, srcoff, size, size);
800 }
801
813 - if (pkt_has_hvx_store(ctx->pkt)) {
802 + if (pkt_has_hvx_store(&ctx->pkt)) {
803 gen_helper_commit_hvx_stores(tcg_env);
804 }
805 }
806
807 static void update_exec_counters(DisasContext *ctx)
808 {
820 - Packet *pkt = ctx->pkt;
821 - int num_insns = pkt->num_insns;
809 int num_real_insns = 0;
810 int num_hvx_insns = 0;
811
825 - for (int i = 0; i < num_insns; i++) {
826 - if (!pkt->insn[i].is_endloop &&
827 - !pkt->insn[i].part1 &&
828 - !GET_ATTRIB(pkt->insn[i].opcode, A_IT_NOP)) {
812 + for (int i = 0; i < ctx->pkt.num_insns; i++) {
813 + if (!ctx->pkt.insn[i].is_endloop &&
814 + !ctx->pkt.insn[i].part1 &&
815 + !GET_ATTRIB(ctx->pkt.insn[i].opcode, A_IT_NOP)) {
816 num_real_insns++;
817 }
831 - if (GET_ATTRIB(pkt->insn[i].opcode, A_CVI)) {
818 + if (GET_ATTRIB(ctx->pkt.insn[i].opcode, A_CVI)) {
819 num_hvx_insns++;
820 }
821 }
@@ -857,12 +844,11 @@ static void gen_commit_packet(DisasContext *ctx)
844 * store. Therefore, we call process_store_log before anything else
845 * involved in committing the packet.
846 */
860 - Packet *pkt = ctx->pkt;
861 - bool has_store_s0 = pkt->pkt_has_scalar_store_s0;
847 + bool has_store_s0 = ctx->pkt.pkt_has_scalar_store_s0;
848 bool has_store_s1 =
863 - (pkt->pkt_has_scalar_store_s1 && !ctx->s1_store_processed);
864 - bool has_hvx_store = pkt_has_hvx_store(pkt);
865 - if (pkt->pkt_has_dczeroa) {
849 + (ctx->pkt.pkt_has_scalar_store_s1 && !ctx->s1_store_processed);
850 + bool has_hvx_store = pkt_has_hvx_store(&ctx->pkt);
851 + if (ctx->pkt.pkt_has_dczeroa) {
852 /*
853 * The dczeroa will be the store in slot 0, check that we don't have
854 * a store in slot 1 or an HVX store.
@@ -889,12 +875,11 @@ static void gen_commit_packet(DisasContext *ctx)
875 FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES,
876 HAS_HVX_STORES, 1);
877 }
892 - if (has_store_s0 && slot_is_predicated(pkt, 0)) {
893 - mask =
894 - FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES,
895 - S0_IS_PRED, 1);
878 + if (has_store_s0 && slot_is_predicated(&ctx->pkt, 0)) {
879 + mask = FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES, S0_IS_PRED,
880 + 1);
881 }
897 - if (has_store_s1 && slot_is_predicated(pkt, 1)) {
882 + if (has_store_s1 && slot_is_predicated(&ctx->pkt, 1)) {
883 mask =
884 FIELD_DP32(mask, PROBE_PKT_SCALAR_HVX_STORES,
885 S1_IS_PRED, 1);
@@ -912,7 +897,7 @@ static void gen_commit_packet(DisasContext *ctx)
897 int args = 0;
898 args =
899 FIELD_DP32(args, PROBE_PKT_SCALAR_STORE_S0, MMU_IDX, ctx->mem_idx);
915 - if (slot_is_predicated(pkt, 0)) {
900 + if (slot_is_predicated(&ctx->pkt, 0)) {
901 args =
902 FIELD_DP32(args, PROBE_PKT_SCALAR_STORE_S0, IS_PREDICATED, 1);
903 }
@@ -924,18 +909,18 @@ static void gen_commit_packet(DisasContext *ctx)
909
910 gen_reg_writes(ctx);
911 gen_pred_writes(ctx);
927 - if (pkt->pkt_has_hvx) {
912 + if (ctx->pkt.pkt_has_hvx) {
913 gen_commit_hvx(ctx);
914 }
915 update_exec_counters(ctx);
916
932 - if (pkt->vhist_insn != NULL) {
917 + if (ctx->pkt.vhist_insn != NULL) {
918 ctx->pre_commit = false;
934 - ctx->insn = pkt->vhist_insn;
935 - pkt->vhist_insn->generate(ctx);
919 + ctx->insn = ctx->pkt.vhist_insn;
920 + ctx->pkt.vhist_insn->generate(ctx);
921 }
922
938 - if (pkt->pkt_has_cof) {
923 + if (ctx->pkt.pkt_has_cof) {
924 gen_end_tb(ctx);
925 }
926 }
@@ -944,7 +929,6 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
929 {
930 uint32_t words[PACKET_WORDS_MAX];
931 int nwords, words_read;
947 - Packet pkt;
932 int i;
933
934 nwords = read_packet_words(env, ctx, words);
@@ -953,22 +937,21 @@ static void decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
937 return;
938 }
939
956 - ctx->pkt = &pkt;
957 - words_read = decode_packet(ctx, nwords, words, &pkt, false);
940 + words_read = decode_packet(ctx, nwords, words, &ctx->pkt, false);
941 if (words_read > 0) {
959 - pkt.pc = ctx->base.pc_next;
960 - if (pkt.pkt_has_write_conflict) {
942 + ctx->pkt.pc = ctx->base.pc_next;
943 + if (ctx->pkt.pkt_has_write_conflict) {
944 gen_exception_decode_fail(ctx, words_read,
945 HEX_CAUSE_REG_WRITE_CONFLICT);
946 return;
947 }
948 gen_start_packet(ctx);
966 - for (i = 0; i < pkt.num_insns; i++) {
967 - ctx->insn = &pkt.insn[i];
949 + for (i = 0; i < ctx->pkt.num_insns; i++) {
950 + ctx->insn = &ctx->pkt.insn[i];
951 gen_insn(ctx);
952 }
953 gen_commit_packet(ctx);
971 - ctx->base.pc_next += pkt.encod_pkt_size_in_bytes;
954 + ctx->base.pc_next += ctx->pkt.encod_pkt_size_in_bytes;
955 } else {
956 gen_exception_decode_fail(ctx, nwords, HEX_CAUSE_INVALID_PACKET);
957 }
target/hexagon/translate.h
+1 -1
@@ -28,7 +28,7 @@
28
29 typedef struct DisasContext {
30 DisasContextBase base;
31 - Packet *pkt;
31 + Packet pkt;
32 Insn *insn;
33 const HexagonCPUDef *hex_def;
34 uint32_t next_PC;