[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/lib/Analysis/UninitializedValues.cpp:651
+                  continue;
+                for (const auto &label : as->labels()) {
+                  const LabelStmt *ls = label->getLabel()->getStmt();
----------------
Is this more concise with `llvm::any_of`?


================
Comment at: clang/lib/Analysis/UninitializedValues.cpp:856
   if (!as->isAsmGoto())
     return;
 
----------------
Should we mark the vals `MayUninitialized` here, rather than below? Then I think we can remove the `isAsmGoto` check you added in `getUninitUse`?


================
Comment at: clang/test/Analysis/uninit-asm-goto.cpp:16
+  if (x < 42)
+    asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) : "r"(x) :: indirect_1, indirect_2);
+  else
----------------
I wonder if it's straight forward to make this a "maybe uninitialized" warning, instead of "is uninitialized?"  Consider the inline asm:

```
asm volatile goto("ja %l1; mov %0, 42" : "=r"(foo) ::: bar);
```
Since we can't introspect the inline asm, we're not sure whether `foo` gets initialized by the asm or not (as the asm could transfer control flow back to the C label before any assignments to the output).  Telling the user it's definitely uninitialized is technically correct in this case, but definitely incorrect for:

```
asm volatile goto("mov %0, 42; ja %l1;" : "=r"(foo) ::: bar);
```


================
Comment at: clang/test/Analysis/uninit-asm-goto.cpp:38
+  return 0;
+}
----------------
Are we able to catch backwards branches from `asm goto`? (if so, would you mind please added that as an additional unit test).

```
int foo;
goto 1;
2:
return foo; // should warn?
1:
asm goto ("": "=r"(foo):::2);
return foo;
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71314/new/

https://reviews.llvm.org/D71314