GrammaTech / gtirb-rewriting

Python API for rewriting GTIRB files
GNU General Public License v3.0
16 stars 3 forks source link

Alignment issues with instructions that use 128bit registers and data #8

Open avncharlie opened 1 year ago

avncharlie commented 1 year ago

Hello,

I ran into an issue inserting a patch with data into a binary that uses instructions operating on 128bit registers, specifically this instruction: xorpd xmm2, XMMWORD PTR [rip+0x69aed]. This was as the memory operand needed to be 16 byte aligned and the data I had inserted with my patch had moved this alignment. After I ensured my patch was 16-byte aligned the binary rewrote successfully. I'm not sure if this is actually an issue or maybe more something to be aware of when writing patches but I thought I might make an issue for anyone else with this problem.

jranieri-grammatech commented 1 year ago

It depends on what exactly your patch was doing. Could you post an example transform that reproduces this?

avncharlie commented 1 year ago

Sure, here's a minimal example.

Below is the c program used.

#include <stdio.h>
#include <immintrin.h> 

int main() {
    __m128d a = _mm_set_pd(5.0, 3.0);
    __m128d b = _mm_set_pd(2.5, 4.5);
    __m128d result;

    result = _mm_and_pd(a, b);

    printf("Result: %lf %lf\n", result[1], result[0]);
    return 0;
}

To compile: gcc -O1 -o test test.c My gcc version is gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

Using -01 causes the compiler to generate instructions that rely on 16 byte aligned data, namely movapd and andpd in this example.

Here is the patch:

context.register_insert_function(
    'test_alignment_func',
    Patch.from_function(lambda _: '''
        nop

        .rodata
        .TEST_STRING:
            .string "TEST STRING"
    ''', Constraints(x86_syntax=X86Syntax.INTEL))
)

When running the original program:

$ ./test
Result: 2.500000 2.000000

When running the instrumented program:

$ ./test_instrumented
[1]    229030 segmentation fault  ./test_instrumented

Running under gdb shows the instrumented program crashed while attempting to call movapd on a memory address that isn't 16 byte aligned:

image

Here is a comparison of the read only memory areas of the original and instrumented binary (dumped using gdb), it appears that the string has been added at the start of this memory area and misaligned everything after it:

image

Adding some space to the patch to ensure the added data is 16 byte aligned fixes the issue:

context.register_insert_function(
    'test_alignment_func',
    Patch.from_function(lambda _: '''
    nop

    .rodata
    .TEST_STRING:
        .string "TEST STRING"
        .space 4
    ''', Constraints(x86_syntax=X86Syntax.INTEL))
)
$ ./test_instrumented
Result: 2.500000 2.000000
jranieri-grammatech commented 1 year ago

I've reproduced this and it probably could be considered a bug in ddisasm. Until that gets addressed, a potential workaround is to:

(or some other variation of using the instructions to infer which data blocks must be 16-byte aligned)

avncharlie commented 5 months ago

For anyone else coming across this, binaries built with MSVC will crash in initialisation code if data added by gtirb-rewriting isn't aligned.