pepper-project / pequin

A system for verifying outsourced computations, and applying SNARKs. Simplified release of the main Pepper codebase.
Other
123 stars 46 forks source link

if .. else if .. or multiple if statements doesn't work with array types. #63

Open tarassh opened 5 years ago

tarassh commented 5 years ago

test_multiple_if.c

#include <stdint.h>

struct In { int16_t flag; };
struct Out { int dummy; };

void u32_to_u8arr2(uint8_t *src, uint32_t v, int stride) {
    src[0*stride] = v & 0xFF;
    src[1*stride] = (v >> 8) & 0xFF;
    src[2*stride] = (v >> 16) & 0xFF;
    src[3*stride] = (v >> 24) & 0xFF;
}

void pred16x16_128_dc(uint8_t *res, uint32_t dcsplat, int stride)
{
    u32_to_u8arr2(res+0, dcsplat, stride);
    u32_to_u8arr2(res+4, dcsplat, stride);
}

void print_luma(uint8_t *luma) {
    printf("%Zx %Zx %Zx %Zx", luma[0], luma[1], luma[2], luma[3]);
    printf("%Zx %Zx %Zx %Zx", luma[4], luma[5], luma[6], luma[7]);
}

void compute(struct In *input, struct Out *output){
    int prediction_mode = input->flag;
    uint8_t res[8];
    int scalar = 0;

    if (prediction_mode == 1) {
        pred16x16_128_dc(res, 0x10101010, 1);
        scalar = 0x10101010;
    }

    if (prediction_mode == 2) {
        pred16x16_128_dc(res, 0x20202020, 1);
        scalar = 0x20202020;
    }

    printf("Mode %Zd", prediction_mode);
    printf("Scalar %Zx", scalar);
    print_luma(res);
}

input.txt

1

prints out:

PRINTF in computation_p 1:
"Mode 1" 
PRINTF in computation_p 1:
"Scalar 10101010" 
PRINTF in computation_p 8:
"0 0 0 0" 
PRINTF in computation_p 8:
"0 0 0 0" 

should print

PRINTF in computation_p 1:
"Mode 1" 
PRINTF in computation_p 1:
"Scalar 10101010" 
PRINTF in computation_p 8:
"10 10 10 10" 
PRINTF in computation_p 8:
"10 10 10 10" 

if you comment out


   /* if (prediction_mode == 2) {
        pred16x16_128_dc(res, 0x20202020, STRIDE);
        scalar = 0x20202020;
    }*/

you will get expected result. Same thing with input value 2

Problem: We have incorrect behavior if the code has more than one if statement which operates with array types. Same thing if we replace it with if else if construction

As you can see, scalar types are ok.

tarassh commented 5 years ago

if I change array to uint8_t res[4] - works fine.

maxhowald commented 5 years ago

Thanks for reporting this! I can reproduce the behavior you describe and confirm it is a bug.

I'll try to investigate more deeply later this week, but I notice that inlining your u32_to_u8arr2 function causes the computation to produce the expected output:

#include <stdint.h>

struct In { int16_t flag; };
struct Out { int dummy; };

void pred16x16_128_dc(uint8_t *src, uint32_t v, int stride)
{
    src[0*stride] = v & 0xFF;
    src[1*stride] = (v >> 8) & 0xFF;
    src[2*stride] = (v >> 16) & 0xFF;
    src[3*stride] = (v >> 24) & 0xFF;

    src[4 + 0*stride] = v & 0xFF;
    src[4 + 1*stride] = (v >> 8) & 0xFF;
    src[4 + 2*stride] = (v >> 16) & 0xFF;
    src[4 + 3*stride] = (v >> 24) & 0xFF;
}

void print_luma(uint8_t *luma) {
    printf("%Zx %Zx %Zx %Zx", luma[0], luma[1], luma[2], luma[3]);
    printf("%Zx %Zx %Zx %Zx", luma[4], luma[5], luma[6], luma[7]);
}

void compute(struct In *input, struct Out *output){
    int prediction_mode = input->flag;
    uint8_t res[8];
    int scalar = 0;

    if (prediction_mode == 1) {
        pred16x16_128_dc(res, 0x10101010, 1);
        scalar = 0x10101010;
    }

    if (prediction_mode == 2) {
        pred16x16_128_dc(res, 0x20202020, 1);
        scalar = 0x20202020;
    }

    printf("Mode %Zd", prediction_mode);
    printf("Scalar %Zx", scalar);
    print_luma(res);
}

This points to the bug being somewhere in the frontend of the compiler, possibly related to some known bugs when passing arrays to function parameters. (#52 documents a bug when passing arrays of structs). So, it's possible that the C compiler is also not handling passing arrays as function pointers properly.

tarassh commented 5 years ago

@maxhowald thanks for the response. This code is small snippet of a bigger algorithm which has dozens of branches. Yes I see that inlining u32_to_u8arr2 fixes the problem. Would be nice at least to understand how to write the code to avoid unpredictable behavior.

maxhowald commented 5 years ago

Update: after a bit more experimenting, I notice that changing the line:

    u32_to_u8arr2(res+0, dcsplat, stride);

to

    u32_to_u8arr2(res, dcsplat, stride);

in your example code causes the output to behave as expected.

So we have a more minimal test case:

#include <stdint.h>

struct In { int16_t flag; };
struct Out { int dummy; };

void set_val(uint8_t *arr, uint8_t val, int idx) {
    arr[idx] = val;
}

void set_val_indirect(uint8_t *arr, uint8_t val, int idx) {
    set_val(arr + 0, val, idx);
}

void compute(struct In *input, struct Out *output){
    int mode = input->flag;
    uint8_t res[8];
    if (mode == 1) {
        set_val_indirect(res, 0x10, 0);
    }
    if (mode == 2) {
        set_val_indirect(res, 0x20, 0);
    }
    // res[0] should be 0x10 or 0x20 depending on the input
    printf("%Zx %Zx %Zx %Zx", res[0], res[1], res[2], res[3]);
}

Changing arr + 0 to arr or arr + 1 causes the output of the printf statement to be as expected.

For some reason, the arr + 0 access is being miscompiled, but only in nested / conditional function calls.

I think this points to a bug somewhere in the frontend C compiler, possibly here or here.

I'll try to investigate this a bit more soon, but perhaps you can use the above as a workaround in the meantime.