On Thu, Jan 22, 2026 at 3:34 PM Marco Liebel <[email protected]>
wrote:

> 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 <[email protected]>
> ---
>  target/hexagon/gen_tcg.h        |   2 +-
>  target/hexagon/macros.h         |   6 +-
>  target/hexagon/translate.h      |   2 +-
>  target/hexagon/decode.c         |   8 +--
>  target/hexagon/genptr.c         |  14 ++--
>  target/hexagon/translate.c      | 111 ++++++++++++++------------------
>  target/hexagon/gen_tcg_funcs.py |   2 +-
>  target/hexagon/hex_common.py    |   4 +-
>  8 files changed, 65 insertions(+), 84 deletions(-)
>
>
> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
> index a0102b6cbd..ffbfc3ac0e 100644
> --- a/target/hexagon/translate.h
> +++ b/target/hexagon/translate.h
> @@ -28,7 +28,7 @@
>
>  typedef struct DisasContext {
>      DisasContextBase base;
> -    Packet *pkt;
> +    Packet pkt;
>

This patch would be alot smaller if you kept pkt as-is and added a new
member
Packet packet;


>      Insn *insn;
>      uint32_t next_PC;
>      uint32_t mem_idx;
> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
> index b5ece60450..c2516d927d 100644
> --- a/target/hexagon/decode.c
> +++ b/target/hexagon/decode.c
> @@ -748,14 +748,12 @@ int disassemble_hexagon(uint32_t *words, int nwords,
> bfd_vma pc,
>                          GString *buf)
>  {
>      DisasContext ctx;
> -    Packet pkt;
>
>      memset(&ctx, 0, sizeof(DisasContext));
> -    ctx.pkt = &pkt;
>
> Then, this line would be
ctx.pkt = &ctx.packet;


@@ -930,7 +915,6 @@ static void decode_and_translate_packet(CPUHexagonState
> *env, DisasContext *ctx)
>  {
>      uint32_t words[PACKET_WORDS_MAX];
>      int nwords;
> -    Packet pkt;
>      int i;
>
>      nwords = read_packet_words(env, ctx, words);
> @@ -939,16 +923,15 @@ static void
> decode_and_translate_packet(CPUHexagonState *env, DisasContext *ctx)
>          return;
>      }
>
> -    ctx->pkt = &pkt;
>

Ditto

Thanks,
Taylor

Reply via email to