[clang] 72aa619 - Warn of uninitialized variables on asm goto's indirect branch

[Bill Wendling via cfe-commits]

Author: Bill Wendling
Date: 2020-03-10T13:48:48-07:00
New Revision: 72aa619a7fe0e2f90959b0614f6388c09aba8913

URL: https://github.com/llvm/llvm-project/commit/72aa619a7fe0e2f90959b0614f6388c09aba8913
DIFF: https://github.com/llvm/llvm-project/commit/72aa619a7fe0e2f90959b0614f6388c09aba8913.diff

LOG: Warn of uninitialized variables on asm goto's indirect branch

Summary:
Outputs from an asm goto block cannot be used on the indirect branch.
It's not supported and may result in invalid code generation.

Reviewers: jyknight, nickdesaulniers, hfinkel

Reviewed By: nickdesaulniers

Subscribers: martong, cfe-commits, rnk, craig.topper, hiraditya, rsmith

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71314

Added: 
    

Modified: 
    clang/lib/Analysis/UninitializedValues.cpp
    clang/test/Analysis/uninit-asm-goto.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/UninitializedValues.cpp b/clang/lib/Analysis/UninitializedValues.cpp
index 16d17de599f2..ba64fbb81884 100644
--- a/clang/lib/Analysis/UninitializedValues.cpp
+++ b/clang/lib/Analysis/UninitializedValues.cpp
@@ -576,6 +576,28 @@ class TransferFunctions : public StmtVisitor<TransferFunctions> {
           continue;
         }
 
+        if (AtPredExit == MayUninitialized) {
+          // If the predecessor's terminator is an "asm goto" that initializes
+          // the variable, then it won't be counted as "initialized" on the
+          // non-fallthrough paths.
+          CFGTerminator term = Pred->getTerminator();
+          if (const auto *as = dyn_cast_or_null<GCCAsmStmt>(term.getStmt())) {
+            const CFGBlock *fallthrough = *Pred->succ_begin();
+            if (as->isAsmGoto() &&
+                llvm::any_of(as->outputs(), [&](const Expr *output) {
+                    return vd == findVar(output).getDecl() &&
+                        llvm::any_of(as->labels(),
+                                     [&](const AddrLabelExpr *label) {
+                          return label->getLabel()->getStmt() == B->Label &&
+                              B != fallthrough;
+                        });
+                })) {
+              Use.setUninitAfterDecl();
+              continue;
+            }
+          }
+        }
+
         unsigned &SV = SuccsVisited[Pred->getBlockID()];
         if (!SV) {
           // When visiting the first successor of a block, mark all NULL
@@ -768,7 +790,11 @@ void TransferFunctions::VisitGCCAsmStmt(GCCAsmStmt *as) {
 
   for (const Expr *o : as->outputs())
     if (const VarDecl *VD = findVar(o).getDecl())
-      vals[VD] = Initialized;
+      if (vals[VD] != Initialized)
+        // If the variable isn't initialized by the time we get here, then we
+        // mark it as potentially uninitialized for those cases where it's used
+        // on an indirect path, where it's not guaranteed to be defined.
+        vals[VD] = MayUninitialized;
 }
 
 void TransferFunctions::VisitObjCMessageExpr(ObjCMessageExpr *ME) {
@@ -809,7 +835,7 @@ static bool runOnBlock(const CFGBlock *block, const CFG &cfg,
       tf.Visit(const_cast<Stmt *>(cs->getStmt()));
   }
   CFGTerminator terminator = block->getTerminator();
-  if (GCCAsmStmt *as = dyn_cast_or_null<GCCAsmStmt>(terminator.getStmt()))
+  if (auto *as = dyn_cast_or_null<GCCAsmStmt>(terminator.getStmt()))
     if (as->isAsmGoto())
       tf.Visit(as);
   return vals.updateValueVectorWithScratch(block);

diff  --git a/clang/test/Analysis/uninit-asm-goto.cpp b/clang/test/Analysis/uninit-asm-goto.cpp
index 1d573b90016e..136b210b8866 100644
--- a/clang/test/Analysis/uninit-asm-goto.cpp
+++ b/clang/test/Analysis/uninit-asm-goto.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -Wuninitialized -verify %s
-// expected-no-diagnostics
 
+// test1: Expect no diagnostics
 int test1(int x) {
     int y;
     asm goto("nop" : "=r"(y) : "r"(x) : : err);
@@ -8,3 +8,52 @@ int test1(int x) {
   err:
     return -1;
 }
+
+int test2(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+         // expected-note {{initialize the variable}}
+  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
+    asm volatile goto("testl %0, %1; testl %2, %3; jne %l5" : "+S"(x), "+D"(y) : "r"(x), "r"(y) :: indirect_1, indirect_2);
+  return x + y;
+indirect_1:
+  return -42;
+indirect_2:
+  return y; // expected-note {{uninitialized use occurs here}}
+}
+
+int test3(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+         // expected-note {{initialize the variable}}
+  asm goto("xorl %1, %0; jmp %l2" : "=&r"(y) : "r"(x) : : fail);
+normal:
+  y += x;
+  return y;
+  if (x) {
+fail:
+    return y; // expected-note {{uninitialized use occurs here}}
+  }
+  return 0;
+}
+
+int test4(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+         // expected-note {{initialize the variable}}
+  goto forward;
+backward:
+  return y; // expected-note {{uninitialized use occurs here}}
+forward:
+  asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : backward);
+  return y;
+}
+
+// test5: Expect no diagnostics
+int test5(int x) {
+  int y;
+  asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) : "r"(x) :: indirect, fallthrough);
+fallthrough:
+  return y;
+indirect:
+  return -2;
+}