Make better use of getStringOperand() for MetadataExtractor.

This change also cleans up some of the code from a prior rebase.
It also captures two additional broken metadata cases (from corrupt
object slot and/or pragma metadata).

Change-Id: I61e161d2b0e211349683e36160d564493d802a12
diff --git a/bcinfo/MetadataExtractor.cpp b/bcinfo/MetadataExtractor.cpp
index 05a18cc..0282276 100644
--- a/bcinfo/MetadataExtractor.cpp
+++ b/bcinfo/MetadataExtractor.cpp
@@ -37,13 +37,24 @@
 
 namespace {
 
-static llvm::StringRef getStringOperand(const llvm::MDNode *node, unsigned operand) {
-  if (llvm::MDString *mds = llvm::dyn_cast_or_null<llvm::MDString>(
-        node->getOperand(operand))) {
+llvm::StringRef getStringOperand(const llvm::Metadata *node) {
+  if (auto *mds = llvm::dyn_cast_or_null<const llvm::MDString>(node)) {
     return mds->getString();
   }
   return llvm::StringRef();
 }
+
+bool extractUIntFromMetadataString(uint32_t *value,
+    const llvm::Metadata *m) {
+  llvm::StringRef SigString = getStringOperand(m);
+  if (SigString != "") {
+    if (!SigString.getAsInteger(10, *value)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }
 
 // Name of metadata node where pragma info resides (should be synced with
@@ -180,15 +191,13 @@
   for (size_t i = 0; i < mObjectSlotCount; i++) {
     llvm::MDNode *ObjectSlot = ObjectSlotMetadata->getOperand(i);
     if (ObjectSlot != nullptr && ObjectSlot->getNumOperands() == 1) {
-      llvm::StringRef Slot = getStringOperand(ObjectSlot, 0);
-      if (Slot != "") {
-        uint32_t USlot = 0;
-        if (Slot.getAsInteger(10, USlot)) {
-          ALOGE("Non-integer object slot value '%s'", Slot.str().c_str());
-          return false;
-        }
-        TmpSlotList[i] = USlot;
+      if (!extractUIntFromMetadataString(&TmpSlotList[i], ObjectSlot->getOperand(0))) {
+        ALOGE("Non-integer object slot value");
+        return false;
       }
+    } else {
+      ALOGE("Corrupt object slot information");
+      return false;
     }
   }
 
@@ -199,16 +208,11 @@
 
 
 static const char *createStringFromValue(llvm::Metadata *m) {
-  if (llvm::MDString *mds = llvm::dyn_cast_or_null<llvm::MDString>(m)) {
-    llvm::StringRef ref = mds->getString();
-    char *c = new char[ref.size() + 1];
-    memcpy(c, ref.data(), ref.size());
-    c[ref.size()] = '\0';
-
-    return c;
-  }
-
-  return nullptr;
+  auto ref = getStringOperand(m);
+  char *c = new char[ref.size() + 1];
+  memcpy(c, ref.data(), ref.size());
+  c[ref.size()] = '\0';
+  return c;
 }
 
 
@@ -400,15 +404,13 @@
   for (size_t i = 0; i < mExportForEachSignatureCount; i++) {
     llvm::MDNode *SigNode = Signatures->getOperand(i);
     if (SigNode != nullptr && SigNode->getNumOperands() == 1) {
-      llvm::StringRef SigString = getStringOperand(SigNode, 0);
-      if (SigString != "") {
-        uint32_t Signature = 0;
-        if (SigString.getAsInteger(10, Signature)) {
-          ALOGE("Non-integer signature value '%s'", SigString.str().c_str());
-          return false;
-        }
-        TmpSigList[i] = Signature;
+      if (!extractUIntFromMetadataString(&TmpSigList[i], SigNode->getOperand(0))) {
+        ALOGE("Non-integer signature value");
+        return false;
       }
+    } else {
+      ALOGE("Corrupt signature information");
+      return false;
     }
   }
 
@@ -462,14 +464,8 @@
   if (mdValue == nullptr)
     return;
 
-  const char *value = createStringFromValue(mdValue);
-  if (value == nullptr)
-    return;
-
-  if (strcmp(value, "no") == 0)
+  if (getStringOperand(mdValue) == "no")
     mIsThreadable = false;
-  return;
-
 }
 
 void MetadataExtractor::readBuildChecksumMetadata(
diff --git a/lib/Core/Compiler.cpp b/lib/Core/Compiler.cpp
index 0d75ded..6a513ff 100644
--- a/lib/Core/Compiler.cpp
+++ b/lib/Core/Compiler.cpp
@@ -151,7 +151,6 @@
   llvm::MCContext *mc_context = nullptr;
 
   passes.add(createTargetTransformInfoWrapperPass(mTarget->getTargetIRAnalysis()));
-  //mTarget->addAnalysisPasses(passes);
 
   // Add our custom passes.
   if (!addCustomPasses(pScript, passes)) {
diff --git a/lib/Renderscript/RSEmbedInfo.cpp b/lib/Renderscript/RSEmbedInfo.cpp
index 731e778..65f7ed7 100644
--- a/lib/Renderscript/RSEmbedInfo.cpp
+++ b/lib/Renderscript/RSEmbedInfo.cpp
@@ -123,7 +123,7 @@
     }
     s << "isThreadable: " << ((isThreadable) ? "yes" : "no") << "\n";
 
-    if (buildChecksum != nullptr) {
+    if (buildChecksum != nullptr && buildChecksum[0]) {
       s << "buildChecksum: " << buildChecksum << "\n";
     }
 
diff --git a/lib/Renderscript/RSForEachExpand.cpp b/lib/Renderscript/RSForEachExpand.cpp
index 9e9b48c..fc05426 100644
--- a/lib/Renderscript/RSForEachExpand.cpp
+++ b/lib/Renderscript/RSForEachExpand.cpp
@@ -358,8 +358,6 @@
     llvm::PHINode *IV;
 
     CondBB = Builder.GetInsertBlock();
-    // DT = &getAnalysis<DominatorTree>();
-    // LI = &getAnalysis<LoopInfo>();
     AfterBB = llvm::SplitBlock(CondBB, Builder.GetInsertPoint(), nullptr, nullptr);
     HeaderBB = llvm::BasicBlock::Create(*Context, "Loop", CondBB->getParent());
 
diff --git a/lib/Renderscript/RSScriptGroupFusion.cpp b/lib/Renderscript/RSScriptGroupFusion.cpp
index 7ee79bf..e16445c 100644
--- a/lib/Renderscript/RSScriptGroupFusion.cpp
+++ b/lib/Renderscript/RSScriptGroupFusion.cpp
@@ -57,7 +57,7 @@
   metadata.extract();
 
   const char* functionName = metadata.getExportForEachNameList()[slot];
-  if (functionName == nullptr) {
+  if (functionName == nullptr || !functionName[0]) {
     return nullptr;
   }
 
diff --git a/lib/Support/Initialization.cpp b/lib/Support/Initialization.cpp
index 7e36e47..16447f6 100644
--- a/lib/Support/Initialization.cpp
+++ b/lib/Support/Initialization.cpp
@@ -51,7 +51,7 @@
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmPrinters();
-  
+
   llvm::PassRegistry &Registry = *llvm::PassRegistry::getPassRegistry();
   llvm::initializeCore(Registry);
   llvm::initializeScalarOpts(Registry);