Implemented global redzones.

src/IceASanInstrumentation.cpp

+//===- subzero/src/IceASanInstrumentation.cpp - ASan ------------*- C++ -*-===//
+//
+//                        The Subzero Code Generator
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief Implements the AddressSanitizer instrumentation class.
+///
+//===----------------------------------------------------------------------===//
+
+#include "IceASanInstrumentation.h"
+
+#include "IceBuildDefs.h"
+#include "IceGlobalInits.h"
+
+#include <sstream>
+
+namespace Ice {
+
+const std::string ASanInstrumentation::RzPrefix = "__$rz";
+
+// Create redzones between all global variables, ensuring that the initializer
+// types of the redzones and their associated globals match so that they are
+// laid out together in memory.
+void ASanInstrumentation::instrumentGlobals(VariableDeclarationList &Globals) {
+  if (BuildDefs::minimal() || DidInsertRedZones)
+    return;
+
+  VariableDeclarationList NewGlobals;
+  // Global holding pointers to all redzones
+  VariableDeclaration *RzArray;
+  // Global holding the size of RzArray
+  VariableDeclaration *RzArraySizeVar;
+  static SizeT RzArraySize;
+
+  RzArray = VariableDeclaration::create(&NewGlobals);

[Karl, 2016/06/10 15:20:27]

Why not combine this with line 35 as the initial value for RzArray?

[tlively, 2016/06/10 17:30:22]

Done.

+  RzArraySizeVar = VariableDeclaration::create(&NewGlobals);

[Karl, 2016/06/10 15:20:27]

Same here.

[tlively, 2016/06/10 17:30:22]

Done.

+  RzArray->setName(Ctx, nextRzName());
+  RzArraySizeVar->setName(Ctx, nextRzName());
+  NewGlobals.push_back(RzArray);
+  NewGlobals.push_back(RzArraySizeVar);
+
+  // TODO(tlively): Consider alignment when determining redzone layout.
+  for (VariableDeclaration *Global : Globals) {
+    VariableDeclaration *RzLeft =
+        createRz(&NewGlobals, RzArray, RzArraySize, Global);
+    VariableDeclaration *RzRight =
+        createRz(&NewGlobals, RzArray, RzArraySize, Global);
+    NewGlobals.push_back(RzLeft);
+    NewGlobals.push_back(Global);
+    NewGlobals.push_back(RzRight);
+  }
+
+  // update the contents of the RzArraySize global
+  llvm::NaClBitcodeRecord::RecordVector SizeContents;
+  for (unsigned i = 0; i < sizeof(RzArraySize); i++) {
+    SizeContents.emplace_back(RzArraySize % (1 << CHAR_BIT));
+    RzArraySize >>= CHAR_BIT;
+  }
+  RzArraySizeVar->addInitializer(
+      VariableDeclaration::DataInitializer::create(&NewGlobals, SizeContents));
+
+  // Replace old list of globals, without messing up arena allocators
+  Globals.clear();
+  Globals.merge(&NewGlobals);
+  DidInsertRedZones = true;
+
+  // Log the new set of globals
+  if (BuildDefs::dump()) {

[Karl, 2016/06/10 15:20:26]

The preferred way of locking the Str lock is with an instance of OstreamLocker.
This class automatically locks on creation, and unlocks when destructed.

Hence, replace Ctx->lockStr() with:

   OstreamLocker _(Ctx);

And remove Ctx->unlockStr();

Note: We typically use identifier "_" to represent variables that we only want
for its initialization/destruction (i.e. no uses/references to it in the code).

[tlively, 2016/06/10 17:30:22]

Done.

+    Ctx->lockStr();
+    Ctx->getStrDump() << "========= Instrumented Globals =========\n";
+    for (VariableDeclaration *Global : Globals) {
+      Global->dump(Ctx->getStrDump());
+    }
+    Ctx->unlockStr();
+  }
+}
+
+std::string ASanInstrumentation::nextRzName() {
+  std::stringstream Name;
+  Name << RzPrefix << RzNum++;
+  return Name.str();
+}
+
+VariableDeclaration *
+ASanInstrumentation::createRz(VariableDeclarationList *List,
+                              VariableDeclaration *RzArray, SizeT &RzArraySize,
+                              VariableDeclaration *Global) {
+  VariableDeclaration *Rz = VariableDeclaration::create(List);
+  Rz->setName(Ctx, nextRzName());
+  if (Global->hasNonzeroInitializer()) {
+    llvm::NaClBitcodeRecord::RecordVector Contents(RzSize, 'R');

[Karl, 2016/06/10 15:20:27]

Minor nit. Do you really want the contents (other than the first byte) be
uninitialized?

[Karl, 2016/06/10 15:30:49]

This record "Contents" is just used to provide the initializer for Rz. It is the
same for all poison variables. Why not make this a class-level field and
initialize in the constructor? 

[tlively, 2016/06/10 17:30:22]

I went a step further and made it a static member of the class.

[tlively, 2016/06/10 17:30:22]

This constructor fills the entire vector with 'R'.

+    Rz->addInitializer(
+        VariableDeclaration::DataInitializer::create(List, Contents));
+  } else {
+    Rz->addInitializer(
+        VariableDeclaration::ZeroInitializer::create(List, RzSize));
+  }
+  Rz->setIsConstant(Global->getIsConstant());
+  RzArray->addInitializer(VariableDeclaration::RelocInitializer::create(
+      List, Rz, RelocOffsetArray(0)));
+  ++RzArraySize;
+  return Rz;
+}
+
+} // end of namespace Ice