[PATCH] Implement NaCl sandboxing of function calls
Mark Seaborn
mseaborn at chromium.org
Mon Mar 10 14:19:33 PDT 2014
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:166
@@ +165,3 @@
+ return;
+ } else if (isCall(Inst.getOpcode(), &IsIndirectCall)) {
+ Call = Inst;
----------------
Nit: drop the "else" to be consistent with the rest of the function -- it isn't needed if the "if" block returns.
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:45
@@ -43,1 +44,3 @@
private:
+ MCInst Call;
+ bool SandboxingCall;
----------------
Please comment these three fields. "Call" and "IsIndirectCall" are only defined if SandboxingCall is true -- so please comment that, and put SandboxingCall first.
================
Comment at: test/MC/Mips/nacl-mask.s:12
@@ -11,2 +11,3 @@
+ .align 4
test1:
----------------
Is the reason for adding this that EmitBundleLock(true) complains if an alignment hasn't been set already? Or does the test work without adding this .align?
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:167
@@ +166,3 @@
+ } else if (isCall(Inst.getOpcode(), &IsIndirectCall)) {
+ Call = Inst;
+ SandboxingCall = true;
----------------
Maybe comment this block: "Save up the call to emit after we see the branch delay instruction that follows"?
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:46
@@ +45,3 @@
+ MCInst Call;
+ bool SandboxingCall;
+ bool IsIndirectCall;
----------------
Maybe something like "Pending" or "Saved" rather than "Sandboxing" would better suggest what's going on here?
http://llvm-reviews.chandlerc.com/D3032
More information about the llvm-commits
mailing list