From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x733.google.com (mail-qk1-x733.google.com [IPv6:2607:f8b0:4864:20::733]) by sourceware.org (Postfix) with ESMTPS id 940553858032 for ; Mon, 15 Aug 2022 14:25:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 940553858032 Received: by mail-qk1-x733.google.com with SMTP id m5so5621551qkk.1 for ; Mon, 15 Aug 2022 07:25:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=3XOn6wCJrtNBTnNPl3Z0XyTDChpJtGmKA1q12URu/eE=; b=g2qLLkCDOXyYgMMDt2dqACHppzkObqXulvh6Y9n7In7xGG/Zcnn/c0p0IUxQzuJHTZ +h6FajN6dX0sDxEMMTButnwAauym4i9YOiRbJ/SEEJyH9bltcDr9cn19YdWKLJ9KCubs nC+fLbyyp7akr2XO7rj1k6KJRFc+ItXUJeogA3d9EeLNXLMvrEeqDte1CwoLc1XBzGzF 09oljQquKfX4yVMsXZzIxjgHB1H6gSy1DneMxypQfuJ8NJC+0fqMxNArT3onCrc0F5xu 468AwyiD1CPE2qiUavqDpCdDaKCorA5wFwjptbM8fz38IwJOAP+FN88kd9gVP2ijlFeR Z23Q== X-Gm-Message-State: ACgBeo2xi2lRYsEX/x9fxHDUaY2f4ldDgC3xJ9ClnS8ak/zuNH8xWXGl 8ySi5M3FTo5UFhJuK8tqq3ma04Xyj1ozvuq0jOA= X-Google-Smtp-Source: AA6agR7UDxMqLFY16MurjdCVPucpZabsXXNOXbFq8YEC8dTrP0niGDdWhbqkK94HdXTMGBNWo1+P7E/nIcYNKQkPj4E= X-Received: by 2002:a05:620a:2699:b0:6b8:c299:23c2 with SMTP id c25-20020a05620a269900b006b8c29923c2mr11219103qkp.768.1660573549843; Mon, 15 Aug 2022 07:25:49 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "H.J. Lu" Date: Mon, 15 Aug 2022 07:25:14 -0700 Message-ID: Subject: Re: [PATCH] x86: avoid i386_dis_printf()'s staging area for potentially long strings To: Jan Beulich Cc: Binutils , Alan Modra Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3018.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, 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 X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2022 14:25:56 -0000 On Mon, Aug 15, 2022 at 4:17 AM Jan Beulich wrote: > > PR binutils/29483 > > When print_insn() processes op_txt[], it may pass strings into > i386_dis_printf() which staging_area[] cannot fit; this was observed for > an invalid form of VPSCATTERDD (both broadcast and zeroing-masking bits > set). Rather than arbitrarily enlarging that local array, avoid its use > altogether when the format string is simply "%s". This merely requires > two local variables to have their type constified. > > While limiting the scope of "res" it became apparent that > - no caller cares about the function's return value, > - the comment about the return value was wrong, > - a particular positive return value would have been meaningless to the > caller. > Therefore convert the function to return "void" at the same time. > --- > An alternative to the special casing would be to introduce something > like i386_dis_puts(), then to be used by all call sites which currently > pass "%s" or format strings without any format characters at all (plus, > of course, i386_dis_printf() itself). > > --- a/opcodes/i386-dis.c > +++ b/opcodes/i386-dis.c > @@ -9264,31 +9264,40 @@ oappend_register (instr_info *ins, const > STYLE is the default style to use in the fprintf_styled_func calls, > however, FMT might include embedded style markers (see oappend_style), > these embedded markers are not printed, but instead change the style > - used in the next fprintf_styled_func call. > + used in the next fprintf_styled_func call. */ > > - Return non-zero to indicate the print call was a success. */ > - > -static int ATTRIBUTE_PRINTF_3 > +static void ATTRIBUTE_PRINTF_3 > i386_dis_printf (instr_info *ins, enum disassembler_style style, > const char *fmt, ...) > { > va_list ap; > enum disassembler_style curr_style = style; > - char *start, *curr; > + const char *start, *curr; > char staging_area[100]; > - int res; > > va_start (ap, fmt); > - res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); > - va_end (ap); > + /* In particular print_insn()'s processing of op_txt[] can hand rather long > + strings here. Bypass vsnprintf() in such cases to avoid capacity issues > + with the staging area. */ > + if (strcmp (fmt, "%s")) > + { > + int res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); > > - if (res < 0) > - return res; > + va_end (ap); > > - if ((size_t) res >= sizeof (staging_area)) > - abort (); > + if (res < 0) > + return; > > - start = curr = staging_area; > + if ((size_t) res >= sizeof (staging_area)) > + abort (); > + > + start = curr = staging_area; > + } > + else > + { > + start = curr = va_arg (ap, const char *); > + va_end (ap); > + } > > do > { > @@ -9303,10 +9312,7 @@ i386_dis_printf (instr_info *ins, enum d > curr_style, > "%.*s", len, start); > if (n < 0) > - { > - res = n; > - break; > - } > + break; > > if (*curr == '\0') > break; > @@ -9340,8 +9346,6 @@ i386_dis_printf (instr_info *ins, enum d > ++curr; > } > while (true); > - > - return res; > } > > static int Please include a testcase to verify that the issue is fixed. Thanks. -- H.J.