| Message ID | 20221106023735.5277-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Two -Wclobbered fixes, plus other cleanup | expand |
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; > } >
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>
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; }
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(-)