void added inline comments.

================
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
----------------
nickdesaulniers wrote:
> 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);
> ```
The back end doesn't know how to generate code for a use in the indirect 
branches. It's really complicated and may result in code that doesn't actually 
work. I don't want to give off the impression that the code may work in these 
cases, because it would be essentially working by accident.


================
Comment at: clang/test/Analysis/uninit-asm-goto.cpp:38
+  return 0;
+}
----------------
nickdesaulniers wrote:
> 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;
> ```
Yes, we should be able to warn about this. I added a testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to