Adding a pass - RewritePNaClLibraryCalls, that replaces known library calls with stable bitcode int…

lib/Transforms/NaCl/RewritePNaClLibraryCalls.cpp

Index: lib/Transforms/NaCl/RewritePNaClLibraryCalls.cpp
diff --git a/lib/Transforms/NaCl/RewritePNaClLibraryCalls.cpp b/lib/Transforms/NaCl/RewritePNaClLibraryCalls.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..76de0ef16d05375f9037d4e6720091873d0dc58c
--- /dev/null
+++ b/lib/Transforms/NaCl/RewritePNaClLibraryCalls.cpp
@@ -0,0 +1,140 @@
+//===- RewritePNaClLibraryCalls.cpp - PNaCl library calls to intrinsics ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass replaces calls to known library functions by calls to intrinsics

[Mark Seaborn, 2013/05/10 16:34:36]

'by' -> 'with'

[eliben, 2013/05/10 17:00:51]

Done.

+// that are part of the PNaCl stable bitcode ABI.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/Twine.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Pass.h"
+#include "llvm/Transforms/NaCl.h"
+
+using namespace llvm;
+
+namespace {
+  class RewritePNaClLibraryCalls : public ModulePass {

[Mark Seaborn, 2013/05/10 16:34:36]

Maybe comment why this is a ModulePass?  Does it need to be a ModulePass if it
introduces declarations for intrinsics?

[eliben, 2013/05/10 17:00:51]

Done.

+  public:
+    static char ID;
+    RewritePNaClLibraryCalls() : ModulePass(ID) {
+      initializeExpandTlsPass(*PassRegistry::getPassRegistry());
+    }
+
+    virtual bool runOnModule(Module &M);
+  private:
+    /// Rewrite the given \p Call to setjmp
+    void rewriteSetjmp(CallInst *Call);
+    /// Rewrite the given \p Call to longjmp
+    void rewriteLongjmp(CallInst *Call);
+
+    Module *TheModule;
+  };
+}
+
+char RewritePNaClLibraryCalls::ID = 0;
+INITIALIZE_PASS(RewritePNaClLibraryCalls, "rewrite-pnacl-library-calls",
+                "Rewrite PNaCl library calls to stable intrinsics",
+                false, false)
+
+bool RewritePNaClLibraryCalls::runOnModule(Module &M) {
+  TheModule = &M;
+  bool Changed = false;
+
+  for (Module::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) {

[Mark Seaborn, 2013/05/10 16:34:36]

Iterating through all calls is inefficient.  It would be better to get the
setjmp/longjmp globals and iterate through their uses.

You should also check that the functions aren't address-taken...

It's not legal for setjmp() to be address-taken:  "It is unspecified whether
setjmp() is a macro or a function."
(http://pubs.opengroup.org/onlinepubs/009696799/functions/setjmp.html)

However, I think it's legal to take the address of longjmp() since it's
described as a function
(http://pubs.opengroup.org/onlinepubs/009696799/functions/longjmp.html).

Maybe we could handle that case by creating a function that wraps the longjmp
intrinsic?

[eliben, 2013/05/10 17:00:51]

Yeah, although given a larger amount of functions it will likely make the code
more complex. I'll experiment to see if it's not too bad.
Good point.
Yes, the C++11 standard explicitly says:

" Names which are defined as macros in C shall be defined as macros in the C++
standard library, even if C grants license for implementation as functions. [
Note: The names defined as macros in C include the following: assert, offsetof,
setjmp, va_arg, va_end, and va_start. — end note ]"
Where should this function be created? I wonder if we can just write it in IR
somewhere in pnacl/support/bitcode and then it will just do the cast and call
the intrinsic. The pass will then just have to replace one call by another.

[eliben, 2013/05/10 23:39:52]

Done.

+    for (Function::iterator FI = MI->begin(), FE = MI->end(); FI != FE; ++FI) {
+      for (BasicBlock::iterator I = FI->begin(), E = FI->end(); I != E;) {
+        Instruction *Inst = I++;
+
+        if (CallInst *Call = dyn_cast<CallInst>(Inst)) {
+          StringRef CalleeName = Call->getCalledFunction()->getName();

[Mark Seaborn, 2013/05/10 16:34:36]

I think you need to check the linkage type too.  These could be internal
functions called "setjmp" or "longjmp" (maybe static) if we haven't stripped the
internal names yet.

[eliben, 2013/05/10 23:39:52]

Done.

+
+          if (CalleeName == "setjmp") {
+            rewriteSetjmp(Call);
+            Changed = true;
+          } else if (CalleeName == "longjmp") {
+            rewriteLongjmp(Call);
+            Changed = true;
+          }
+        }
+      }
+    }
+  }
+
+  return Changed;
+}
+
+void RewritePNaClLibraryCalls::rewriteSetjmp(CallInst *Call) {
+  FunctionType *FTy = Call->getCalledFunction()->getFunctionType();
+
+  // Sanity check on the setjmp call.
+  if (FTy->getNumParams() != 1 ||
+      !FTy->getReturnType()->isIntegerTy() ||
+      !FTy->getParamType(0)->isPointerTy()) {
+    report_fatal_error(Twine("Wrong signature of function ") +
+                       Call->getCalledFunction()->getName());
+  }
+  const DebugLoc &DLoc = Call->getDebugLoc();
+
+  // Cast the jmp_buf argument to i8* since this is what the intrinsic expects.
+  Type *I8Ptr = Type::getInt8Ty(TheModule->getContext())->getPointerTo();

[Mark Seaborn, 2013/05/10 16:34:36]

You could use:
NaclSetjmpFunc->getFunctionType()->getParamType(0)
to avoid fixing the knowledge of the arg type being i8* here.

+  BitCastInst *JmpBufCast = new BitCastInst(Call->getArgOperand(0), I8Ptr,
+                                            "jmp_buf_i8", Call);
+  JmpBufCast->setDebugLoc(DLoc);
+
+  // Find the intrinsic function and call it.
+  Function *NaclSetjmpFunc = Intrinsic::getDeclaration(TheModule,

[Mark Seaborn, 2013/05/10 16:34:36]

Maybe "Nacl" -> "NaCl"?

[eliben, 2013/05/10 23:39:52]

Done.

+                                                       Intrinsic::nacl_setjmp);
+  SmallVector<Value *, 2> Args;
+  Args.push_back(JmpBufCast);
+  CallInst *NaclSetjmpCall = CallInst::Create(NaclSetjmpFunc, Args, "", Call);
+  NaclSetjmpCall->setDebugLoc(DLoc);

[Mark Seaborn, 2013/05/10 16:34:36]

Use takeName() too

[eliben, 2013/05/10 23:39:52]

Done.

+
+  // Replace the original call.
+  Call->replaceAllUsesWith(NaclSetjmpCall);
+  Call->eraseFromParent();
+}
+
+void RewritePNaClLibraryCalls::rewriteLongjmp(CallInst *Call) {
+  FunctionType *FTy = Call->getCalledFunction()->getFunctionType();
+
+  // Sanity check on the longjmp call.
+  if (FTy->getNumParams() != 2 ||
+      !FTy->getReturnType()->isVoidTy() ||
+      !FTy->getParamType(0)->isPointerTy()) {
+    report_fatal_error(Twine("Wrong signature of function ") +
+                       Call->getCalledFunction()->getName());
+  }
+  const DebugLoc &DLoc = Call->getDebugLoc();
+
+  // Cast the jmp_buf argument to i8* since this is what the intrinsic expects.
+  Type *I8Ptr = Type::getInt8Ty(TheModule->getContext())->getPointerTo();
+  BitCastInst *JmpBufCast = new BitCastInst(Call->getArgOperand(0), I8Ptr,
+                                            "jmp_buf_i8", Call);
+  JmpBufCast->setDebugLoc(DLoc);
+
+  // Find the intrinsic function and call it.
+  Function *NaclLongjmpFunc = Intrinsic::getDeclaration(TheModule,
+                                                        Intrinsic::nacl_longjmp);

[Mark Seaborn, 2013/05/10 16:34:36]

Line >80 chars

[eliben, 2013/05/10 23:39:52]

Done.

+  SmallVector<Value *, 2> Args;
+  Args.push_back(JmpBufCast);
+  Args.push_back(Call->getArgOperand(1));
+  CallInst *NaclLongjmpCall = CallInst::Create(NaclLongjmpFunc, Args, "", Call);
+  NaclLongjmpCall->setDebugLoc(DLoc);
+
+  // Remove the original call. There's no need for RAUW because longjmp
+  // returns void.
+  Call->eraseFromParent();
+}
+
+ModulePass *llvm::createRewritePNaClLibraryCallsPass() {
+  return new RewritePNaClLibraryCalls();
+}