[PATCH] Patch that aligns on bundle size the targets of indirect jumps, and that adds masks before indirect jumps and returns
Mark Seaborn
mseaborn at chromium.org
Wed Feb 26 10:20:34 PST 2014
Please make the commit message more informative -- it should mention NaCl. Something like:
"MIPS: Implement NaCl sandboxing of indirect jumps
* Align targets of indirect jumps to instruction bundle boundaries (in MI layer).
* Add masking instructions before indirect jumps (in MC layer)."
Then LGTM, thanks.
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsMCNaCl.h:20
@@ +19,3 @@
+
+// This function creates ELF Streamer for Mips NaCl.
+MCELFStreamer *createMipsNaClELFStreamer(MCContext &Context, MCAsmBackend &TAB,
----------------
Nit: "an MCELFStreamer"? (Using the class name is more greppable.)
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:10
@@ +9,3 @@
+//
+// This file implements ELF Streamer for Mips NaCl. It emits .o object files as
+// required by NaCl's SFI sandbox. It inserts address-masking instructions
----------------
Nit: "MCELFStreamer"
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:14
@@ +13,3 @@
+// bundle size all functions and all targets of indirect branches. It aligns
+// call instructions on bundle end, so that return address is always aligned on
+// bundle size.
----------------
Aligning call instructions isn't done yet, right?
================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:31
@@ +30,3 @@
+
+/// Extend the generic ELFStreamer class so that it can mask dangerous
+/// instructions.
----------------
Nit: MCELFStreamer
================
Comment at: test/MC/Mips/nacl-align.ll:52
@@ +51,3 @@
+
+; This test tests that block whose address is taken is bundle-aligned in NaCl.
+
----------------
Nit: "a block"
================
Comment at: test/MC/Mips/nacl-align.ll:98
@@ +97,3 @@
+}
+
+
----------------
Nit: remove empty lines at end of file
================
Comment at: test/MC/Mips/nacl-align.ll:88
@@ +87,3 @@
+
+; This test tests that NaCl functions are bundle-aligned.
+
----------------
You might want to put this one first, since it's the most fundamental. Also, it would avoid ".align 4" accidentally matching something in the previous function.
http://llvm-reviews.chandlerc.com/D2847
More information about the llvm-commits
mailing list