From 62738d3641ce3c7b10775b170b1287b84d6838c1 Mon Sep 17 00:00:00 2001
From: Sean Silva <silvasean@google.com>
Date: Thu, 17 Sep 2020 19:03:33 -0700
Subject: [PATCH] [RefE2E] Fix nul-termination bug.

I was seeing some of the error messages come out with some garbage at
the end. This fixes it.
---
 lib/E2E/LowerToLLVM.cpp     | 11 +++++++----
 test/E2E/lower-to-llvm.mlir |  6 +++---
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/E2E/LowerToLLVM.cpp b/lib/E2E/LowerToLLVM.cpp
index bab7d64c1..72cda7be6 100644
--- a/lib/E2E/LowerToLLVM.cpp
+++ b/lib/E2E/LowerToLLVM.cpp
@@ -153,10 +153,13 @@ public:
 static LLVM::GlobalOp createGlobalString(ModuleOp module, StringAttr msg,
                                          OpBuilder &builder, Location loc) {
   // TODO: Deduplicate strings.
+  std::string msgNulTerminated = msg.getValue().str();
+  msgNulTerminated.push_back('\0');
   auto arrayTy = LLVMType::getArrayTy(LLVMType::getInt8Ty(module.getContext()),
-                                      msg.getValue().size());
+                                      msgNulTerminated.size());
   OpBuilder::InsertionGuard guard(builder);
   builder.setInsertionPointToStart(module.getBody());
+
   // To get a unique symbol name, use a suffix derived from the current number
   // of ops in the module.
   // We can't use the SymbolTable's logic for this because the module
@@ -166,9 +169,9 @@ static LLVM::GlobalOp createGlobalString(ModuleOp module, StringAttr msg,
       (Twine("__npcomp_string_") +
        Twine(llvm::size(llvm::to_vector<6>(module.getOps<LLVM::GlobalOp>()))))
           .str();
-  auto globalOp =
-      builder.create<LLVM::GlobalOp>(loc, arrayTy, /*isConstant=*/true,
-                                     LLVM::Linkage::Internal, symbolName, msg);
+  auto globalOp = builder.create<LLVM::GlobalOp>(
+      loc, arrayTy, /*isConstant=*/true, LLVM::Linkage::Internal, symbolName,
+      builder.getStringAttr(msgNulTerminated));
   return globalOp;
 }
 
diff --git a/test/E2E/lower-to-llvm.mlir b/test/E2E/lower-to-llvm.mlir
index ec7f071e7..43b903cf7 100644
--- a/test/E2E/lower-to-llvm.mlir
+++ b/test/E2E/lower-to-llvm.mlir
@@ -211,7 +211,7 @@ func @inputs1results2(%arg0: !npcomprt.tensor) -> (!npcomprt.tensor, !npcomprt.t
 
 // Test emission of compiler runtime functions.
 
-// CHECK:         llvm.mlir.global internal constant @[[STRSYM:.*]]("msg")
+// CHECK:         llvm.mlir.global internal constant @[[STRSYM:.*]]("msg\00")
 // CHECK:         llvm.func @__npcomp_compiler_rt_abort_if(!llvm.i1, !llvm.ptr<i8>)
 // CHECK:         llvm.func @__npcomp_compiler_rt_to_memref(!llvm.ptr<i8>) -> !llvm.struct<(i64, ptr<i8>)>
 // CHECK:         llvm.func @__npcomp_compiler_rt_from_memref(!llvm.i64, !llvm.ptr<i8>) -> !llvm.ptr<i8>
@@ -219,9 +219,9 @@ func @inputs1results2(%arg0: !npcomprt.tensor) -> (!npcomprt.tensor, !npcomprt.t
 
 // CHECK-LABEL:   llvm.func @calls_abort_if(
 // CHECK-SAME:                              %[[VAL_0:.*]]: !llvm.i1) {
-// CHECK:         %[[VAL_0:.*]] = llvm.mlir.addressof @[[STRSYM]] : !llvm.ptr<array<3 x i8>>
+// CHECK:         %[[VAL_0:.*]] = llvm.mlir.addressof @[[STRSYM]] : !llvm.ptr<array<4 x i8>>
 // CHECK:         %[[VAL_1:.*]] = llvm.mlir.constant(0 : i32) : !llvm.i32
-// CHECK:         %[[VAL_2:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_1]], %[[VAL_1]]] : (!llvm.ptr<array<3 x i8>>, !llvm.i32, !llvm.i32) -> !llvm.ptr<i8>
+// CHECK:         %[[VAL_2:.*]] = llvm.getelementptr %[[VAL_0]]{{\[}}%[[VAL_1]], %[[VAL_1]]] : (!llvm.ptr<array<4 x i8>>, !llvm.i32, !llvm.i32) -> !llvm.ptr<i8>
 // CHECK:         llvm.call @__npcomp_compiler_rt_abort_if(%[[VAL_3:.*]], %[[VAL_2]]) : (!llvm.i1, !llvm.ptr<i8>) -> ()
 // CHECK:         llvm.return