scrcpy icon indicating copy to clipboard operation
scrcpy copied to clipboard

remove compound statements

Open Wirtos opened this issue 4 years ago • 6 comments

Wirtos avatar Jun 19 '21 15:06 Wirtos

So after that I was pleased to be able use this feature in scrcpy

As for template data structures, things like https://github.com/rxi/vec or https://github.com/Wirtos/cmap worked just fine for me without the need for any compiler extensions, but since I've got my fork building for any general compiler/platform I don't want to be too invasive.

Wirtos avatar Jun 20 '21 10:06 Wirtos

As for template data structures, things like https://github.com/rxi/vec

Thank you for the reference. After a quick look, the impl does not seem correct in term of pointer aliasing (in theory, a T** should not be cast to a char **). Cf discussion.

rom1v avatar Jun 20 '21 11:06 rom1v

(in theory, a T** should not be cast to a char **).

Indeed, my local version uses void** and iirc this exact impl has a bug in insert macro, which always returns success, even if underlying function fails. As for subprocess why not use https://github.com/sheredom/subprocess.h? This one has proper parsing on windows.

Wirtos avatar Jun 20 '21 14:06 Wirtos

Indeed, my local version uses void**

But even with void ** I think it is a strict aliasing violation: it aliases a T ** with a void **.

It would be great to produce a real sample to demonstrate the issue, but here is a sample with a simpler aliasing violation (struct A* vs struct B*):

#include <stdio.h>

struct A { int x; };
struct B { int x; };

void f(struct A *a, struct B *b) {
    a->x = 2;
    b->x = 3;
    printf("%i\n", a->x);
}

int main() {
    struct A a = { .x = 1 };
    f(&a, (struct B *) &a);
    return 0;
}
$ gcc a.c && ./a.out
3
$ gcc -O2 a.c && ./a.out
2

rom1v avatar Jun 20 '21 14:06 rom1v

I can adapt the sample to highlight the strict aliasing violation with a char **:

#include <stdio.h>

static int v0 = 100;
static char v1 = 42;

void f(int **a, char **b) {
    *a = &v0;
    *b = &v1;
    printf("%i\n", **a);
}

int main() {
    int data = 5;
    int *a = &data;
    f(&a, (char **) &a);
    return 0;
}
$ gcc a.c && ./a.out
42
$ gcc -O2 a.c && ./a.out
100

However, I cannot expose the issue with a void ** (which does not mean that it would be correct).

EDIT: but I can with a const void ** (a non-const pointer to a const void *):

#include <stdio.h>

static const int v0 = 100;
static const char v1 = 42;

void f(const int **a, const void **b) {
    *a = &v0;
    *b = &v1;
    printf("%i\n", **a);
}

int main() {
    const int data = 5;
    const int *a = &data;
    f(&a, (const void **) &a);
    return 0;
}
$ gcc a.c && ./a.out
174662954
$ gcc -O2 a.c && ./a.out
42

EDIT2: the sample is incorrect, the results are identical with -fno-strict-aliasing, so it does not prove anything.

Fixed sample to show the strict aliasing issue for the char ** version (could not reproduce for void ** or const void **):

#include <stdio.h>

static const int v0 = 100;
static const char v1 = 42;

void f(const int **a, char **b) {
    *a = &v0;
    *b = NULL;
    if (*a)
        // *b == *a (NULL)
        printf("Should never be printed: %i\n", **a);
}

int main() {
    const int data = 5;
    const int *a = &data;
    f(&a, (char **) &a);
    return 0;
}

EDIT3: I asked on stackoverflow: https://stackoverflow.com/questions/69896206/is-void-an-exception-to-strict-aliasing-rules#69896206

rom1v avatar Jun 20 '21 14:06 rom1v

However, I cannot expose the issue with a void ** (which does not mean that it would be correct).

Iirc it's UB, valid impl would require an intermediate void * variable, having a void ** cast simply silents the warning in compilers (which is still invalid)

Wirtos avatar Jun 20 '21 15:06 Wirtos