[PATCH 1/3] x86: work around compiler diagnosing dangling pointer
Alan Modra
amodra@gmail.com
Mon Apr 24 10:24:18 GMT 2023
On Mon, Apr 24, 2023 at 09:34:27AM +0200, Jan Beulich via Binutils wrote:
> For quite come time print_insn() has been storing the address of a local
> variable into info->private_data. Since the compiler can't know that the
> field won't be accessed again after print_insn() returns, it may kind of
> legitimately diagnose this. And recent enough gcc does as of the
> introduction of the fetch_error() return paths (replacing setjmp()-based
> error handling).
>
> Utilizing that neither prefix_name() nor i386_dis_printf() actually use
> info->private_data, zap the pointer in fetch_error(), after having
> retrieved it for local use.
> ---
> Let's hope that this addresses the observed issues, which I haven't been
> seeing myself. And of course there are further return paths which may
> (sooner or later) also have such a warning trigger.
I'll be surprised if your patch is enough. I have the following in my
local tree, tested to work with a freshly built gcc-13 compiler.
Would you like me to commit this (and revert your patch)?
opcodes/i386-dis.c: In function ‘print_insn’:
opcodes/i386-dis.c:9865:22: error: storing the address of local
variable ‘priv’ in ‘*info.private_data’ [-Werror=dangling-pointer=]
* i386-dis.c (print_insn): Clear info->private_data before
returning.
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index f021bdaa3e7..01e5ba81723 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -9731,6 +9731,7 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
{
const struct dis386 *dp;
int i;
+ int ret;
char *op_txt[MAX_OPERANDS];
int needcomma;
bool intel_swap_2_3;
@@ -9887,16 +9888,21 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
i386_dis_printf (&ins, dis_style_mnemonic, "%s%s",
(i == 0 ? "" : " "),
prefix_name (&ins, ins.all_prefixes[i], sizeflag));
- return i;
+ ret = i;
+ goto out;
case ckp_fetch_error:
- return fetch_error (&ins);
+ goto fetch_error_out;
}
ins.insn_codep = ins.codep;
if (!fetch_code (info, ins.codep + 1))
- return fetch_error (&ins);
+ {
+ fetch_error_out:
+ ret = fetch_error (&ins);
+ goto out;
+ }
ins.two_source_ops = (*ins.codep == 0x62) || (*ins.codep == 0xc8);
@@ -9909,7 +9915,8 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
i386_dis_printf (&ins, dis_style_mnemonic, "%s ",
prefix_name (&ins, ins.all_prefixes[i], sizeflag));
i386_dis_printf (&ins, dis_style_mnemonic, "fwait");
- return i + 1;
+ ret = i + 1;
+ goto out;
}
if (*ins.codep == 0x0f)
@@ -9918,7 +9925,7 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
ins.codep++;
if (!fetch_code (info, ins.codep + 1))
- return fetch_error (&ins);
+ goto fetch_error_out;
threebyte = *ins.codep;
dp = &dis386_twobyte[threebyte];
ins.need_modrm = twobyte_has_modrm[threebyte];
@@ -9942,30 +9949,30 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
ins.end_codep = ins.codep;
if (ins.need_modrm && !fetch_modrm (&ins))
- return fetch_error (&ins);
+ goto fetch_error_out;
if (dp->name == NULL && dp->op[0].bytemode == FLOATCODE)
{
if (!get_sib (&ins, sizeflag)
|| !dofloat (&ins, sizeflag))
- return fetch_error (&ins);
+ goto fetch_error_out;
}
else
{
dp = get_valid_dis386 (dp, &ins);
if (dp == &err_opcode)
- return fetch_error (&ins);
+ goto fetch_error_out;
if (dp != NULL && putop (&ins, dp->name, sizeflag) == 0)
{
if (!get_sib (&ins, sizeflag))
- return fetch_error (&ins);
+ goto fetch_error_out;
for (i = 0; i < MAX_OPERANDS; ++i)
{
ins.obufp = ins.op_out[i];
ins.op_ad = MAX_OPERANDS - 1 - i;
if (dp->op[i].rtn
&& !dp->op[i].rtn (&ins, dp->op[i].bytemode, sizeflag))
- return fetch_error (&ins);
+ goto fetch_error_out;
/* For EVEX instruction after the last operand masking
should be printed. */
if (i == 0 && ins.vex.evex)
@@ -10055,14 +10062,16 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
if (ins.need_vex && ins.vex.register_specifier != 0)
{
i386_dis_printf (&ins, dis_style_text, "(bad)");
- return ins.end_codep - priv.the_buffer;
+ ret = ins.end_codep - priv.the_buffer;
+ goto out;
}
/* If EVEX.z is set, there must be an actual mask register in use. */
if (ins.vex.zeroing && ins.vex.mask_register_specifier == 0)
{
i386_dis_printf (&ins, dis_style_text, "(bad)");
- return ins.end_codep - priv.the_buffer;
+ ret = ins.end_codep - priv.the_buffer;
+ goto out;
}
switch (dp->prefix_requirement)
@@ -10073,7 +10082,8 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
if (ins.need_vex ? !ins.vex.prefix : !(ins.prefixes & PREFIX_DATA))
{
i386_dis_printf (&ins, dis_style_text, "(bad)");
- return ins.end_codep - priv.the_buffer;
+ ret = ins.end_codep - priv.the_buffer;
+ goto out;
}
ins.used_prefixes |= PREFIX_DATA;
/* Fall through. */
@@ -10100,7 +10110,8 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
&& !ins.vex.w != !(ins.used_prefixes & PREFIX_DATA)))
{
i386_dis_printf (&ins, dis_style_text, "(bad)");
- return ins.end_codep - priv.the_buffer;
+ ret = ins.end_codep - priv.the_buffer;
+ goto out;
}
break;
@@ -10156,7 +10167,8 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
if ((ins.codep - ins.start_codep) > MAX_CODE_LENGTH)
{
i386_dis_printf (&ins, dis_style_text, "(bad)");
- return MAX_CODE_LENGTH;
+ ret = MAX_CODE_LENGTH;
+ goto out;
}
/* Calculate the number of operands this instruction has. */
@@ -10264,7 +10276,10 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
info);
break;
}
- return ins.codep - priv.the_buffer;
+ ret = ins.codep - priv.the_buffer;
+ out:
+ info->private_data = NULL;
+ return ret;
}
/* Here for backwards compatibility. When gdb stops using
--
Alan Modra
Australia Development Lab, IBM
More information about the Binutils
mailing list