[RESEND PATCH 0/6] Two -Wclobbered fixes, plus other cleanup

[RESEND PATCH 0/6] Two -Wclobbered fixes, plus other cleanup

[Richard Henderson]

[ Resend, since apparently only one patch made it to the list. ]

Stefan reported for accel/tcg, and I reproduced on Ubuntu 22.04.


r~

Richard Henderson (6):
  disas/nanomips: Move setjmp into nanomips_dis
  disas/nanomips: Merge insn{1,2,3} into words[3]
  disas/nanomips: Split out read_u16
  disas/nanomips: Tidy read for 48-bit opcodes
  tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code
  accel/tcg: Split out setjmp_gen_code

 accel/tcg/translate-all.c |  68 ++++++++++++------------
 disas/nanomips.c          | 106 ++++++++++++++++----------------------
 tcg/tcg.c                 |  12 +++++
 3 files changed, 90 insertions(+), 96 deletions(-)

-- 
2.34.1

[RESEND PATCH 1/6] disas/nanomips: Move setjmp into nanomips_dis

[Richard Henderson]

Reduce the number of local variables within the scope of the
setjmp by moving it to the existing helper.  The actual length
returned from Disassemble is not used, because we have already
determined the length while reading bytes.  Fixes:

nanomips.c: In function ‘print_insn_nanomips’:
nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas/nanomips.c | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index 9647f1a8e3..9a69e6880a 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,22 +21905,27 @@ static const Pool MAJOR[2] = {
        0x0                 },        /* P16 */
 };
 
-static int nanomips_dis(char **buf,
-                 Dis_info *info,
-                 unsigned short one,
-                 unsigned short two,
-                 unsigned short three)
+static bool nanomips_dis(char **buf, Dis_info *info,
+                         unsigned short one,
+                         unsigned short two,
+                         unsigned short three)
 {
     uint16 bits[3] = {one, two, three};
-
     TABLE_ENTRY_TYPE type;
-    int size = Disassemble(bits, buf, &type, MAJOR, 2, info);
-    return size;
+    int ret;
+
+    ret = sigsetjmp(info->buf, 0);
+    if (ret != 0) {
+        return false;
+    }
+
+    ret = Disassemble(bits, buf, &type, MAJOR, 2, info);
+    return ret >= 0;
 }
 
 int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
 {
-    int status;
+    int status, length;
     bfd_byte buffer[2];
     uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
     g_autofree char *buf = NULL;
@@ -21950,6 +21955,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
     } else {
         insn1 = bfd_getl16(buffer);
     }
+    length = 2;
     (*info->fprintf_func)(info->stream, "%04x ", insn1);
 
     /* Handle 32-bit opcodes.  */
@@ -21965,6 +21971,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
         } else {
             insn2 = bfd_getl16(buffer);
         }
+        length = 4;
         (*info->fprintf_func)(info->stream, "%04x ", insn2);
     } else {
         (*info->fprintf_func)(info->stream, "     ");
@@ -21982,27 +21989,16 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
         } else {
             insn3 = bfd_getl16(buffer);
         }
+        length = 6;
         (*info->fprintf_func)(info->stream, "%04x ", insn3);
     } else {
         (*info->fprintf_func)(info->stream, "     ");
     }
 
     /* Handle runtime errors. */
-    if (sigsetjmp(disassm_info.buf, 0) != 0) {
-        info->insn_type = dis_noninsn;
-        return insn3 ? 6 : insn2 ? 4 : 2;
+    if (nanomips_dis(&buf, &disassm_info, insn1, insn2, insn3)) {
+        (*info->fprintf_func) (info->stream, "%s", buf);
     }
 
-    int length = nanomips_dis(&buf, &disassm_info, insn1, insn2, insn3);
-
-    /* FIXME: Should probably use a hash table on the major opcode here.  */
-
-    (*info->fprintf_func) (info->stream, "%s", buf);
-    if (length > 0) {
-        return length / 8;
-    }
-
-    info->insn_type = dis_noninsn;
-
-    return insn3 ? 6 : insn2 ? 4 : 2;
+    return length;
 }
-- 
2.34.1

[RESEND PATCH 2/6] disas/nanomips: Merge insn{1,2,3} into words[3]

[Richard Henderson]

Since Disassemble wants the data in this format, collect
it that way.  This allows using a loop to print the bytes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas/nanomips.c | 44 +++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index 9a69e6880a..5438def9af 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21905,12 +21905,8 @@ static const Pool MAJOR[2] = {
        0x0                 },        /* P16 */
 };
 
-static bool nanomips_dis(char **buf, Dis_info *info,
-                         unsigned short one,
-                         unsigned short two,
-                         unsigned short three)
+static bool nanomips_dis(char **buf, Dis_info *info, uint16_t words[3])
 {
-    uint16 bits[3] = {one, two, three};
     TABLE_ENTRY_TYPE type;
     int ret;
 
@@ -21919,7 +21915,7 @@ static bool nanomips_dis(char **buf, Dis_info *info,
         return false;
     }
 
-    ret = Disassemble(bits, buf, &type, MAJOR, 2, info);
+    ret = Disassemble(words, buf, &type, MAJOR, 2, info);
     return ret >= 0;
 }
 
@@ -21927,7 +21923,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
 {
     int status, length;
     bfd_byte buffer[2];
-    uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
+    uint16_t words[3] = { };
     g_autofree char *buf = NULL;
 
     info->bytes_per_chunk = 2;
@@ -21951,15 +21947,14 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
     }
 
     if (info->endian == BFD_ENDIAN_BIG) {
-        insn1 = bfd_getb16(buffer);
+        words[0] = bfd_getb16(buffer);
     } else {
-        insn1 = bfd_getl16(buffer);
+        words[0] = bfd_getl16(buffer);
     }
     length = 2;
-    (*info->fprintf_func)(info->stream, "%04x ", insn1);
 
     /* Handle 32-bit opcodes.  */
-    if ((insn1 & 0x1000) == 0) {
+    if ((words[0] & 0x1000) == 0) {
         status = (*info->read_memory_func)(memaddr + 2, buffer, 2, info);
         if (status != 0) {
             (*info->memory_error_func)(status, memaddr + 2, info);
@@ -21967,17 +21962,15 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
         }
 
         if (info->endian == BFD_ENDIAN_BIG) {
-            insn2 = bfd_getb16(buffer);
+            words[1] = bfd_getb16(buffer);
         } else {
-            insn2 = bfd_getl16(buffer);
+            words[1] = bfd_getl16(buffer);
         }
         length = 4;
-        (*info->fprintf_func)(info->stream, "%04x ", insn2);
-    } else {
-        (*info->fprintf_func)(info->stream, "     ");
     }
+
     /* Handle 48-bit opcodes.  */
-    if ((insn1 >> 10) == 0x18) {
+    if ((words[0] >> 10) == 0x18) {
         status = (*info->read_memory_func)(memaddr + 4, buffer, 2, info);
         if (status != 0) {
             (*info->memory_error_func)(status, memaddr + 4, info);
@@ -21985,18 +21978,23 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
         }
 
         if (info->endian == BFD_ENDIAN_BIG) {
-            insn3 = bfd_getb16(buffer);
+            words[2] = bfd_getb16(buffer);
         } else {
-            insn3 = bfd_getl16(buffer);
+            words[2] = bfd_getl16(buffer);
         }
         length = 6;
-        (*info->fprintf_func)(info->stream, "%04x ", insn3);
-    } else {
-        (*info->fprintf_func)(info->stream, "     ");
+    }
+
+    for (int i = 0; i < 6; i += 2) {
+        if (i < length) {
+            (*info->fprintf_func)(info->stream, "%04x ", words[i / 2]);
+        } else {
+            (*info->fprintf_func)(info->stream, "     ");
+        }
     }
 
     /* Handle runtime errors. */
-    if (nanomips_dis(&buf, &disassm_info, insn1, insn2, insn3)) {
+    if (nanomips_dis(&buf, &disassm_info, words)) {
         (*info->fprintf_func) (info->stream, "%s", buf);
     }
 
-- 
2.34.1

[RESEND PATCH 3/6] disas/nanomips: Split out read_u16

[Richard Henderson]

Split out a helper function for reading a uint16_t
with the correct endianness.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas/nanomips.c | 48 +++++++++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index 5438def9af..52c7537379 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21919,10 +21919,24 @@ static bool nanomips_dis(char **buf, Dis_info *info, uint16_t words[3])
     return ret >= 0;
 }
 
+static bool read_u16(uint16_t *ret, bfd_vma memaddr,
+                     struct disassemble_info *info)
+{
+    int status = (*info->read_memory_func)(memaddr, (bfd_byte *)ret, 2, info);
+    if (status != 0) {
+        (*info->memory_error_func)(status, memaddr, info);
+        return false;
+    }
+
+    if ((info->endian == BFD_ENDIAN_BIG) != HOST_BIG_ENDIAN) {
+        bswap16s(ret);
+    }
+    return true;
+}
+
 int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
 {
-    int status, length;
-    bfd_byte buffer[2];
+    int length;
     uint16_t words[3] = { };
     g_autofree char *buf = NULL;
 
@@ -21940,48 +21954,24 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
     disassm_info.fprintf_func = info->fprintf_func;
     disassm_info.stream = info->stream;
 
-    status = (*info->read_memory_func)(memaddr, buffer, 2, info);
-    if (status != 0) {
-        (*info->memory_error_func)(status, memaddr, info);
+    if (!read_u16(&words[0], memaddr, info)) {
         return -1;
     }
-
-    if (info->endian == BFD_ENDIAN_BIG) {
-        words[0] = bfd_getb16(buffer);
-    } else {
-        words[0] = bfd_getl16(buffer);
-    }
     length = 2;
 
     /* Handle 32-bit opcodes.  */
     if ((words[0] & 0x1000) == 0) {
-        status = (*info->read_memory_func)(memaddr + 2, buffer, 2, info);
-        if (status != 0) {
-            (*info->memory_error_func)(status, memaddr + 2, info);
+        if (!read_u16(&words[1], memaddr + 2, info)) {
             return -1;
         }
-
-        if (info->endian == BFD_ENDIAN_BIG) {
-            words[1] = bfd_getb16(buffer);
-        } else {
-            words[1] = bfd_getl16(buffer);
-        }
         length = 4;
     }
 
     /* Handle 48-bit opcodes.  */
     if ((words[0] >> 10) == 0x18) {
-        status = (*info->read_memory_func)(memaddr + 4, buffer, 2, info);
-        if (status != 0) {
-            (*info->memory_error_func)(status, memaddr + 4, info);
+        if (!read_u16(&words[1], memaddr + 4, info)) {
             return -1;
         }
-
-        if (info->endian == BFD_ENDIAN_BIG) {
-            words[2] = bfd_getb16(buffer);
-        } else {
-            words[2] = bfd_getl16(buffer);
-        }
         length = 6;
     }
 
-- 
2.34.1

[RESEND PATCH 4/6] disas/nanomips: Tidy read for 48-bit opcodes

[Richard Henderson]

There is no point in looking for a 48-bit opcode if we've
not read the second word for a 32-bit opcode.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas/nanomips.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index 52c7537379..092ea0ca0c 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -21965,14 +21965,14 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
             return -1;
         }
         length = 4;
-    }
 
-    /* Handle 48-bit opcodes.  */
-    if ((words[0] >> 10) == 0x18) {
-        if (!read_u16(&words[1], memaddr + 4, info)) {
-            return -1;
+        /* Handle 48-bit opcodes.  */
+        if ((words[0] >> 10) == 0x18) {
+            if (!read_u16(&words[1], memaddr + 4, info)) {
+                return -1;
+            }
+            length = 6;
         }
-        length = 6;
     }
 
     for (int i = 0; i < 6; i += 2) {
-- 
2.34.1

[RESEND PATCH 5/6] tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/translate-all.c | 10 ----------
 tcg/tcg.c                 | 12 ++++++++++++
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 921944a5ab..9ee21f7f52 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -821,16 +821,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     trace_translate_block(tb, pc, tb->tc.ptr);
 
     /* generate machine code */
-    tb->jmp_reset_offset[0] = TB_JMP_RESET_OFFSET_INVALID;
-    tb->jmp_reset_offset[1] = TB_JMP_RESET_OFFSET_INVALID;
-    tcg_ctx->tb_jmp_reset_offset = tb->jmp_reset_offset;
-    if (TCG_TARGET_HAS_direct_jump) {
-        tcg_ctx->tb_jmp_insn_offset = tb->jmp_target_arg;
-        tcg_ctx->tb_jmp_target_addr = NULL;
-    } else {
-        tcg_ctx->tb_jmp_insn_offset = NULL;
-        tcg_ctx->tb_jmp_target_addr = tb->jmp_target_arg;
-    }
 
 #ifdef CONFIG_PROFILER
     qatomic_set(&prof->tb_count, prof->tb_count + 1);
diff --git a/tcg/tcg.c b/tcg/tcg.c
index b43b6a7981..436fcf6ebd 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4228,6 +4228,18 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb, target_ulong pc_start)
     }
 #endif
 
+    /* Initialize goto_tb jump offsets. */
+    tb->jmp_reset_offset[0] = TB_JMP_RESET_OFFSET_INVALID;
+    tb->jmp_reset_offset[1] = TB_JMP_RESET_OFFSET_INVALID;
+    tcg_ctx->tb_jmp_reset_offset = tb->jmp_reset_offset;
+    if (TCG_TARGET_HAS_direct_jump) {
+        tcg_ctx->tb_jmp_insn_offset = tb->jmp_target_arg;
+        tcg_ctx->tb_jmp_target_addr = NULL;
+    } else {
+        tcg_ctx->tb_jmp_insn_offset = NULL;
+        tcg_ctx->tb_jmp_target_addr = tb->jmp_target_arg;
+    }
+
     tcg_reg_alloc_start(s);
 
     /*
-- 
2.34.1

[RESEND PATCH 6/6] accel/tcg: Split out setjmp_gen_code

[Richard Henderson]

Isolate the code protected by setjmp.  Fixes:

translate-all.c: In function ‘tb_gen_code’:
translate-all.c:748:51: error: argument ‘cflags’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/translate-all.c | 58 ++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9ee21f7f52..ac3ee3740c 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -742,6 +742,37 @@ void page_collection_unlock(struct page_collection *set)
 
 #endif /* !CONFIG_USER_ONLY */
 
+/*
+ * Isolate the portion of code gen which can setjmp/longjmp.
+ * Return the size of the generated code, or negative on error.
+ */
+static int setjmp_gen_code(CPUArchState *env, TranslationBlock *tb,
+                           target_ulong pc, void *host_pc,
+                           int *max_insns, int64_t *ti)
+{
+    int ret = sigsetjmp(tcg_ctx->jmp_trans, 0);
+    if (unlikely(ret != 0)) {
+        return ret;
+    }
+
+    tcg_func_start(tcg_ctx);
+
+    tcg_ctx->cpu = env_cpu(env);
+    gen_intermediate_code(env_cpu(env), tb, *max_insns, pc, host_pc);
+    assert(tb->size != 0);
+    tcg_ctx->cpu = NULL;
+    *max_insns = tb->icount;
+
+#ifdef CONFIG_PROFILER
+    qatomic_set(&tcg_ctx->prof.tb_count, tcg_ctx->prof.tb_count + 1);
+    qatomic_set(&tcg_ctx->prof.interm_time,
+                tcg_ctx->prof.interm_time + profile_getclock() - *ti);
+    *ti = profile_getclock();
+#endif
+
+    return tcg_gen_code(tcg_ctx, tb, pc);
+}
+
 /* Called with mmap_lock held for user mode emulation.  */
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
@@ -754,8 +785,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     int gen_code_size, search_size, max_insns;
 #ifdef CONFIG_PROFILER
     TCGProfile *prof = &tcg_ctx->prof;
-    int64_t ti;
 #endif
+    int64_t ti;
     void *host_pc;
 
     assert_memory_lock();
@@ -805,33 +836,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     ti = profile_getclock();
 #endif
 
-    gen_code_size = sigsetjmp(tcg_ctx->jmp_trans, 0);
-    if (unlikely(gen_code_size != 0)) {
-        goto error_return;
-    }
-
-    tcg_func_start(tcg_ctx);
-
-    tcg_ctx->cpu = env_cpu(env);
-    gen_intermediate_code(cpu, tb, max_insns, pc, host_pc);
-    assert(tb->size != 0);
-    tcg_ctx->cpu = NULL;
-    max_insns = tb->icount;
-
     trace_translate_block(tb, pc, tb->tc.ptr);
 
-    /* generate machine code */
-
-#ifdef CONFIG_PROFILER
-    qatomic_set(&prof->tb_count, prof->tb_count + 1);
-    qatomic_set(&prof->interm_time,
-                prof->interm_time + profile_getclock() - ti);
-    ti = profile_getclock();
-#endif
-
-    gen_code_size = tcg_gen_code(tcg_ctx, tb, pc);
+    gen_code_size = setjmp_gen_code(env, tb, pc, host_pc, &max_insns, &ti);
     if (unlikely(gen_code_size < 0)) {
- error_return:
         switch (gen_code_size) {
         case -1:
             /*
-- 
2.34.1

Re: [RESEND PATCH 1/6] disas/nanomips: Move setjmp into nanomips_dis

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 4312 bytes --]

On Sun, 6 Nov 2022, Richard Henderson wrote:
> Reduce the number of local variables within the scope of the
> setjmp by moving it to the existing helper.  The actual length
> returned from Disassemble is not used, because we have already
> determined the length while reading bytes.  Fixes:
>
> nanomips.c: In function ‘print_insn_nanomips’:
> nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas/nanomips.c | 44 ++++++++++++++++++++------------------------
> 1 file changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/disas/nanomips.c b/disas/nanomips.c
> index 9647f1a8e3..9a69e6880a 100644
> --- a/disas/nanomips.c
> +++ b/disas/nanomips.c
> @@ -21905,22 +21905,27 @@ static const Pool MAJOR[2] = {
>        0x0                 },        /* P16 */
> };
>
> -static int nanomips_dis(char **buf,
> -                 Dis_info *info,
> -                 unsigned short one,
> -                 unsigned short two,
> -                 unsigned short three)
> +static bool nanomips_dis(char **buf, Dis_info *info,
> +                         unsigned short one,
> +                         unsigned short two,
> +                         unsigned short three)
> {
>     uint16 bits[3] = {one, two, three};
> -
>     TABLE_ENTRY_TYPE type;
> -    int size = Disassemble(bits, buf, &type, MAJOR, 2, info);
> -    return size;
> +    int ret;
> +
> +    ret = sigsetjmp(info->buf, 0);
> +    if (ret != 0) {
> +        return false;
> +    }
> +
> +    ret = Disassemble(bits, buf, &type, MAJOR, 2, info);
> +    return ret >= 0;
> }

Maybe you could lose ret too and simplify it to something like this?

if (sigsetjmp(info->buf, 0)) {
     return false;
}

return Disassemble(bits, buf, &type, MAJOR, 2, info) >= 0;

Storing the return value in a local car just to use it in the next line 
does not seem necessary to me but it's just an idea, not really important 
so as you like.

Regards,
BALATON Zoltan

> int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
> {
> -    int status;
> +    int status, length;
>     bfd_byte buffer[2];
>     uint16_t insn1 = 0, insn2 = 0, insn3 = 0;
>     g_autofree char *buf = NULL;
> @@ -21950,6 +21955,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
>     } else {
>         insn1 = bfd_getl16(buffer);
>     }
> +    length = 2;
>     (*info->fprintf_func)(info->stream, "%04x ", insn1);
>
>     /* Handle 32-bit opcodes.  */
> @@ -21965,6 +21971,7 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
>         } else {
>             insn2 = bfd_getl16(buffer);
>         }
> +        length = 4;
>         (*info->fprintf_func)(info->stream, "%04x ", insn2);
>     } else {
>         (*info->fprintf_func)(info->stream, "     ");
> @@ -21982,27 +21989,16 @@ int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
>         } else {
>             insn3 = bfd_getl16(buffer);
>         }
> +        length = 6;
>         (*info->fprintf_func)(info->stream, "%04x ", insn3);
>     } else {
>         (*info->fprintf_func)(info->stream, "     ");
>     }
>
>     /* Handle runtime errors. */
> -    if (sigsetjmp(disassm_info.buf, 0) != 0) {
> -        info->insn_type = dis_noninsn;
> -        return insn3 ? 6 : insn2 ? 4 : 2;
> +    if (nanomips_dis(&buf, &disassm_info, insn1, insn2, insn3)) {
> +        (*info->fprintf_func) (info->stream, "%s", buf);
>     }
>
> -    int length = nanomips_dis(&buf, &disassm_info, insn1, insn2, insn3);
> -
> -    /* FIXME: Should probably use a hash table on the major opcode here.  */
> -
> -    (*info->fprintf_func) (info->stream, "%s", buf);
> -    if (length > 0) {
> -        return length / 8;
> -    }
> -
> -    info->insn_type = dis_noninsn;
> -
> -    return insn3 ? 6 : insn2 ? 4 : 2;
> +    return length;
> }
>

Re: [RESEND PATCH 2/6] disas/nanomips: Merge insn{1,2,3} into words[3]

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> Since Disassemble wants the data in this format, collect
> it that way.  This allows using a loop to print the bytes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   disas/nanomips.c | 44 +++++++++++++++++++++-----------------------
>   1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/disas/nanomips.c b/disas/nanomips.c
> index 9a69e6880a..5438def9af 100644
> --- a/disas/nanomips.c
> +++ b/disas/nanomips.c
> @@ -21905,12 +21905,8 @@ static const Pool MAJOR[2] = {
>          0x0                 },        /* P16 */
>   };
>   
> -static bool nanomips_dis(char **buf, Dis_info *info,
> -                         unsigned short one,
> -                         unsigned short two,
> -                         unsigned short three)
> +static bool nanomips_dis(char **buf, Dis_info *info, uint16_t words[3])

words[] can be const.

> +    for (int i = 0; i < 6; i += 2) {

I'd rather convert this magic 6 and iterate over ARRAY_SIZE(words).

Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        if (i < length) {
> +            (*info->fprintf_func)(info->stream, "%04x ", words[i / 2]);
> +        } else {
> +            (*info->fprintf_func)(info->stream, "     ");
> +        }
>       }

Re: [RESEND PATCH 3/6] disas/nanomips: Split out read_u16

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> Split out a helper function for reading a uint16_t
> with the correct endianness.

Eh I was thinking about that when reviewing the previous patch :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   disas/nanomips.c | 48 +++++++++++++++++++-----------------------------
>   1 file changed, 19 insertions(+), 29 deletions(-)

Re: [RESEND PATCH 6/6] accel/tcg: Split out setjmp_gen_code

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> Isolate the code protected by setjmp.  Fixes:
> 
> translate-all.c: In function ‘tb_gen_code’:
> translate-all.c:748:51: error: argument ‘cflags’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/translate-all.c | 58 ++++++++++++++++++++++-----------------
>   1 file changed, 33 insertions(+), 25 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [RESEND PATCH 5/6] tcg: Move TCG_TARGET_HAS_direct_jump init to tb_gen_code

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/translate-all.c | 10 ----------
>   tcg/tcg.c                 | 12 ++++++++++++
>   2 files changed, 12 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [RESEND PATCH 4/6] disas/nanomips: Tidy read for 48-bit opcodes

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> There is no point in looking for a 48-bit opcode if we've
> not read the second word for a 32-bit opcode.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   disas/nanomips.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [RESEND PATCH 1/6] disas/nanomips: Move setjmp into nanomips_dis

[Philippe Mathieu-Daudé]

On 6/11/22 03:37, Richard Henderson wrote:
> Reduce the number of local variables within the scope of the
> setjmp by moving it to the existing helper.  The actual length
> returned from Disassemble is not used, because we have already
> determined the length while reading bytes.  Fixes:
> 
> nanomips.c: In function ‘print_insn_nanomips’:
> nanomips.c:21925:14: error: variable ‘insn1’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21925:25: error: variable ‘insn2’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21925:36: error: variable ‘insn3’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> nanomips.c:21926:22: error: variable ‘buf’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   disas/nanomips.c | 44 ++++++++++++++++++++------------------------
>   1 file changed, 20 insertions(+), 24 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>