Fix problems with sandboxing and the ARM integrated assembler.

src/IceAssemblerARM32.cpp

 //===- subzero/src/IceAssemblerARM32.cpp - Assembler for ARM32 --*- C++ -*-===//
 //
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 //
 // Modified by the Subzero authors.
 //
 //===----------------------------------------------------------------------===//
 //
@@ skipping 442 matching lines @@
   if (!BuildDefs::dump())
     return InstARM32::InstSize;
   Ostream &Str = Ctx->getStrEmit();
   IValueT Inst = Asm.load<IValueT>(position());
   Str << "\t"
       << "bl\t" << symbol(Ctx) << "\t@ .word "
       << llvm::format_hex_no_prefix(Inst, 8) << "\n";
   return InstARM32::InstSize;
 }
 
+void AssemblerARM32::padWithNop(intptr_t Padding) {
+  constexpr intptr_t InstWidth = sizeof(IValueT);
+  assert(Padding % InstWidth == 0 &&
+         "Padding not mulitple of instruction size");

[Jim Stichnoth, 2015/12/08 19:54:58]

multiple

[Karl, 2015/12/08 20:49:36]

Done.

+  for (intptr_t i = 0; i < Padding; i += InstWidth)
+    nop();
+}
+
 BlRelocatableFixup *
 AssemblerARM32::createBlFixup(const ConstantRelocatable *BlTarget) {
   BlRelocatableFixup *F =
       new (allocate<BlRelocatableFixup>()) BlRelocatableFixup();
   F->set_kind(llvm::ELF::R_ARM_CALL);
   F->set_value(BlTarget);
   Buffer.installFixup(F);
   return F;
 }
 
@@ skipping 171 matching lines @@
   // TODO(kschimpf): Handle far jumps.
   constexpr const char *BranchName = "b";
   if (L->isBound()) {
     const int32_t Dest = L->getPosition() - Buffer.size();
     emitType05(Cond, Dest, Link, BranchName);
     return;
   }
   const IOffsetT Position = Buffer.size();
   // Use the offset field of the branch instruction for linking the sites.
   emitType05(Cond, L->getEncodedPosition(), Link, BranchName);
-  if (!needsTextFixup())
+  if (!needsTextFixup() && !getPreliminary())

[Jim Stichnoth, 2015/12/08 20:52:27]

This is clearly a problem that affects all assemblers, and so it would be great
to factor it up to the base Assembler class.

Can you try modifying Label::linkTo() to also take a const Assembler * argument,
and then do an early return if Assembler::getPreliminary() is true?  Then the
getPreliminary() checks are unneeded for the target-specific assemblers.

The same change should be applied to the x86 nearLinkTo() method.

I think this is simple enough that it could be part of this CL, or it could be a
separate CL if you like.

[Karl, 2015/12/08 21:38:11]

Moved check inside linkTo() and nearLinkTo(). Code now reads like there is a
single pass, even if there isn't.

     L->linkTo(Position);
 }
 
 void AssemblerARM32::emitCompareOp(CondARM32::Cond Cond, IValueT Opcode,
                                    const Operand *OpRn, const Operand *OpSrc1,
                                    const char *InstName) {
   // XXX (register)
   //   XXX<c> <Rn>, <Rm>{, <shift>}
   //
   // ccccyyyxxxx1nnnn0000iiiiitt0mmmm where cccc=Cond, nnnn=Rn, mmmm=Rm, iiiii
@@ skipping 600 matching lines @@
   // mmmm=Rm, iiii defines shift constant, and tt=ShiftKind.
   constexpr const char *MvnName = "mvn";
   IValueT Rd = encodeRegister(OpRd, "Rd", MvnName);
   constexpr IValueT MvnOpcode = B3 | B2 | B1 | B0; // i.e. 1111
   constexpr IValueT Rn = 0;
   constexpr bool SetFlags = false;
   emitType01(Cond, MvnOpcode, Rd, Rn, OpSrc, SetFlags, RdIsPcAndSetFlags,
              MvnName);
 }
 
+void AssemblerARM32::nop(CondARM32::Cond Cond) {
+  // NOP - Section A8.8.119, encoding A1:
+  //  nop<c>
+  //
+  // cccc0011001000001111000000000000 where cccc=Cond.
+  AssemblerBuffer::EnsureCapacity ensured(&Buffer);
+  const IValueT Encoding = (encodeCondition(Cond) << kConditionShift) |
+                           (B25 | B24 | B21 | B15 | B14 | B13 | B12);
+  emitInst(Encoding);
+}
+
 void AssemblerARM32::sbc(const Operand *OpRd, const Operand *OpRn,
                          const Operand *OpSrc1, bool SetFlags,
                          CondARM32::Cond Cond) {
   // SBC (register) - ARM section 18.8.162, encoding A1:
   //   sbc{s}<c> <Rd>, <Rn>, <Rm>{, <shift>}
   //
   // cccc0000110snnnnddddiiiiitt0mmmm where cccc=Cond, dddd=Rd, nnnn=Rn,
   // mmmm=Rm, iiiii=Shift, tt=ShiftKind, and s=SetFlags.
   //
   // SBC (Immediate) - ARM section A8.8.161, encoding A1:
@@ skipping 72 matching lines @@
     // double by using two (32 bit) store instructions.
     //
     // STR (immediate) - ARM section A8.8.207, encoding A1:
     //   str<c> <Rt>, [<Rn>{, #+/-<imm12>}]     ; p=1, w=0
     //   str<c> <Rt>, [<Rn>], #+/-<imm12>       ; p=1, w=1
     //   str<c> <Rt>, [<Rn>, #+/-<imm12>]!      ; p=0, w=1
     //
     // cccc010pu1w0nnnnttttiiiiiiiiiiii where cccc=Cond, tttt=Rt, nnnn=Rn,
     // iiiiiiiiiiii=imm12, u=1 if +.
     constexpr bool IsByte = false;
     return emitMemOp(Cond, IsLoad, IsByte, Rt, OpAddress, TInfo, StrName);

[John, 2015/12/08 19:45:04]

Optional: I know the language allows to return a void expression expression, but
would you mind doing:

  emitMemOp(...);
  return;

instead?

[Jim Stichnoth, 2015/12/08 19:54:58]

This pattern of returning a void expression was suggested (by me) and first
added in:

https://chromiumcodereview.appspot.com/1418523002/#msg4

After reacquainting myself with the issue, I agree that this pattern should
probably only be used with setNeedsTextFixup(), and would ultimately disappear
when full iasm support is there.

[Karl, 2015/12/08 19:58:35]

I mainly started doing this because it was much easier to write:

  if (...)
    return setNeedsTextFixup();

rather than:

  if (...) {
    setNeedsTextFixup();
    return;
  }

To be consistent, I started doing it whenever the last element was a call. I can
change it none the less.

[Karl, 2015/12/08 20:49:36]

Removing returns of this form.

-    return setNeedsTextFixup();
   }
   }
 }
 
 void AssemblerARM32::orr(const Operand *OpRd, const Operand *OpRn,
                          const Operand *OpSrc1, bool SetFlags,
                          CondARM32::Cond Cond) {
   // ORR (register) - ARM Section A8.8.123, encoding A1:
   //   orr{s}<c> <Rd>, <Rn>, <Rm>
   //
@@ skipping 244 matching lines @@
     // rr defined (RotationValue) rotate.
     constexpr IValueT UxtOpcode = B26 | B25 | B23 | B22 | B21 | B20;
     emitUxt(Cond, UxtOpcode, Rd, Rn, Rm, Rotation, UxtName);
     return;
   }
   }
 }
 
 } // end of namespace ARM32
 } // end of namespace Ice