Merge pull request #1175 from undingen/checkAndThrowCAPIException_size

checkAndThrowCAPIException needs to do a 32bit comparison in some cases

src/asm_writing/assembler.cpp

@@ -76,14 +76,15 @@ GenericRegister GenericRegister::fromDwarf(int dwarf_regnum) {
 
 
 
-void Assembler::emitArith(Immediate imm, Register r, int opcode) {
+void Assembler::emitArith(Immediate imm, Register r, int opcode, MovType type) {
     // assert(r != RSP && "This breaks unwinding, please don't use.");
 
     int64_t amount = imm.val;
     RELEASE_ASSERT(fitsInto<int32_t>(amount), "");
     assert(0 <= opcode && opcode < 8);
 
-    int rex = REX_W;
+    assert(type == MovType::Q || type == MovType::L);
+    int rex = type == MovType::Q ? REX_W : 0;
 
     int reg_idx = r.regnum;
     if (reg_idx >= 8) {
@@ -91,7 +92,8 @@ void Assembler::emitArith(Immediate imm, Register r, int opcode) {
         reg_idx -= 8;
     }
 
-    emitRex(rex);
+    if (rex)
+        emitRex(rex);
     if (-0x80 <= amount && amount < 0x80) {
         emitByte(0x83);
         emitModRM(0b11, opcode, reg_idx);
@@ -872,8 +874,8 @@ void Assembler::cmp(Register reg1, Register reg2) {
     emitModRM(0b11, reg1_idx, reg2_idx);
 }
 
-void Assembler::cmp(Register reg, Immediate imm) {
-    emitArith(imm, reg, OPCODE_CMP);
+void Assembler::cmp(Register reg, Immediate imm, MovType type) {
+    emitArith(imm, reg, OPCODE_CMP, type);
 }
 
 void Assembler::cmp(Indirect mem, Immediate imm, MovType type) {

src/asm_writing/assembler.h

@@ -87,7 +87,7 @@ private:
     void emitRex(uint8_t rex);
     void emitModRM(uint8_t mod, uint8_t reg, uint8_t rm);
     void emitSIB(uint8_t scalebits, uint8_t index, uint8_t base);
-    void emitArith(Immediate imm, Register reg, int opcode);
+    void emitArith(Immediate imm, Register reg, int opcode, MovType type = MovType::Q);
     void emitArith(Immediate imm, Indirect mem, int opcode);
 
     int getModeFromOffset(int offset, int reg_idx) const;
@@ -169,8 +169,9 @@ public:
     void leave();
 
     void cmp(Register reg1, Register reg2);
-    void cmp(Register reg, Immediate imm);
+    void cmp(Register reg, Immediate imm, MovType type = MovType::Q);
     void cmp(Indirect mem, Immediate imm, MovType type = MovType::Q);
+    void cmpl(Register reg, Immediate imm) { return cmp(reg, imm, MovType::L); }
     void cmpl(Indirect mem, Immediate imm) { return cmp(mem, imm, MovType::L); }
     void cmp(Indirect mem, Register reg);
 

src/asm_writing/rewriter.cpp

@@ -1898,21 +1898,22 @@ void Rewriter::_allocateAndCopyPlus1(RewriterVar* result, RewriterVar* first_ele
     assertConsistent();
 }
 
-void Rewriter::checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val) {
+void Rewriter::checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val, assembler::MovType type) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
-    addAction([=]() { this->_checkAndThrowCAPIException(r, exc_val); }, { r }, ActionType::MUTATION);
+    addAction([=]() { this->_checkAndThrowCAPIException(r, exc_val, type); }, { r }, ActionType::MUTATION);
 }
 
-void Rewriter::_checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val) {
+void Rewriter::_checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val, assembler::MovType type) {
     if (LOG_IC_ASSEMBLY)
         assembler->comment("_checkAndThrowCAPIException");
 
     assembler::Register var_reg = r->getInReg();
-    if (exc_val == 0)
+    if (exc_val == 0) {
+        RELEASE_ASSERT(type == assembler::MovType::Q, "unimplemented");
         assembler->test(var_reg, var_reg);
-    else
-        assembler->cmp(var_reg, assembler::Immediate(exc_val));
+    } else
+        assembler->cmp(var_reg, assembler::Immediate(exc_val), type);
 
     _setupCall(false, RewriterVar::SmallVector(), RewriterVar::SmallVector());
     {

src/asm_writing/rewriter.h

@@ -489,7 +489,7 @@ protected:
     int _allocate(RewriterVar* result, int n);
     void _allocateAndCopy(RewriterVar* result, RewriterVar* array, int n);
     void _allocateAndCopyPlus1(RewriterVar* result, RewriterVar* first_elem, RewriterVar* rest, int n_rest);
-    void _checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val);
+    void _checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val, assembler::MovType size);
 
     // The public versions of these are in RewriterVar
     void _addGuard(RewriterVar* var, RewriterVar* val_constant);
@@ -596,7 +596,9 @@ public:
     RewriterVar* allocateAndCopyPlus1(RewriterVar* first_elem, RewriterVar* rest, int n_rest);
 
     // This emits `if (r == exc_val) throwCAPIException()`
-    void checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val = 0);
+    // type should be either MovType::Q if you want a 64bit comparison or MovType::L for a 32bit comparison.
+    void checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val, assembler::MovType type);
+    void checkAndThrowCAPIException(RewriterVar* r) { checkAndThrowCAPIException(r, 0, assembler::MovType::Q); }
 
     void abort();
 

src/runtime/objmodel.cpp

@@ -2960,7 +2960,7 @@ bool dataDescriptorSetSpecialCases(Box* obj, STOLEN(Box*) val, Box* descr, Setat
                 /* has_side_effects */ true, (void*)getset_descr->set, args);
 
             if (descr->cls == capi_getset_cls)
-                rewrite_args->rewriter->checkAndThrowCAPIException(r_rtn, -1);
+                rewrite_args->rewriter->checkAndThrowCAPIException(r_rtn, -1, assembler::MovType::L);
 
             rewrite_args->out_success = true;
         }
@@ -3506,7 +3506,7 @@ BoxedInt* lenInternal(Box* obj, LenRewriteArgs* rewrite_args) noexcept(S == CAPI
 
             // Some CPython code seems to think that any negative return value means an exception,
             // but the docs say -1. TODO it would be nice to just handle any negative value.
-            rewrite_args->rewriter->checkAndThrowCAPIException(r_n, -1);
+            rewrite_args->rewriter->checkAndThrowCAPIException(r_n, -1, assembler::MovType::Q);
 
             RewriterVar* r_r = rewrite_args->rewriter->call(false, (void*)boxInt, r_n)->setType(RefType::OWNED);
 
@@ -5761,7 +5761,7 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
                     // support calling a RewriterVar (can only call fixed function addresses).
                     r_sqm->addAttrGuard(offsetof(PySequenceMethods, sq_contains), (intptr_t)sqm->sq_contains);
                     RewriterVar* r_b = rewrite_args->rewriter->call(true, (void*)sqm->sq_contains, r_rhs, r_lhs);
-                    rewrite_args->rewriter->checkAndThrowCAPIException(r_b, -1);
+                    rewrite_args->rewriter->checkAndThrowCAPIException(r_b, -1, assembler::MovType::L);
 
                     // This could be inlined:
                     RewriterVar* r_r;

src/runtime/types.cpp

@@ -1379,7 +1379,7 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
                     = rewrite_args->rewriter->call(true, (void*)tpinit, r_made, rewrite_args->arg2, rewrite_args->arg3);
 
                 assert(S == CXX && "this need to be converted");
-                rewrite_args->rewriter->checkAndThrowCAPIException(r_err, -1);
+                rewrite_args->rewriter->checkAndThrowCAPIException(r_err, -1, assembler::MovType::L);
             }
         }
     } else {

test/extra/cffi_test.py

-# expected: reffail
 import os, sys, subprocess, shutil
 sys.path.append(os.path.dirname(__file__) + "/../lib")