RFC: Prevent disassembly beyond symbolic boundaries
Commit Message
Hi Guys,
Currently objdump will disassemble beyond a symbolic boundary if it
needs extra bytes to decode an instruction. For example (with x86):
.file "foo.c"
.text
.globl foo
.type foo, @function
foo:
.byte 0x24
.byte 0x2f
.byte 0x83
.size foo, .-foo
.globl bar
.type bar, @function
bar:
.byte 0x0f
.byte 0xba
.byte 0xe2
.byte 0x03
.size bar, .-bar
This will disassemble as:
0000000000000000 <foo>:
0: 24 2f and $0x2f,%al
2: 83 0f ba orl $0xffffffba,(%rdi)
0000000000000003 <bar>:
3: 0f ba e2 03 bt $0x3,%edx
Note how the instruction decoded at address 0x2 has stolen two bytes
from "foo", but these bytes are also decoded (correctly this time) as
part of the first instruction of foo.
I have a patch (attached) which changes this behaviour, so that the
disassembly would be:
0: 24 2f and $0x2f,%al
2: 83 .byte 0x83
00000003 <bar>:
3: 0f ba e2 03 bt $0x3,%edx
The patch works by adding an extra field to the end of the
disassemble_info structure, setting it inside objdump's
disassemble_bytes function and then checking it in the opcode
library's buffer_read_memory function. This means that other users of
the opcodes library will not be affected by the change.
The patch makes sure to only enable this feature when disassembling
code sections, data sections are unaffected. I have omitted adding a
new test for the feature since the gas/i386/x86_64-opcode-inval test
already covers this.
What do people think ? To me this seems like a good idea, but I
willing to consider alternative suggestions if people have them.
Cheers
Nick
PS. The patch only affects objdump. So if you disassemble a binary
using GDB for example, the old behaviour will still be seen. Changing
GDB's behaviour is possible, although it would be quite a big
job as there are lots of different places where the disassembler part
of the opcodes library is called and memory is read.
Comments
Nick,
> Currently objdump will disassemble beyond a symbolic boundary if it
> needs extra bytes to decode an instruction. For example (with x86):
>
> .file "foo.c"
> .text
> .globl foo
> .type foo, @function
> foo:
> .byte 0x24
> .byte 0x2f
> .byte 0x83
> .size foo, .-foo
>
> .globl bar
> .type bar, @function
> bar:
> .byte 0x0f
> .byte 0xba
> .byte 0xe2
> .byte 0x03
> .size bar, .-bar
>
> This will disassemble as:
>
> 0000000000000000 <foo>:
> 0: 24 2f and $0x2f,%al
> 2: 83 0f ba orl $0xffffffba,(%rdi)
>
> 0000000000000003 <bar>:
> 3: 0f ba e2 03 bt $0x3,%edx
>
> Note how the instruction decoded at address 0x2 has stolen two bytes
> from "foo", but these bytes are also decoded (correctly this time) as
> part of the first instruction of foo.
>
> I have a patch (attached) which changes this behaviour, so that the
> disassembly would be:
>
> 0: 24 2f and $0x2f,%al
> 2: 83 .byte 0x83
>
> 00000003 <bar>:
> 3: 0f ba e2 03 bt $0x3,%edx
[…]
> What do people think ? To me this seems like a good idea, but I
> willing to consider alternative suggestions if people have them.
I am curious. Why do you think it was a problem ?
Even if there is a symbol in the middle of an instruction, I’d like
to understand what the processor will execute. Before the proposed
change, it was possible, but after it isn’t easy anymore.
(But I agree I never met this issue. I am just curious here).
Tristan.
Hi Tristan,
>> This will disassemble as:
>>
>> 0000000000000000 <foo>:
>> 0: 24 2f and $0x2f,%al
>> 2: 83 0f ba orl $0xffffffba,(%rdi)
>>
>> 0000000000000003 <bar>:
>> 3: 0f ba e2 03 bt $0x3,%edx
>>
>> Note how the instruction decoded at address 0x2 has stolen two bytes
>> from "foo", but these bytes are also decoded (correctly this time) as
>> part of the first instruction of foo.
> I am curious. Why do you think it was a problem ?
Strangely enough, this actually causes regressions with the perf tool's
testsuite:
https://bugzilla.redhat.com/show_bug.cgi?id=1054767
What happens is that perf test 21 runs objdump on a binary, *parses*
this output and compares that to the actual bytes in the binary.
Because of the overrun feature shown above you actually get more bytes
displayed in objdump's output than actually exist in the binary and so
the perf test fails.
> Even if there is a symbol in the middle of an instruction, I’d like
> to understand what the processor will execute.
Except that even the current the displayed disassembly is not what the
processor would execute. In the example above the processor would
execute the ORL instruction starting at address 0x2. but it would not
continue on to execute the BT instruction at address 0x3. Instead it
would start decoding from address 0x5, whatever instruction that might be...
> Before the proposed
> change, it was possible, but after it isn’t easy anymore.
True - but this only matters if the processor would execute from that
piece of memory. What if the byte(s) are actually data ? (eg a
constant pool). Then it would make more sense to display the bytes as
just byte values.
The point being that if there is a symbol that is in the middle of an
instruction then something hinky is going on. Either the symbol is
misplaced or the instruction is not really an instruction or else an
assembly programmer is being extra super clever and hiding data inside
instructions.
How about a tweak to the patch then ? What if the -D option
(disassemble all) disables this feature, and so the disassembled
instruction is displayed as before, whilst the -d option (disassemble
code) leaves it enabled. Then if you want to see bytes as instructions
you can use the -D option (possibly combined with -j), but if you want
to see a more likely, only real instructions disassembled version, then
use the -d option. (Obviously the patch would need to be extended with
an update to the documentation too).
Cheers
Nick
> On 19 Jun 2015, at 13:41, Nicholas Clifton <nickc@redhat.com> wrote:
>
> Hi Tristan,
>
>>> This will disassemble as:
>>>
>>> 0000000000000000 <foo>:
>>> 0: 24 2f and $0x2f,%al
>>> 2: 83 0f ba orl $0xffffffba,(%rdi)
>>>
>>> 0000000000000003 <bar>:
>>> 3: 0f ba e2 03 bt $0x3,%edx
>>>
>>> Note how the instruction decoded at address 0x2 has stolen two bytes
>>> from "foo", but these bytes are also decoded (correctly this time) as
>>> part of the first instruction of foo.
>
>> I am curious. Why do you think it was a problem ?
>
> Strangely enough, this actually causes regressions with the perf tool's testsuite:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1054767
>
> What happens is that perf test 21 runs objdump on a binary, *parses* this output and compares that to the actual bytes in the binary. Because of the overrun feature shown above you actually get more bytes displayed in objdump's output than actually exist in the binary and so the perf test fails.
I can argue that this is an issue in the perf tool. After all, the objdump output is clear that pc goes backward.
>> Even if there is a symbol in the middle of an instruction, I’d like
>> to understand what the processor will execute.
>
> Except that even the current the displayed disassembly is not what the processor would execute. In the example above the processor would execute the ORL instruction starting at address 0x2. but it would not continue on to execute the BT instruction at address 0x3. Instead it would start decoding from address 0x5, whatever instruction that might be…
That’s a very good point!
>> Before the proposed
>> change, it was possible, but after it isn’t easy anymore.
>
> True - but this only matters if the processor would execute from that piece of memory. What if the byte(s) are actually data ? (eg a constant pool). Then it would make more sense to display the bytes as just byte values.
OTOH, if this is a constant pool it is possible that objdump is already of out track for a while.
> The point being that if there is a symbol that is in the middle of an instruction then something hinky is going on. Either the symbol is misplaced or the instruction is not really an instruction or else an assembly programmer is being extra super clever and hiding data inside instructions.
Yes. My scenario was setting a label on a known part of an instruction like the offset in a call instruction you want to patch later.
But I agree that before and after your proposed change, objdump output is not very readable.
> How about a tweak to the patch then ? What if the -D option (disassemble all) disables this feature, and so the disassembled instruction is displayed as before, whilst the -d option (disassemble code) leaves it enabled. Then if you want to see bytes as instructions you can use the -D option (possibly combined with -j), but if you want to see a more likely, only real instructions disassembled version, then use the -d option. (Obviously the patch would need to be extended with an update to the documentation too).
It’s up to you. I don’t insist at all on modifying your change, I was just curious about the motivation.
And the scenario I had in mind is not really affected by your proposal.
Tristan.
Hi Guys,
I have gone ahead and applied the patch, with the addition of
disabling the new behaviour when -D is used.
Cheers
Nick
@@ -212,6 +212,14 @@ typedef struct disassemble_info
/* Command line options specific to the target disassembler. */
char * disassembler_options;
+ /* If non-zero then try not disassemble beyond this address, even if
+ there are values left in the buffer. This address is the address
+ of the nearest symbol forwards from the start of the disassembly,
+ and it is assumed that it lies on the boundary between instructions.
+ If an instruction spans this address then this is an error in the
+ file being disassembled. */
+ bfd_vma stop_vma;
+
} disassemble_info;
@@ -1685,7 +1685,16 @@ disassemble_bytes (struct disassemble_info * inf,
}
}
+ if ((section->flags & (SEC_CODE | SEC_HAS_CONTENTS))
+ == (SEC_CODE | SEC_HAS_CONTENTS))
+ /* Set a stop_vma so that the disassembler will not read
+ beyond the next symbol. We assume that symbols appear on
+ the boundaries between instructions. We only do this when
+ disassembling code of course. */
+ inf->stop_vma = section->vma + stop_offset;
+
octets = (*disassemble_fn) (section->vma + addr_offset, inf);
+ inf->stop_vma = 0;
inf->fprintf_func = (fprintf_ftype) fprintf;
inf->stream = stdout;
if (insn_width == 0 && inf->bytes_per_line != 0)
@@ -38,7 +38,9 @@ buffer_read_memory (bfd_vma memaddr,
if (memaddr < info->buffer_vma
|| memaddr - info->buffer_vma > max_addr_offset
- || memaddr - info->buffer_vma + end_addr_offset > max_addr_offset)
+ || memaddr - info->buffer_vma + end_addr_offset > max_addr_offset
+ || (info->stop_vma && (memaddr >= info->stop_vma
+ || memaddr + end_addr_offset > info->stop_vma)))
/* Out of bounds. Use EIO because GDB uses it. */
return EIO;
memcpy (myaddr, info->buffer + octets, length);
@@ -4664,7 +4664,7 @@ _print_insn_bfin (bfd_vma pc, disassemble_info *outf)
return -1;
priv->iw0 = iw0;
- if ((iw0 & 0xc000) == 0xc000)
+ if (((iw0 & 0xc000) == 0xc000) && ((iw0 & 0xff00) != 0xf800))
{
/* 32-bit insn. */
if (ifetch (pc + 2, outf, &iw1))
@@ -88,9 +88,8 @@ static const char *crname[] = {
static const unsigned isiz[] = { 2, 0, 1, 0 };
int
-print_insn_mcore (memaddr, info)
- bfd_vma memaddr;
- struct disassemble_info *info;
+print_insn_mcore (bfd_vma memaddr,
+ struct disassemble_info *info)
{
unsigned char ibytes[4];
fprintf_ftype print_func = info->fprintf_func;
@@ -233,6 +232,9 @@ print_insn_mcore (memaddr, info)
val = (memaddr + 2 + ((inst & 0xFF) << 2)) & 0xFFFFFFFC;
+ /* We are not reading an instruction, so allow
+ reads to extend beyond the next symbol. */
+ info->stop_vma = 0;
status = info->read_memory_func (val, ibytes, 4, info);
if (status != 0)
{
@@ -263,6 +265,9 @@ print_insn_mcore (memaddr, info)
val = (memaddr + 2 + ((inst & 0xFF) << 2)) & 0xFFFFFFFC;
+ /* We are not reading an instruction, so allow
+ reads to extend beyond the next symbol. */
+ info->stop_vma = 0;
status = info->read_memory_func (val, ibytes, 4, info);
if (status != 0)
{
@@ -905,6 +905,8 @@ print_insn_sh (bfd_vma memaddr, struct disassemble_info *info)
size = 2;
else
size = 4;
+ /* Not reading an instruction - disable stop_vma. */
+ info->stop_vma = 0;
status = info->read_memory_func (disp_pc_addr, bytes, size, info);
if (status == 0)
{
@@ -249,6 +249,9 @@ print_insn_tic6x (bfd_vma addr, struct disassemble_info *info)
fp_offset = addr & 0x1f;
fp_addr = addr - fp_offset;
+ /* Read in a block of instructions. Since there might be a
+ symbol in the middle of this block, disable stop_vma. */
+ info->stop_vma = 0;
status = info->read_memory_func (fp_addr, fp, 32, info);
if (status)
{
@@ -402,7 +402,8 @@ print_insn_vax (bfd_vma memaddr, disassemble_info *info)
argp = NULL;
/* Check if the info buffer has more than one byte left since
the last opcode might be a single byte with no argument data. */
- if (info->buffer_length - (memaddr - info->buffer_vma) > 1)
+ if (info->buffer_length - (memaddr - info->buffer_vma) > 1
+ && (info->stop_vma == 0 || memaddr < (info->stop_vma - 1)))
{
FETCH_DATA (info, buffer + 2);
}
@@ -5,13 +5,13 @@
Disassembly of section .text:
0+000 <.*>.*(615c|5c61).*
-0+002 <foo> e3a00000 mov r0, #0
-0+006 <foo\+0x4> e3a00000 mov r0, #0
-0+00a <foo\+0x8> e3a00000 mov r0, #0
-0+00e <foo\+0xc> e3a00001 mov r0, #1
-0+012 <foo\+0x10> e3a00001 mov r0, #1
-0+016 <foo\+0x14> e3a00001 mov r0, #1
-0+01a <foo\+0x18> e3a00002 mov r0, #2
-0+01e <foo\+0x1c> e3a00002 mov r0, #2
-0+022 <foo\+0x20> e3a00002 mov r0, #2
+0+004 <foo> e3a00000 mov r0, #0
+0+008 <foo\+0x4> e3a00000 mov r0, #0
+0+00c <foo\+0x8> e3a00000 mov r0, #0
+0+010 <foo\+0xc> e3a00001 mov r0, #1
+0+014 <foo\+0x10> e3a00001 mov r0, #1
+0+018 <foo\+0x14> e3a00001 mov r0, #1
+0+01c <foo\+0x18> e3a00002 mov r0, #2
+0+020 <foo\+0x1c> e3a00002 mov r0, #2
+0+024 <foo\+0x20> e3a00002 mov r0, #2
#...
@@ -6,9 +6,10 @@
mov r0, #\@ @comment
.endm
-.byte '\\
-.byte '\a
-
+ .byte '\\
+ .byte '\a
+ .byte 0
+ .byte 0
foo:
bar
bar
@@ -12,26 +12,26 @@ Disassembly of section .text:
0+1 <aad0>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 0a d5 or dl,ch
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+3 <aad1>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 02 d4 add dl,ah
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+5 <aam0>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 0a d4 or dl,ah
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+7 <aam1>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 02 3f add bh,BYTE PTR \[rdi\]
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+9 <aas>:
[ ]*[a-f0-9]+: 3f \(bad\)
0+a <bound>:
-[ ]*[a-f0-9]+: 62 \(bad\)
-[ ]*[a-f0-9]+: 10 27 adc BYTE PTR \[rdi\],ah
+[ ]*[a-f0-9]+: 62 .byte 0x62
+[ ]*[a-f0-9]+: 10 .byte 0x10
0+c <daa>:
[ ]*[a-f0-9]+: 27 \(bad\)
@@ -12,26 +12,26 @@ Disassembly of section .text:
0+1 <aad0>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 0a d5 or %ch,%dl
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+3 <aad1>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 02 d4 add %ah,%dl
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+5 <aam0>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 0a d4 or %ah,%dl
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+7 <aam1>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 02 3f add \(%rdi\),%bh
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+9 <aas>:
[ ]*[a-f0-9]+: 3f \(bad\)
0+a <bound>:
-[ ]*[a-f0-9]+: 62 \(bad\)
-[ ]*[a-f0-9]+: 10 27 adc %ah,\(%rdi\)
+[ ]*[a-f0-9]+: 62 .byte 0x62
+[ ]*[a-f0-9]+: 10 .byte 0x10
0+c <daa>:
[ ]*[a-f0-9]+: 27 \(bad\)
@@ -12,26 +12,26 @@ Disassembly of section .text:
0+1 <aad0>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 0a d5 or dl,ch
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+3 <aad1>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 02 d4 add dl,ah
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+5 <aam0>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 0a d4 or dl,ah
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+7 <aam1>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 02 3f add bh,BYTE PTR \[rdi\]
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+9 <aas>:
[ ]*[a-f0-9]+: 3f \(bad\)
0+a <bound>:
-[ ]*[a-f0-9]+: 62 \(bad\)
-[ ]*[a-f0-9]+: 10 27 adc BYTE PTR \[rdi\],ah
+[ ]*[a-f0-9]+: 62 .byte 0x62
+[ ]*[a-f0-9]+: 10 .byte 0x10
0+c <daa>:
[ ]*[a-f0-9]+: 27 \(bad\)
@@ -11,26 +11,26 @@ Disassembly of section .text:
0+1 <aad0>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 0a d5 or %ch,%dl
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+3 <aad1>:
[ ]*[a-f0-9]+: d5 \(bad\)
-[ ]*[a-f0-9]+: 02 d4 add %ah,%dl
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+5 <aam0>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 0a d4 or %ah,%dl
+[ ]*[a-f0-9]+: 0a .byte 0xa
0+7 <aam1>:
[ ]*[a-f0-9]+: d4 \(bad\)
-[ ]*[a-f0-9]+: 02 3f add \(%rdi\),%bh
+[ ]*[a-f0-9]+: 02 .byte 0x2
0+9 <aas>:
[ ]*[a-f0-9]+: 3f \(bad\)
0+a <bound>:
-[ ]*[a-f0-9]+: 62 \(bad\)
-[ ]*[a-f0-9]+: 10 27 adc %ah,\(%rdi\)
+[ ]*[a-f0-9]+: 62 .byte 0x62
+[ ]*[a-f0-9]+: 10 .byte 0x10
0+c <daa>:
[ ]*[a-f0-9]+: 27 \(bad\)