Unexpected -Wuninitialized with asm goto with outputs

[nathanchance]

The kernel test robot reported an issue with a PowerPC patch taking advantage of asm goto with outputs. This appears to be a false positive, as the variable is only used on the fallthrough path. A simplified reproducer:

$ cat test.c
static inline int copy_inst_from_kernel_nofault(unsigned int *inst,
                                                unsigned int *src)
{
        unsigned int val;

        asm goto("1: lwz %U1%X1 %0, %1   # get_user\n"
                 ".section __ex_table,\"a\";"
                 ".balign 4;"
                 ".long (1b) - . ;"
                 ".long (%l2) - . ;"
                 ".previous"
                 : "=r" (*(unsigned int *)(&val))
                 : "m<>" (*(unsigned int *)(src))
                 :
                 : Efault);

        *inst = val;
        return 0;

Efault:
        return -14; /* -EFAULT */
}

$ clang --target=powerpc-linux-gnu -Wuninitialized -fsyntax-only test.c
test.c:17:10: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
        *inst = val;
                ^~~
test.c:4:18: note: initialize the variable 'val' to silence this warning
        unsigned int val;
                        ^
                         = 0
1 warning generated.

[nickdesaulniers]

@gwelymernans mentioned that the casting may be messing something up. It looks to me like an r-value is being used as an output, which looks to me like -fheinous-gnu-extensions.

[bwendling]

I'm not sure I agree. It doesn't warn if I use it as an l-value:

$ cat ../t.c
int f(void);
int f(void) {
  int val;

  *&val = 37;
  return val;
}
$ clang -fsyntax-only t.c -Weverything
$

[bwendling]

https://reviews.llvm.org/D114848

[nathanchance]

That patch resolves the warnings in the problematic 0day config and does not introduce new ones for that config or x86_64 allmodconfig so LGTM on that front.

Curiously, I am surprised clang won't warn about this, either before or after the patch:

$ cat test.c
int foo(void);
int foo(void)
{
        int val;
        return *&val;
}

$ clang -fsyntax-only -Weverything test.c

$ gcc -Wall -Wextra -Wpedantic -c -o /dev/null test.c
test.c: In function ‘foo’:
test.c:5:16: warning: ‘val’ is used uninitialized [-Wuninitialized]
    5 |         return *&val;
      |                ^
test.c:4:13: note: ‘val’ declared here
    4 |         int val;
      |             ^~~

[bwendling]

Hmm...that's a bug for sure, but unrelated to this one.

[nathanchance]

This is now handled on the kernel side by initializing val under a preprocessor version check: https://git.kernel.org/powerpc/c/39bc1df821e07561bcbc44f6463f14101775782d

[isanbard]

The patch has landed.

[bwendling]

Should we ask that this be added to 13.0.1?

[nathanchance]

Sounds like it is not too late to request it to be added.

@tstellar Would it be possible to add https://github.com/llvm/llvm-project/commit/c4582a689c2c74e0635309979176c7ada086f066 to release/13.x? If there is a better way to request this while Bugzilla is being migrated, let me know.

[nathanchance]

Backport requested: https://github.com/llvm/llvm-project/issues/52633

[nathanchance]

Merged into release/13.x: https://github.com/llvm/llvm-project/commit/d904698b53e4f21a675d4ca0463843432ff4d590