[Analysis] Ignore casts and unary ops for uninitialized values

[void]

Summary

A series of unary operators and casts may obscure the variable we're
trying to analyze. Ignore them for the uninitialized value analysis.
Other checks determine if the unary operators result in a valid l-value.

Link: https://github.com/ClangBuiltLinux/linux/issues/1521

[nathanchance]

I tested the problematic Linux kernel configuration that uncovered this issue with this patch and the issue was resolved. I tested an x86_64 allmodconfig build and did not see any warnings.

clang/lib/Analysis/UninitializedValues.cpp

594–595

Should "it's don't count" be "don't count"?

[void]

Pretend that I've spoken English for most of my lyfe.

clang/lib/Analysis/UninitializedValues.cpp

594–595

You does not like mine English? :-P

[void]

Friendly ping to reviewers. :-)

[nickdesaulniers]

clang/lib/Analysis/UninitializedValues.cpp

819–820

Q: are there any unary operators that are not casts? For example isn't something like -x a unary negation operator? I worry that this could be an infinite loop as written.

If this fear is unfounded, then I think the code could be rewritten as:

while (const auto *UO = dyn_cast<UnaryOperator>(Ex))
  Ex = stripCasts(C, UO->getSubExpr());

[void]

clang/lib/Analysis/UninitializedValues.cpp

819–820

It's not just casts that I want to remove. In particular, the & and * operators need to be removed to get to the variable. It *should* eventually get to a variable or non-unary expression once it goes through all of the unary operators and casts.

Note there are other sema checks that determine if the expression is a proper l-value, so I can throw away the unary-ops.

[nickdesaulniers]

clang/lib/Analysis/UninitializedValues.cpp

819–820

Ok, my original concern seems fine.

I still think you should rewrite the loop as suggested. At the least, a dyn_cast shouldn't be necessary once we've already checked with isa; cast could simply be used. Or even better just one dyn_cast as suggested.

[void]

Simplify the loop.

clang/lib/Analysis/UninitializedValues.cpp

while (!Queue.empty()) {
           // this code is intended to be reachable) so give a diagnostic here
           // and go no further down this path.
           Use.setUninitAfterDecl();
           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.
+          // the variable, then don't count it as "initialized" on the indirect
+          // 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) {
void TransferFunctions::VisitDeclStmt(DeclStmt *DS) {
   }
 }
 
 void TransferFunctions::VisitGCCAsmStmt(GCCAsmStmt *as) {
   // An "asm goto" statement is a terminator that may initialize some variables.
   if (!as->isAsmGoto())
     return;
 
-  for (const Expr *o : as->outputs())
-    if (const VarDecl *VD = findVar(o).getDecl())
+  ASTContext &C = ac.getASTContext();
+  for (const Expr *O : as->outputs()) {
+    const Expr *Ex = stripCasts(C, O);
+
+    // Strip away any unary operators. Invalid l-values are reported by other
+    // semantic analysis passes.
+    while (isa<UnaryOperator>(Ex))
+      Ex = stripCasts(C, dyn_cast<UnaryOperator>(Ex)->getSubExpr());
+
+    if (const VarDecl *VD = findVar(Ex).getDecl())
       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) {
   // If the Objective-C message expression is an implicit no-return that
   // is not modeled in the CFG, set the tracked dataflow values to Unknown.
   if (objCNoRet.isImplicitNoReturn(ME)) {
     vals.setAllScratchValues(Unknown);
   }
 }

clang/test/Analysis/uninit-asm-goto.cpp

 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;
 }
+
+// test6: Expect no diagnostics.
+int test6(unsigned int *x) {
+  unsigned int val;
+
+  // See through casts and unary operators.
+  asm goto("nop" : "=r" (*(unsigned int *)(&val)) ::: indirect);
+  *x = val;
+  return 0;
+indirect:
+  return -1;
+}