Skip to content

Commit 28c0c4b

Browse files
undingenundingen
committed
Add signal support
We check for signals on most calls to runtime functions in the llvm tier and the bjit and inside the interpreter on some additional places, because checking on every bytecode is a too large slowdown (>10%).
1 parent f11cb87 commit 28c0c4b

File tree

17 files changed

+300
-53
lines changed

17 files changed

+300
-53
lines changed

‎from_cpython/Lib/test/test_signal.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
# expected: fail
21
import unittest
32
from test import test_support
43
from contextlib import closing
@@ -80,7 +79,8 @@ def run_test(self):
8079
# don't worry about re-setting the default handlers.
8180
signal.signal(signal.SIGHUP, self.handlerA)
8281
signal.signal(signal.SIGUSR1, self.handlerB)
83-
signal.signal(signal.SIGUSR2, signal.SIG_IGN)
82+
# Pyston change: pyston uses SIGUSR2 internally
83+
# signal.signal(signal.SIGUSR2, signal.SIG_IGN)
8484
signal.signal(signal.SIGALRM, signal.default_int_handler)
8585

8686
# Variables the signals will modify:
@@ -117,9 +117,11 @@ def run_test(self):
117117
if test_support.verbose:
118118
print "HandlerBCalled exception caught"
119119

120-
child = ignoring_eintr(subprocess.Popen, ['kill', '-USR2', str(pid)])
121-
if child:
122-
self.wait(child) # Nothing should happen.
120+
121+
# Pyston change: pyston uses SIGUSR2 internally
122+
# child = ignoring_eintr(subprocess.Popen, ['kill', '-USR2', str(pid)])
123+
# if child:
124+
# self.wait(child) # Nothing should happen.
123125

124126
try:
125127
signal.alarm(1)

‎from_cpython/Modules/signalmodule.c‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,9 @@ initsignal(void)
626626
old_siginthandler = PyOS_setsig(SIGINT, signal_handler);
627627
}
628628

629+
// Pyston change: let the GC scan the handlers
630+
PyGC_AddPotentialRoot(Handlers, sizeof(Handlers));
631+
629632
#ifdef SIGHUP
630633
x = PyInt_FromLong(SIGHUP);
631634
PyDict_SetItemString(d, "SIGHUP", x);
@@ -895,10 +898,6 @@ PyErr_CheckSignals(void)
895898
if (!is_tripped)
896899
return 0;
897900

898-
// Pyston change:
899-
Py_FatalError("TODO");
900-
901-
#if 0
902901
int i;
903902
PyObject *f;
904903

@@ -943,7 +942,6 @@ PyErr_CheckSignals(void)
943942
Py_DECREF(result);
944943
}
945944
}
946-
#endif
947945

948946
return 0;
949947
}

‎src/codegen/ast_interpreter.cpp‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -922,43 +922,66 @@ Value ASTInterpreter::visit_stmt(AST_stmt* node) {
922922
printf("\n");
923923
}
924924

925+
Value rtn;
925926
switch (node->type) {
926927
case AST_TYPE::Assert:
927-
return visit_assert((AST_Assert*)node);
928+
rtn = visit_assert((AST_Assert*)node);
929+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
930+
break;
928931
case AST_TYPE::Assign:
929-
return visit_assign((AST_Assign*)node);
932+
rtn = visit_assign((AST_Assign*)node);
933+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
934+
break;
930935
case AST_TYPE::Delete:
931-
return visit_delete((AST_Delete*)node);
936+
rtn = visit_delete((AST_Delete*)node);
937+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
938+
break;
932939
case AST_TYPE::Exec:
933-
return visit_exec((AST_Exec*)node);
940+
rtn = visit_exec((AST_Exec*)node);
941+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
942+
break;
934943
case AST_TYPE::Expr:
935944
// docstrings are str constant expression statements.
936945
// ignore those while interpreting.
937-
if ((((AST_Expr*)node)->value)->type != AST_TYPE::Str)
938-
return visit_expr((AST_Expr*)node);
946+
if ((((AST_Expr*)node)->value)->type != AST_TYPE::Str) {
947+
rtn = visit_expr((AST_Expr*)node);
948+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
949+
}
939950
break;
940951
case AST_TYPE::Pass:
941-
return Value(); // nothing todo
952+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
953+
break; // nothing todo
942954
case AST_TYPE::Print:
943-
return visit_print((AST_Print*)node);
955+
rtn = visit_print((AST_Print*)node);
956+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
957+
break;
944958
case AST_TYPE::Raise:
945-
return visit_raise((AST_Raise*)node);
959+
rtn = visit_raise((AST_Raise*)node);
960+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
961+
break;
946962
case AST_TYPE::Return:
947-
return visit_return((AST_Return*)node);
963+
rtn = visit_return((AST_Return*)node);
964+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
965+
break;
948966
case AST_TYPE::Global:
949-
return visit_global((AST_Global*)node);
967+
rtn = visit_global((AST_Global*)node);
968+
ASTInterpreterJitInterface::pendingCallsCheckHelper();
969+
break;
950970

951971
// pseudo
952972
case AST_TYPE::Branch:
953-
return visit_branch((AST_Branch*)node);
973+
rtn = visit_branch((AST_Branch*)node);
974+
break;
954975
case AST_TYPE::Jump:
955-
return visit_jump((AST_Jump*)node);
976+
rtn = visit_jump((AST_Jump*)node);
977+
break;
956978
case AST_TYPE::Invoke:
957-
return visit_invoke((AST_Invoke*)node);
979+
rtn = visit_invoke((AST_Invoke*)node);
980+
break;
958981
default:
959982
RELEASE_ASSERT(0, "not implemented");
960983
};
961-
return Value();
984+
return rtn;
962985
}
963986

964987
Value ASTInterpreter::visit_return(AST_Return* node) {
@@ -1652,6 +1675,11 @@ Box* ASTInterpreterJitInterface::landingpadHelper(void* _interpreter) {
16521675
return rtn;
16531676
}
16541677

1678+
void ASTInterpreterJitInterface::pendingCallsCheckHelper() {
1679+
if (unlikely(_pendingcalls_to_do))
1680+
makePendingCalls();
1681+
}
1682+
16551683
Box* ASTInterpreterJitInterface::setExcInfoHelper(void* _interpreter, Box* type, Box* value, Box* traceback) {
16561684
ASTInterpreter* interpreter = (ASTInterpreter*)_interpreter;
16571685
interpreter->getFrameInfo()->exc = ExcInfo(type, value, traceback);

‎src/codegen/ast_interpreter.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ struct ASTInterpreterJitInterface {
4545
static Box* derefHelper(void* interp, InternedString s);
4646
static Box* doOSRHelper(void* interp, AST_Jump* node);
4747
static Box* landingpadHelper(void* interp);
48+
static void pendingCallsCheckHelper();
4849
static Box* setExcInfoHelper(void* interp, Box* type, Box* value, Box* traceback);
4950
static void setLocalClosureHelper(void* interp, long vreg, InternedString id, Box* v);
5051
static Box* uncacheExcInfoHelper(void* interp);

‎src/codegen/baseline_jit.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,10 @@ void JitFragmentWriter::emitOSRPoint(AST_Jump* node) {
474474
addAction([=]() { _emitOSRPoint(result, node_var); }, { result, node_var, getInterp() }, ActionType::NORMAL);
475475
}
476476

477+
void JitFragmentWriter::emitPendingCallsCheck() {
478+
call(false, (void*)ASTInterpreterJitInterface::pendingCallsCheckHelper);
479+
}
480+
477481
void JitFragmentWriter::emitPrint(RewriterVar* dest, RewriterVar* var, bool nl) {
478482
if (!dest)
479483
dest = call(false, (void*)getSysStdout);
@@ -696,12 +700,17 @@ RewriterVar* JitFragmentWriter::emitPPCall(void* func_addr, llvm::ArrayRef<Rewri
696700
RewriterVar* obj_cls_var = result->getAttr(offsetof(Box, cls));
697701
addAction([=]() { _emitRecordType(type_recorder_var, obj_cls_var); }, { type_recorder_var, obj_cls_var },
698702
ActionType::NORMAL);
703+
704+
emitPendingCallsCheck();
699705
return result;
700706
}
707+
emitPendingCallsCheck();
701708
return result;
702709
#else
703710
assert(args_vec.size() < 7);
704-
return call(false, func_addr, args_vec);
711+
RewriterVar* result = call(false, func_addr, args_vec);
712+
emitPendingCallsCheck();
713+
return result;
705714
#endif
706715
}
707716

‎src/codegen/baseline_jit.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ class JitFragmentWriter : public Rewriter {
246246
void emitExec(RewriterVar* code, RewriterVar* globals, RewriterVar* locals, FutureFlags flags);
247247
void emitJump(CFGBlock* b);
248248
void emitOSRPoint(AST_Jump* node);
249+
void emitPendingCallsCheck();
249250
void emitPrint(RewriterVar* dest, RewriterVar* var, bool nl);
250251
void emitRaise0();
251252
void emitRaise3(RewriterVar* arg0, RewriterVar* arg1, RewriterVar* arg2);

‎src/codegen/entry.cpp‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -378,16 +378,6 @@ static void handle_sigprof_investigate_stattimer(int signum) {
378378
}
379379
#endif
380380

381-
static void handle_sigint(int signum) {
382-
assert(signum == SIGINT);
383-
// TODO: this should set a flag saying a KeyboardInterrupt is pending.
384-
// For now, just call abort(), so that we get a traceback at least.
385-
fprintf(stderr, "SIGINT!\n");
386-
joinRuntime();
387-
Stats::dump(false);
388-
abort();
389-
}
390-
391381
void initCodegen() {
392382
llvm::InitializeNativeTarget();
393383
llvm::InitializeNativeTargetAsmPrinter();
@@ -481,9 +471,8 @@ void initCodegen() {
481471

482472
setupRuntime();
483473

484-
// signal(SIGFPE, &handle_sigfpe);
485-
signal(SIGUSR1, &handle_sigusr1);
486-
signal(SIGINT, &handle_sigint);
474+
// signal(SIGFPE, &handle_sigfpe);
475+
// signal(SIGUSR1, &handle_sigusr1);
487476

488477
#if ENABLE_SAMPLING_PROFILER
489478
struct itimerval prof_timer;

‎src/codegen/irgen/irgenerator.cpp‎

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -347,11 +347,53 @@ class IREmitterImpl : public IREmitter {
347347
llvm::BasicBlock*& curblock;
348348
IRGenerator* irgenerator;
349349

350+
void emitPendingCallsCheck(llvm::BasicBlock* exc_dest) {
351+
auto&& builder = *getBuilder();
352+
353+
llvm::GlobalVariable* pendingcalls_to_do_gv = g.cur_module->getGlobalVariable("_pendingcalls_to_do");
354+
if (!pendingcalls_to_do_gv) {
355+
static_assert(sizeof(_pendingcalls_to_do) == 4, "");
356+
pendingcalls_to_do_gv = new llvm::GlobalVariable(
357+
*g.cur_module, g.i32, false, llvm::GlobalValue::ExternalLinkage, 0, "_pendingcalls_to_do");
358+
pendingcalls_to_do_gv->setAlignment(4);
359+
}
360+
361+
llvm::BasicBlock* cur_block = builder.GetInsertBlock();
362+
363+
llvm::BasicBlock* pendingcalls_set = createBasicBlock("_pendingcalls_set");
364+
pendingcalls_set->moveAfter(cur_block);
365+
llvm::BasicBlock* join_block = createBasicBlock("continue_after_pendingcalls_check");
366+
join_block->moveAfter(pendingcalls_set);
367+
368+
llvm::Value* pendingcalls_to_do_val = builder.CreateLoad(pendingcalls_to_do_gv, true /* volatile */);
369+
llvm::Value* is_zero
370+
= builder.CreateICmpEQ(pendingcalls_to_do_val, getConstantInt(0, pendingcalls_to_do_val->getType()));
371+
372+
llvm::Metadata* md_vals[]
373+
= { llvm::MDString::get(g.context, "branch_weights"), llvm::ConstantAsMetadata::get(getConstantInt(1000)),
374+
llvm::ConstantAsMetadata::get(getConstantInt(1)) };
375+
llvm::MDNode* branch_weights = llvm::MDNode::get(g.context, llvm::ArrayRef<llvm::Metadata*>(md_vals));
376+
377+
378+
builder.CreateCondBr(is_zero, join_block, pendingcalls_set, branch_weights);
379+
{
380+
setCurrentBasicBlock(pendingcalls_set);
381+
382+
if (exc_dest) {
383+
builder.CreateInvoke(g.funcs.makePendingCalls, join_block, exc_dest);
384+
} else {
385+
builder.CreateCall(g.funcs.makePendingCalls);
386+
builder.CreateBr(join_block);
387+
}
388+
}
389+
390+
cur_block = join_block;
391+
setCurrentBasicBlock(join_block);
392+
}
393+
350394
llvm::CallSite emitCall(const UnwindInfo& unw_info, llvm::Value* callee, const std::vector<llvm::Value*>& args,
351395
ExceptionStyle target_exception_style) {
352-
llvm::Value* stmt = unw_info.current_stmt ? embedRelocatablePtr(unw_info.current_stmt, g.llvm_aststmt_type_ptr)
353-
: getNullPtr(g.llvm_aststmt_type_ptr);
354-
getBuilder()->CreateStore(stmt, irstate->getStmtVar());
396+
emitSetCurrentStmt(unw_info.current_stmt);
355397

356398
if (target_exception_style == CXX && (unw_info.hasHandler() || irstate->getExceptionStyle() == CAPI)) {
357399
// Create the invoke:
@@ -375,9 +417,13 @@ class IREmitterImpl : public IREmitter {
375417
// Normal case:
376418
getBuilder()->SetInsertPoint(normal_dest);
377419
curblock = normal_dest;
420+
emitPendingCallsCheck(exc_dest);
378421
return rtn;
379422
} else {
423+
380424
llvm::CallInst* cs = getBuilder()->CreateCall(callee, args);
425+
if (target_exception_style == CXX)
426+
emitPendingCallsCheck(NULL);
381427
return cs;
382428
}
383429
}
@@ -479,6 +525,15 @@ class IREmitterImpl : public IREmitter {
479525
return llvm::BasicBlock::Create(g.context, name, irstate->getLLVMFunction());
480526
}
481527

528+
// Our current frame introspection approach requires that we update the currently executed stmt before doing a call
529+
// to a function which could throw an exception, inspect the python call frame,...
530+
// Only patchpoint don't need to set the current statement because the stmt will be inluded in the stackmap args.
531+
void emitSetCurrentStmt(AST_stmt* stmt) {
532+
getBuilder()->CreateStore(stmt ? embedRelocatablePtr(stmt, g.llvm_aststmt_type_ptr)
533+
: getNullPtr(g.llvm_aststmt_type_ptr),
534+
irstate->getStmtVar());
535+
}
536+
482537
llvm::Value* createCall(const UnwindInfo& unw_info, llvm::Value* callee, const std::vector<llvm::Value*>& args,
483538
ExceptionStyle target_exception_style = CXX) override {
484539
#ifndef NDEBUG
@@ -2137,7 +2192,7 @@ class IRGeneratorImpl : public IRGenerator {
21372192

21382193
// Don't call deinitFrame when this is a OSR function because the interpreter will call it
21392194
if (!irstate->getCurFunction()->entry_descriptor)
2140-
emitter.createCall(unw_info, g.funcs.deinitFrame, irstate->getFrameInfoVar());
2195+
emitter.getBuilder()->CreateCall(g.funcs.deinitFrame, irstate->getFrameInfoVar());
21412196

21422197
for (auto& p : symbol_table) {
21432198
p.second->decvref(emitter);
@@ -2878,9 +2933,10 @@ class IRGeneratorImpl : public IRGenerator {
28782933
}
28792934

28802935
void doSafePoint(AST_stmt* next_statement) override {
2881-
// Unwind info is always needed in allowGLReadPreemption if it has any chance of
2882-
// running arbitrary code like finalizers.
2883-
emitter.createCall(UnwindInfo(next_statement, NULL), g.funcs.allowGLReadPreemption);
2936+
// We need to setup frame introspection by updating the current stmt because we can run can run arbitrary code
2937+
// like finalizers inside allowGLReadPreemption.
2938+
emitter.emitSetCurrentStmt(next_statement);
2939+
emitter.getBuilder()->CreateCall(g.funcs.allowGLReadPreemption);
28842940
}
28852941

28862942
// Create a (or reuse an existing) block that will catch a CAPI exception, and then forward
@@ -2902,16 +2958,17 @@ class IRGeneratorImpl : public IRGenerator {
29022958
assert(!phi_node);
29032959
phi_node = emitter.getBuilder()->CreatePHI(g.llvm_aststmt_type_ptr, 0);
29042960

2905-
emitter.createCall(UnwindInfo(current_stmt, NULL), g.funcs.caughtCapiException,
2906-
{ phi_node, embedRelocatablePtr(irstate->getSourceInfo(), g.i8_ptr) });
2961+
emitter.emitSetCurrentStmt(current_stmt);
2962+
emitter.getBuilder()->CreateCall(g.funcs.caughtCapiException,
2963+
{ phi_node, embedRelocatablePtr(irstate->getSourceInfo(), g.i8_ptr) });
29072964

29082965
if (!final_dest) {
29092966
// Propagate the exception out of the function:
29102967
if (irstate->getExceptionStyle() == CXX) {
29112968
emitter.getBuilder()->CreateCall(g.funcs.reraiseCapiExcAsCxx);
29122969
emitter.getBuilder()->CreateUnreachable();
29132970
} else {
2914-
emitter.createCall(UnwindInfo(current_stmt, NULL), g.funcs.deinitFrame, irstate->getFrameInfoVar());
2971+
emitter.getBuilder()->CreateCall(g.funcs.deinitFrame, irstate->getFrameInfoVar());
29152972
emitter.getBuilder()->CreateRet(getNullPtr(g.llvm_value_type_ptr));
29162973
}
29172974
} else {

‎src/codegen/runtime_hooks.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ void initGlobalFuncs(GlobalState& g) {
200200
GET(createSet);
201201
GET(initFrame);
202202
GET(deinitFrame);
203+
GET(makePendingCalls);
203204

204205
GET(getattr);
205206
GET(getattr_capi);

‎src/codegen/runtime_hooks.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ struct GlobalFuncs {
3434

3535
llvm::Value* boxInt, *unboxInt, *boxFloat, *unboxFloat, *createFunctionFromMetadata, *getFunctionMetadata,
3636
*boxInstanceMethod, *boxBool, *unboxBool, *createTuple, *createDict, *createList, *createSlice,
37-
*createUserClass, *createClosure, *createGenerator, *createSet, *initFrame, *deinitFrame;
37+
*createUserClass, *createClosure, *createGenerator, *createSet, *initFrame, *deinitFrame, *makePendingCalls;
3838
llvm::Value* getattr, *getattr_capi, *setattr, *delattr, *delitem, *delGlobal, *nonzero, *binop, *compare,
3939
*augbinop, *unboxedLen, *getitem, *getitem_capi, *getclsattr, *getGlobal, *setitem, *unaryop, *import,
4040
*importFrom, *importStar, *repr, *exceptionMatches, *yield, *getiterHelper, *hasnext, *setGlobal, *apply_slice;

0 commit comments

Comments
 (0)