From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by sourceware.org (Postfix) with ESMTPS id 3B0DA3858D28 for ; Mon, 24 Apr 2023 10:24:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3B0DA3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x1032.google.com with SMTP id 98e67ed59e1d1-24b3451b2fcso3137098a91.3 for ; Mon, 24 Apr 2023 03:24:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682331862; x=1684923862; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=qBPr+vfeK10Rd55tq31U0W9S9ufxYilxIXvGHaO1+Zc=; b=iniKtJM14YX7YkigeAEsKgqpUivSokVklwu1aAmGXozDX4RK7ClTLtTCpRiN37uRET Fo6wazISaZvRt4HOc3kti+muUpL1VCTT8nyBs2vICo8jvK7z9cM2AF8j2IwW2XOWWyQH hHnOYkZGqx/5599Ht2bjqS/jQFL6tk7UDCBngT9Q8VE+4HntIxy2QK+cqNwssnWlpOA/ BUmoetDLDuEMEm+UKxa5GOJs2waD7HpI4ldMJza2m+y/+clLIZIE1a2QlTv8QGOgLSQO +nijRcf0eVy+A5DCSv7FUHLFsiFgdk1EWEiJye0g6Rhc/8yRll1XH1dz1EwdXP5biGBR D7rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682331862; x=1684923862; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=qBPr+vfeK10Rd55tq31U0W9S9ufxYilxIXvGHaO1+Zc=; b=V5Mmd8LaIkhx+3s4EMVMphik6U1wTUiSWEBHIVR4tFGJSlXTT6TDk6hjdzJCVXxfPS aBymj6Md1dV5eu4oQx7e72GASpxixlZkkv1K8emWIpmrKk/7lGklJMi28OedWf3VBrkf Fydr+geL+LNtgTQw1cMHPAuGwWJOOB8XGL9ZpUlydBdBHsvCQGrFG7Lt1KtQbJkZFSyM KMhDRId0KHoSuijoY6FPuRV52jLQgCCEh6lJX+9Q0Nd92ihrKg6WA6/kftEn7N82XZA0 OD9kYdaT6Cf5uuHwbNpu7Pf7suL+w6Ienmj+hRj0vZR8z7De/9EAVj09bY/h4mMTKm8E waYg== X-Gm-Message-State: AAQBX9dSWIdJ/zMEyyWLKsEKVaJGMPFur8/TDeilOF00kICuBLfMv6q1 Hw6kBFJEp+i9+jQkJFQRCFuepyzscpM= X-Google-Smtp-Source: AKy350ae4vRwvTqPXY9EXJ3oYdiOHsawFVQ0FWuYrZ9rt4RfnGScCizw6dWlLPvO+0syXKIswuan2w== X-Received: by 2002:a17:90b:1191:b0:247:9422:62b4 with SMTP id gk17-20020a17090b119100b00247942262b4mr13499555pjb.14.1682331862083; Mon, 24 Apr 2023 03:24:22 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id ix3-20020a170902f80300b0019a96a6543esm6318535plb.184.2023.04.24.03.24.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Apr 2023 03:24:21 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D8F951140749; Mon, 24 Apr 2023 19:54:18 +0930 (ACST) Date: Mon, 24 Apr 2023 19:54:18 +0930 From: Alan Modra To: Jan Beulich Cc: Binutils , "H.J. Lu" Subject: Re: [PATCH 1/3] x86: work around compiler diagnosing dangling pointer Message-ID: References: <61597ebf-cc5e-2029-6520-31f7adfeea68@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-3035.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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