Bug 29483

Summary: abort at i386-dis.c:9289
Product: binutils Reporter: Alan Modra <amodra>
Component: binutilsAssignee: Not yet assigned to anyone <unassigned>
Severity: normal CC: hjl.tools, jbeulich
Priority: P2    
Version: 2.40   
Target Milestone: 2.40   
Host: Target:
Build: Last reconfirmed:

Description Alan Modra 2022-08-13 10:39:36 UTC
cat > xxx.s <<EOF
 .byte	0x65,0x62,0x62,0x7d,0x97,0xa0,0x94,0xff,0x20,0x20,0x20,0xae
gas/as-new -o xxx.o xxx.s
binutils/objdump -d xxx.o

The abort is this one:

  if ((size_t) res >= sizeof (staging_area))
    abort ();

Doubling the buffer size fixes this particular test case.
Comment 1 Sourceware Commits 2022-08-16 16:41:11 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:


commit 9096fc28c62741bfb7962eb5dfdee28a7b1d1345
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Aug 16 08:25:49 2022 -0700

    When displaying operands, invalid opcodes may overflow operand buffer
    due to additional styling characters.  Each style is encoded with 3
    bytes.  Define MAX_OPERAND_BUFFER_SIZE for operand buffer size and
    increase it from 100 bytes to 128 bytes to accommodate 9 sets of styles
    in an operand.
            PR binutils/29483
            * testsuite/gas/i386/i386.exp: Run pr29483.
            * testsuite/gas/i386/pr29483.d: New file.
            * testsuite/gas/i386/pr29483.s: Likewise.
            PR binutils/29483
            * i386-dis.c (MAX_OPERAND_BUFFER_SIZE): New.
            (obuf): Replace 100 with MAX_OPERAND_BUFFER_SIZE.
            (staging_area): Likewise.
            (op_out): Likewise.
Comment 2 H.J. Lu 2022-08-16 16:41:45 UTC
Fixed for 2.40.
Comment 3 Sourceware Commits 2022-09-12 06:20:07 UTC
The master branch has been updated by Jan Beulich <jbeulich@sourceware.org>:


commit ac3fe48fd61a92d03c66152038df4fc184bf5fcd
Author: Jan Beulich <jbeulich@suse.com>
Date:   Mon Sep 12 08:19:55 2022 +0200

    x86: avoid i386_dis_printf()'s staging area for a fair part of output
    While PR binutils/29483 has now been addressed differently, this
    originally proposed change still has its merits: Avoiding vsnprintf()
    for typically far more than half of the overall output results in a 2-3%
    performance gain in my testing (with debug builds of objdump, libbfd,
    and libopcodes).
    With that part of output no longer using staging_area[], the array also
    doesn't need to be quite as large anymore (the largest presently used
    size is 27, from "64-bit address is disabled").
    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
    Therefore convert the function to return "void" at the same time.