gollvm: insure that //line directives work correctly

Clean up the code that handles debug location meta-data to
insure that Go's //line directives work properly. This required
insuring that if an expression had no usable debug location we
use the debug location associated with its enclosing statement,
and keeping track of file changes within a function using the
DILexicalBlockFile pseudo-scope.

Change-Id: Ib72ce00f457ee8699fc15afc6d008a6ec16af1e8
Reviewed-on: https://go-review.googlesource.com/113723
Reviewed-by: Cherry Zhang <cherryyz@google.com>
diff --git a/bridge/go-llvm-dibuildhelper.cpp b/bridge/go-llvm-dibuildhelper.cpp
index 3f9853f..c40a1f2 100644
--- a/bridge/go-llvm-dibuildhelper.cpp
+++ b/bridge/go-llvm-dibuildhelper.cpp
@@ -183,6 +183,7 @@
 
 void DIBuildHelper::endFunction(Bfunction *function)
 {
+  cleanFileScope();
   llvm::DISubprogram *fscope = llvm::cast<llvm::DISubprogram>(currentDIScope());
 
   // Create debug meta-data for parameter variables.
@@ -212,6 +213,7 @@
     function->function()->setSubprogram(fscope);
 
   // Done with this scope
+  cleanFileScope();
   popDIScope();
   assert(diScopeStack_.size() == 1);
 
@@ -264,6 +266,8 @@
   if (! interestingBlock(block))
     return;
 
+  cleanFileScope();
+
   // In the case of the top block, we still want to process any local
   // variables it contains, but we'll want to insure that they are
   // parented by the function itself.
@@ -274,13 +278,19 @@
   processVarsInBLock(block->vars(), scope);
 }
 
-void DIBuildHelper::processExprInst(Bexpression *expr, llvm::Instruction *inst)
+void DIBuildHelper::processExprInst(Bstatement *stmt,
+                                    Bexpression *expr,
+                                    llvm::Instruction *inst)
 {
-  Location eloc = expr->location();
-  // && !linemap()->is_predeclared(eloc)) {
-  if (! linemap()->is_unknown(eloc)) {
+  Location loc = expr->location();
+  if (linemap()->is_unknown(loc)) {
+    Location sloc = stmt->location();
+    if (!linemap()->is_unknown(sloc))
+      loc = sloc;
+  }
+  if (! linemap()->is_unknown(loc)) {
     known_locations_ += 1;
-    inst->setDebugLoc(debugLocFromLocation(eloc));
+    inst->setDebugLoc(debugLocFromLocation(loc));
   }
 }
 
@@ -312,6 +322,19 @@
 llvm::DebugLoc DIBuildHelper::debugLocFromLocation(Location loc)
 {
   llvm::LLVMContext &context = typemanager()->context();
+
+  // In the (somewhat unusual) case of a file/line directive, create a new
+  // pseudo-scope to capture the fact that the file has changed. Pop off
+  // any previously created file scope prior to doing this.
+  llvm::DIFile *curFile = currentDIScope()->getFile();
+  llvm::DIFile *locFile = diFileFromLocation(loc);
+  if (curFile != locFile) {
+    cleanFileScope();
+    llvm::DILexicalBlockFile *dilbf =
+        dibuilder().createLexicalBlockFile(currentDIScope(), locFile);
+    pushDIScope(dilbf);
+  }
+
   return llvm::DILocation::get(context, linemap()->location_line(loc),
                                linemap()->location_column(loc),
                                currentDIScope());
@@ -336,3 +359,10 @@
   assert(scope);
   diScopeStack_.push_back(scope);
 }
+
+void DIBuildHelper::cleanFileScope()
+{
+  assert(diScopeStack_.size());
+  if (llvm::dyn_cast<llvm::DILexicalBlockFile>(diScopeStack_.back()) != nullptr)
+    diScopeStack_.pop_back();
+}
diff --git a/bridge/go-llvm-dibuildhelper.h b/bridge/go-llvm-dibuildhelper.h
index 4d5df84..68fb077 100644
--- a/bridge/go-llvm-dibuildhelper.h
+++ b/bridge/go-llvm-dibuildhelper.h
@@ -36,6 +36,7 @@
 class Bnode;
 class Bblock;
 class Bexpression;
+class Bstatement;
 class Bfunction;
 class Btype;
 class Bvariable;
@@ -64,7 +65,9 @@
 
   llvm::DIFile *diFileFromLocation(Location location);
 
-  void processExprInst(Bexpression *expr, llvm::Instruction *inst);
+  void processExprInst(Bstatement *containingStmt,
+                       Bexpression *expr,
+                       llvm::Instruction *inst);
 
   // Support for -fdebug-prefix
   void addDebugPrefix(std::pair<llvm::StringRef, llvm::StringRef>);
@@ -79,6 +82,10 @@
   llvm::DIScope *popDIScope();
   void pushDIScope(llvm::DIScope *);
 
+  // If there is a "set file" pseudo scope at the top of
+  // the scope stack, remove it.
+  void cleanFileScope();
+
   // Various getters
   llvm::DIBuilder &dibuilder() { return *dibuilder_.get(); }
   Llvm_linemap *linemap() { return linemap_; }
diff --git a/bridge/go-llvm.cpp b/bridge/go-llvm.cpp
index 1e09d29..e6355d3 100644
--- a/bridge/go-llvm.cpp
+++ b/bridge/go-llvm.cpp
@@ -2554,7 +2554,10 @@
             Bfunction *function, Bnode *topNode,
             DIBuildHelper *diBuildHelper, llvm::BasicBlock *entryBlock);
 
-  llvm::BasicBlock *walk(Bnode *node, llvm::BasicBlock *curblock);
+  // Here 'stmt' is the containing stmt, or null if node itself is a stmt.
+  llvm::BasicBlock *walk(Bnode *node,
+                         Bstatement *stmt,
+                         llvm::BasicBlock *curblock);
   void finishFunction(llvm::BasicBlock *entry);
 
   Bfunction *function() { return function_; }
@@ -2574,7 +2577,9 @@
                             unsigned expl = Llvm_backend::ChooseVer);
   llvm::BasicBlock *eraseBlockIfUnused(llvm::BasicBlock *bb);
   llvm::BasicBlock *getBlockForLabel(Blabel *lab);
-  llvm::BasicBlock *walkExpr(llvm::BasicBlock *curblock, Bexpression *expr);
+  llvm::BasicBlock *walkExpr(llvm::BasicBlock *curblock,
+                             Bstatement *containingStmt,
+                             Bexpression *expr);
   std::pair<llvm::Instruction*, llvm::BasicBlock *>
   rewriteToMayThrowCall(llvm::CallInst *call,
                         llvm::BasicBlock *curblock);
@@ -2586,7 +2591,9 @@
   llvm::BasicBlock *populateFinallyBlock(llvm::BasicBlock *finBB,
                                          Bvariable *finvar,
                                          llvm::Value *extmp);
-  void walkReturn(llvm::BasicBlock *curblock, Bexpression *re);
+  void walkReturn(llvm::BasicBlock *curblock,
+                  Bstatement *stmt,
+                  Bexpression *re);
   void appendLifetimeIntrinsic(llvm::Value *alloca,
                                llvm::Instruction *insertBefore,
                                llvm::BasicBlock *curBlock,
@@ -2828,6 +2835,7 @@
 }
 
 llvm::BasicBlock *GenBlocks::walkExpr(llvm::BasicBlock *curblock,
+                                      Bstatement *containingStmt,
                                       Bexpression *expr)
 {
   // Delete dead instructions before visiting the children,
@@ -2839,7 +2847,7 @@
   // Visit children first
   const std::vector<Bnode *> &kids = expr->children();
   for (auto &child : kids)
-    curblock = walk(child, curblock);
+    curblock = walk(child, containingStmt, curblock);
 
   // In case it becomes dead after visiting some child...
   if (!curblock)
@@ -2858,7 +2866,7 @@
     if (inst != originst)
       changed = true;
     if (dibuildhelper_)
-      dibuildhelper_->processExprInst(expr, inst);
+      dibuildhelper_->processExprInst(containingStmt, expr, inst);
     curblock->getInstList().push_back(inst);
     curblock = pair.second;
     newinsts.push_back(inst);
@@ -2879,7 +2887,7 @@
   Bstatement *falseStmt = ifst->getIfStmtFalseBlock();
 
   // Walk condition first
-  curblock = walkExpr(curblock, cond);
+  curblock = walkExpr(curblock, ifst, cond);
 
   // Create true block
   llvm::BasicBlock *tblock = curblock ? mkLLVMBlock("then") : nullptr;
@@ -2899,13 +2907,13 @@
   }
 
   // Visit true block
-  llvm::BasicBlock *tsucc = walk(trueStmt, tblock);
+  llvm::BasicBlock *tsucc = walk(trueStmt, nullptr, tblock);
   if (tsucc && ! tsucc->getTerminator())
     llvm::BranchInst::Create(ft, tsucc);
 
   // Walk false block if present
   if (falseStmt) {
-    llvm::BasicBlock *fsucc = walk(falseStmt, fblock);
+    llvm::BasicBlock *fsucc = walk(falseStmt, nullptr, fblock);
     if (fsucc && ! fsucc->getTerminator())
       llvm::BranchInst::Create(ft, fsucc);
   }
@@ -2923,7 +2931,7 @@
 
   // Walk switch value first
   Bexpression *swval = swst->getSwitchStmtValue();
-  curblock = walkExpr(curblock, swval);
+  curblock = walkExpr(curblock, swst, swval);
 
   // Unpack switch
   unsigned ncases = swst->getSwitchStmtNumCases();
@@ -2953,7 +2961,7 @@
   // Walk statement/block
   for (unsigned idx = 0; idx < ncases; ++idx) {
     Bstatement *st = swst->getSwitchStmtNthStmt(idx);
-    llvm::BasicBlock *newblock = walk(st, blocks[idx]);
+    llvm::BasicBlock *newblock = walk(st, nullptr, blocks[idx]);
     if (newblock && newblock->getTerminator() == nullptr) {
       // The front end inserts a goto statement at the end for
       // non-fallthrough cases. Fall through to the next case
@@ -3002,9 +3010,11 @@
   return bb;
 }
 
-void GenBlocks::walkReturn(llvm::BasicBlock *curblock, Bexpression *re)
+void GenBlocks::walkReturn(llvm::BasicBlock *curblock,
+                           Bstatement *stmt,
+                           Bexpression *re)
 {
-  llvm::BasicBlock *bb = walkExpr(curblock, re);
+  llvm::BasicBlock *bb = walkExpr(curblock, stmt, re);
   assert(curblock == bb);
   if (curblock) {
     llvm::Instruction *term = curblock->getTerminator();
@@ -3051,7 +3061,7 @@
     llvm::BranchInst::Create(finallyBlock_, curblock);
   } else {
     // Walk return expression
-    walkReturn(curblock, re);
+    walkReturn(curblock, rst, re);
   }
 
   // A return terminates the current block
@@ -3095,7 +3105,7 @@
   padBlockStack_.push_back(padbb);
 
   // Walk the undcall expression.
-  curblock = walkExpr(curblock, undcallex);
+  curblock = walkExpr(curblock, defst, undcallex);
 
   // Pop the pad block stack.
   padBlockStack_.pop_back();
@@ -3111,7 +3121,7 @@
 
   // Catch block containing defer call.
   curblock = catchbb;
-  auto bb = walkExpr(curblock, defcallex);
+  auto bb = walkExpr(curblock, defst, defcallex);
   assert(bb == curblock);
   llvm::BranchInst::Create(finbb, catchbb);
 
@@ -3189,7 +3199,7 @@
   if (cachedReturn_ != nullptr) {
     llvm::BasicBlock *curblock = finRetBB;
     Bexpression *re = cachedReturn_->getReturnStmtExpr();
-    walkReturn(curblock, re);
+    walkReturn(curblock, cachedReturn_, re);
     cachedReturn_ = nullptr;
     finRetBB = nullptr;
   }
@@ -3314,7 +3324,7 @@
   padBlockStack_.push_back(padbb);
 
   // Walk the body statement.
-  curblock = walk(body, curblock);
+  curblock = walk(body, nullptr, curblock);
 
   // Pop the pad block stack.
   padBlockStack_.pop_back();
@@ -3347,7 +3357,7 @@
 
   // Push second pad, walk exception stmt, then pop the pad.
   padBlockStack_.push_back(catchpadbb);
-  auto bb = walk(ifexception, catchbb);
+  auto bb = walk(ifexception, nullptr, catchbb);
   if (bb)
     llvm::BranchInst::Create(finokBB, bb);
   padBlockStack_.pop_back();
@@ -3365,7 +3375,7 @@
   // Handle finally statement where applicable.
   curblock = contBB;
   if (finally != nullptr) {
-    curblock = walk(finally, curblock);
+    curblock = walk(finally, nullptr, curblock);
 
     // Augment the end of the finally block with an if/jump to
     // return or resume, and populate the return/resume blocks.
@@ -3376,16 +3386,17 @@
 }
 
 llvm::BasicBlock *GenBlocks::walk(Bnode *node,
+                                  Bstatement *containingStmt,
                                   llvm::BasicBlock *curblock)
 {
   Bexpression *expr = node->castToBexpression();
   if (expr)
-    return walkExpr(curblock, expr);
+    return walkExpr(curblock, containingStmt, expr);
   Bstatement *stmt = node->castToBstatement();
   assert(stmt);
   switch (stmt->flavor()) {
     case N_ExprStmt: {
-      curblock = walkExpr(curblock, stmt->getExprStmtExpr());
+      curblock = walkExpr(curblock, stmt, stmt->getExprStmtExpr());
       break;
     }
     case N_BlockStmt: {
@@ -3396,7 +3407,7 @@
       if (dibuildhelper_)
         dibuildhelper_->beginLexicalBlock(bblock);
       for (auto &st : stmt->getChildStmts())
-        curblock = walk(st, curblock);
+        curblock = walk(st, nullptr, curblock);
       if (dibuildhelper_)
         dibuildhelper_->endLexicalBlock(bblock);
       if (curblock)
@@ -3507,7 +3518,7 @@
   // Walk the code statements
   GenBlocks gb(context_, this, function, code_stmt,
                dibh, entryBlock);
-  llvm::BasicBlock *block = gb.walk(code_stmt, entryBlock);
+  llvm::BasicBlock *block = gb.walk(code_stmt, nullptr, entryBlock);
   gb.finishFunction(entryBlock);
 
   // Fix up epilog block if needed
diff --git a/unittests/BackendCore/BackendDebugEmit.cpp b/unittests/BackendCore/BackendDebugEmit.cpp
index 77179a9..a3e937e 100644
--- a/unittests/BackendCore/BackendDebugEmit.cpp
+++ b/unittests/BackendCore/BackendDebugEmit.cpp
@@ -172,10 +172,10 @@
   h.mkLocal("x", bu32t);
 
   const char *exp = R"RAW_RESULT(
-    define void @foo(i8* nest %nest.0) #0 {
+    define void @foo(i8* nest %nest.0) #0 !dbg !3 {
     entry:
       %x = alloca i32
-      ret void
+      ret void, !dbg !8
     }
   )RAW_RESULT";
 
@@ -231,11 +231,47 @@
   bool broken = h.finish(PreserveDebugInfo);
   EXPECT_FALSE(broken && "Module failed to verify.");
 
-  be->dumpModule();
-
   // Check for remapped source file.
   bool ok = h.expectModuleDumpContains("!DIFile(filename: \"barcode.go\", directory: \"/something/another\")");
   EXPECT_TRUE(ok);
 }
 
+TEST(BackendDebugEmit, TestFileLineDirectives) {
+
+  FcnTestHarness h;
+  Llvm_backend *be = h.be();
+  Btype *bi64t = be->integer_type(false, 64);
+  BFunctionType *befty = mkFuncTyp(be, L_PARM, bi64t, L_RES, bi64t, L_END);
+  Bfunction *func = h.mkFunction("bar", befty);
+  Bvariable *p0 = func->getNthParamVar(0);
+
+  Location loc = h.newFileLineLoc("watermelon.go", 43);
+  Bexpression *vex1 = be->var_expression(p0, loc);
+  Bvariable *xv = h.mkLocal("x", bi64t, vex1);
+
+  loc = h.newFileLineLoc("kiwifruit.go", 43);
+  Bexpression *vex2 = be->var_expression(p0, loc);
+  Bexpression *vex3 = be->var_expression(xv, loc);
+  h.mkAssign(vex2, vex3);
+
+  loc = h.newFileLineLoc("apple.go", 11);
+  Bexpression *vex4 = be->var_expression(p0, loc);
+  h.mkReturn(std::vector<Bexpression*>{vex4});
+
+  bool broken = h.finish(PreserveDebugInfo);
+  EXPECT_FALSE(broken && "Module failed to verify.");
+
+  be->dumpModule();
+
+  // Three of the constructs above had different files applied to them
+  // (equivalent of Go //line directive); make sure that the files
+  // appear in the meta-data.
+  bool ok = h.expectModuleDumpContains("!DIFile(filename: \"watermelon.go\", directory: \"\")");
+  EXPECT_TRUE(ok);
+  ok = h.expectModuleDumpContains("!DIFile(filename: \"kiwifruit.go\", directory: \"\")");
+  EXPECT_TRUE(ok);
+  ok = h.expectModuleDumpContains("!DIFile(filename: \"apple.go\", directory: \"\")");
+  EXPECT_TRUE(ok);
+}
+
 }