shared/runtime,py: Fix IRQ dispatch issues from code review.

- Add ishard parameter to mp_irq_prepare_handler() to handle soft/hard
  IRQ differently. Soft IRQ passes callable to scheduler as-is; hard
  IRQ wraps all callables for unified dispatch.
- Add generic callable wrapper (IRQ_CALLABLE sentinel) for bound methods
  and other callables that dispatches via mp_call_function_1().
- Add n_pos_args validation to @Native wrapper (must be exactly 1).
- Add scope_flags check to bytecode wrapper to reject generators.
- Remove unused mp_irq_get_extra() function.
- Fix reentrance guard in mp_obj_gen_resume_irq() to return error
  instead of raising exception (unsafe in hard IRQ with GC locked).

Signed-off-by: Andrew Leech <[email protected]>

Author: pi-anl

ports/stm32/timer.c

@@ -1524,7 +1524,8 @@ static mp_obj_t pyb_timer_callback(mp_obj_t self_in, mp_obj_t callback) {
     } else {
         __HAL_TIM_DISABLE_IT(&self->tim, TIM_IT_UPDATE);
         // Prepare callback: auto-instantiate generator functions, prime generator instances.
-        self->callback = mp_irq_prepare_handler(callback, self_in);
+        // For hard IRQ, wrap callables for unified dispatch; for soft IRQ, leave as-is.
+        self->callback = mp_irq_prepare_handler(callback, self_in, self->ishard);
         // start timer, so that it interrupts on overflow, but clear any
         // pending interrupts which may have been set by initializing it.
         __HAL_TIM_CLEAR_FLAG(&self->tim, TIM_IT_UPDATE);

py/bc.h

@@ -174,6 +174,7 @@
 #define MP_CODE_STATE_EXC_SP_IDX_IRQ_FUNC_BC  ((uint16_t)-2)  // Wrapped bytecode function
 #define MP_CODE_STATE_EXC_SP_IDX_IRQ_FUNC_NAT ((uint16_t)-3)  // Wrapped @native function
 #define MP_CODE_STATE_EXC_SP_IDX_IRQ_VIPER    ((uint16_t)-4)  // Wrapped @viper function
+#define MP_CODE_STATE_EXC_SP_IDX_IRQ_CALLABLE ((uint16_t)-5)  // Wrapped generic callable
 #endif
 
 // To convert mp_code_state_t.exc_sp_idx to/from a pointer to mp_exc_stack_t

py/objgenerator.c

@@ -262,8 +262,10 @@ mp_vm_return_kind_t mp_obj_gen_resume_irq(mp_obj_t self_in, mp_obj_t send_value,
     mp_obj_gen_instance_t *self = MP_OBJ_TO_PTR(self_in);
 
     // Reentrance guard (common to all types)
+    // Can't raise exception in hard IRQ context (GC locked), so return error
     if (self->pend_exc == MP_OBJ_NULL) {
-        mp_raise_ValueError(MP_ERROR_TEXT("handler already executing"));
+        *ret_val = MP_OBJ_FROM_PTR(&mp_const_GeneratorExit_obj);  // Use existing const
+        return MP_VM_RETURN_EXCEPTION;
     }
 
     uint16_t exc_sp_idx = self->code_state.exc_sp_idx;
@@ -350,6 +352,16 @@ mp_vm_return_kind_t mp_obj_gen_resume_irq(mp_obj_t self_in, mp_obj_t send_value,
         self->pend_exc = mp_const_none;
     #endif
 
+    } else if (exc_sp_idx == MP_CODE_STATE_EXC_SP_IDX_IRQ_CALLABLE) {
+        // Generic callable wrapper: dispatch via mp_call_function_1
+        // Callable is stored in state[0]
+        mp_obj_t callable = self->code_state.state[0];
+
+        self->pend_exc = MP_OBJ_NULL;
+        *ret_val = mp_call_function_1(callable, send_value);
+        self->pend_exc = mp_const_none;
+        return MP_VM_RETURN_NORMAL;
+
     } else {
         // Bytecode generator: resume from saved IP
         if (self->code_state.ip == 0) {

shared/runtime/mpirq.c

@@ -40,43 +40,43 @@
 /******************************************************************************
  UNIFIED IRQ HANDLER WRAPPING
 
- All IRQ handlers are converted to generator-compatible objects at registration
- time. This eliminates type checks in the hot dispatch path.
+ For hard IRQ: All handlers are converted to generator-compatible objects at
+ registration time. This eliminates type checks in the hot dispatch path.
 
- Handler types:
+ Handler types for hard IRQ:
  - Real generators: Used as-is after priming (run to first yield)
  - Bytecode functions: Wrapped as mp_type_gen_instance with IRQ_FUNC_BC sentinel
  - Native functions: Wrapped as mp_type_gen_instance with IRQ_FUNC_NAT sentinel
  - Viper functions: Wrapped as mp_type_gen_instance with IRQ_VIPER sentinel
+ - Other callables: Wrapped with IRQ_CALLABLE sentinel, calls via mp_call_function_1
+
+ For soft IRQ: Generators are instantiated and primed, but other callables are
+ passed directly to mp_sched_schedule without wrapping.
 
  The sentinel value in exc_sp_idx tells mp_obj_gen_resume_irq() how to handle it.
  ******************************************************************************/
 
-// Get pointer to extra data stored after the state[] + exc_stack[] arrays.
-// This requires knowing n_state and n_exc_stack (stored in code_state).
-static inline mp_irq_handler_extra_t *mp_irq_get_extra(mp_obj_gen_instance_t *gen, size_t n_exc_stack) {
-    size_t state_size = gen->code_state.n_state * sizeof(mp_obj_t);
-    size_t exc_size = n_exc_stack * sizeof(mp_exc_stack_t);
-    return (mp_irq_handler_extra_t *)((byte *)gen->code_state.state + state_size + exc_size);
-}
-
 // Wrap a bytecode function as a generator-compatible object for fast IRQ dispatch.
 // Calls mp_setup_code_state once at wrap time to initialize everything.
 static mp_obj_t mp_irq_wrap_bytecode_function(mp_obj_t func_in) {
     mp_obj_fun_bc_t *fun = MP_OBJ_TO_PTR(func_in);
 
     // Decode state requirements from bytecode prelude
     const uint8_t *ip = fun->bytecode;
-    size_t n_state, n_pos_args;
+    size_t n_state, n_pos_args, scope_flags;
     {
-        size_t n_exc_stack, scope_flags, n_kwonly_args, n_def_args;
+        size_t n_exc_stack, n_kwonly_args, n_def_args;
         MP_BC_PRELUDE_SIG_DECODE_INTO(ip, n_state, n_exc_stack, scope_flags, n_pos_args, n_kwonly_args, n_def_args);
         (void)n_exc_stack;
-        (void)scope_flags;
         (void)n_kwonly_args;
         (void)n_def_args;
     }
 
+    // Reject generator functions (those with yield) - they should be passed as gen_wrap
+    if (scope_flags & MP_SCOPE_FLAG_GENERATOR) {
+        mp_raise_ValueError(MP_ERROR_TEXT("use generator function, not plain function with yield"));
+    }
+
     // Verify function signature: must accept exactly 1 positional arg
     if (n_pos_args != 1) {
         mp_raise_ValueError(MP_ERROR_TEXT("IRQ callback must take exactly 1 argument"));
@@ -120,11 +120,16 @@ static mp_obj_t mp_irq_wrap_bytecode_function(mp_obj_t func_in) {
 static mp_obj_t mp_irq_wrap_native_function(mp_obj_t func_in) {
     mp_obj_fun_bc_t *fun = MP_OBJ_TO_PTR(func_in);
 
-    // Get prelude to determine state size
+    // Get prelude to determine state size and validate signature
     const uint8_t *prelude_ptr = mp_obj_fun_native_get_prelude_ptr(fun);
     const uint8_t *ip = prelude_ptr;
     MP_BC_PRELUDE_SIG_DECODE(ip);
 
+    // Verify function signature: must accept exactly 1 positional arg
+    if (n_pos_args != 1) {
+        mp_raise_ValueError(MP_ERROR_TEXT("IRQ callback must take exactly 1 argument"));
+    }
+
     // Native functions don't need exception stack
     size_t extra_size = sizeof(mp_irq_handler_extra_t);
     size_t total_var_size = n_state * sizeof(mp_obj_t) + extra_size;
@@ -173,8 +178,37 @@ static mp_obj_t mp_irq_wrap_viper_function(mp_obj_t func_in) {
     return MP_OBJ_FROM_PTR(o);
 }
 
+// Wrap a @viper function with argument count validation.
+// Viper functions use a special prelude format that must be parsed differently.
+static mp_obj_t mp_irq_wrap_viper_function_validated(mp_obj_t func_in) {
+    // Note: viper functions have different prelude structure - validation at call time
+    // The n_pos_args check would require parsing viper-specific metadata which is complex.
+    // Since viper is called with correct signature, argument errors will be caught at call time.
+    return mp_irq_wrap_viper_function(func_in);
+}
 #endif // MICROPY_EMIT_NATIVE
 
+// Wrap any callable as a generator-compatible object for hard IRQ dispatch.
+// Uses mp_call_function_1 at dispatch time (slower than specialized wrappers but works
+// for any callable including bound methods, closures, etc).
+static mp_obj_t mp_irq_wrap_callable(mp_obj_t callable) {
+    // Minimal allocation: generator instance with space to store the callable
+    // We store the callable in state[0], and use IRQ_CALLABLE sentinel
+    size_t n_state = 2;  // state[0] = callable, state[1] unused
+    size_t total_var_size = n_state * sizeof(mp_obj_t);
+
+    mp_obj_gen_instance_t *o = mp_obj_malloc_var(mp_obj_gen_instance_t, code_state.state,
+        byte, total_var_size, &mp_type_gen_instance);
+
+    o->pend_exc = mp_const_none;
+    o->code_state.fun_bc = NULL;  // Not used for generic callable
+    o->code_state.n_state = n_state;
+    o->code_state.exc_sp_idx = MP_CODE_STATE_EXC_SP_IDX_IRQ_CALLABLE;
+    o->code_state.state[0] = callable;  // Store the actual callable here
+
+    return MP_OBJ_FROM_PTR(o);
+}
+
 /******************************************************************************
  DECLARE PUBLIC DATA
  ******************************************************************************/
@@ -203,7 +237,7 @@ void mp_irq_init(mp_irq_obj_t *self, const mp_irq_methods_t *methods, mp_obj_t p
     self->ishard = false;
 }
 
-mp_obj_t mp_irq_prepare_handler(mp_obj_t callback, mp_obj_t parent) {
+mp_obj_t mp_irq_prepare_handler(mp_obj_t callback, mp_obj_t parent, bool ishard) {
     // Auto-instantiate generator functions (bytecode or native).
     if (mp_obj_is_type(callback, &mp_type_gen_wrap)
         #if MICROPY_EMIT_NATIVE
@@ -223,19 +257,29 @@ mp_obj_t mp_irq_prepare_handler(mp_obj_t callback, mp_obj_t parent) {
             }
             mp_raise_ValueError(MP_ERROR_TEXT("generator must yield"));
         }
-    } else if (mp_obj_is_type(callback, &mp_type_fun_bc)) {
-        // Wrap bytecode functions as generator-compatible objects for fast IRQ dispatch.
-        callback = mp_irq_wrap_bytecode_function(callback);
-    #if MICROPY_EMIT_NATIVE
-    } else if (mp_obj_is_type(callback, &mp_type_fun_native)) {
-        // Wrap @native functions
-        callback = mp_irq_wrap_native_function(callback);
-    } else if (mp_obj_is_type(callback, &mp_type_fun_viper)) {
-        // Wrap @viper functions
-        callback = mp_irq_wrap_viper_function(callback);
-    #endif
-    } else if (callback != mp_const_none && !mp_obj_is_callable(callback)) {
-        mp_raise_ValueError(MP_ERROR_TEXT("callback must be None, callable, or generator"));
+        // Generator is ready - no wrapping needed (already gen_instance)
+    } else if (ishard) {
+        // Hard IRQ: wrap all callables as generator-compatible objects for unified dispatch
+        if (mp_obj_is_type(callback, &mp_type_fun_bc)) {
+            callback = mp_irq_wrap_bytecode_function(callback);
+        #if MICROPY_EMIT_NATIVE
+        } else if (mp_obj_is_type(callback, &mp_type_fun_native)) {
+            callback = mp_irq_wrap_native_function(callback);
+        } else if (mp_obj_is_type(callback, &mp_type_fun_viper)) {
+            callback = mp_irq_wrap_viper_function_validated(callback);
+        #endif
+        } else if (callback != mp_const_none) {
+            // Generic callable (bound method, closure, etc) - wrap for hard IRQ dispatch
+            if (!mp_obj_is_callable(callback)) {
+                mp_raise_ValueError(MP_ERROR_TEXT("callback must be None, callable, or generator"));
+            }
+            callback = mp_irq_wrap_callable(callback);
+        }
+    } else {
+        // Soft IRQ: don't wrap - mp_sched_schedule will call via mp_call_function_1
+        if (callback != mp_const_none && !mp_obj_is_callable(callback)) {
+            mp_raise_ValueError(MP_ERROR_TEXT("callback must be None, callable, or generator"));
+        }
     }
 
     return callback;
@@ -270,7 +314,8 @@ int mp_irq_dispatch(mp_obj_t handler, mp_obj_t parent, bool ishard) {
                     // Only signal error for real generators that have exhausted
                     if (exc_idx != MP_CODE_STATE_EXC_SP_IDX_IRQ_FUNC_BC
                         && exc_idx != MP_CODE_STATE_EXC_SP_IDX_IRQ_FUNC_NAT
-                        && exc_idx != MP_CODE_STATE_EXC_SP_IDX_IRQ_VIPER) {
+                        && exc_idx != MP_CODE_STATE_EXC_SP_IDX_IRQ_VIPER
+                        && exc_idx != MP_CODE_STATE_EXC_SP_IDX_IRQ_CALLABLE) {
                         result = -1;  // Generator exhausted
                     }
                 } else if (ret == MP_VM_RETURN_EXCEPTION) {

shared/runtime/mpirq.h

@@ -85,7 +85,7 @@ extern const mp_obj_type_t mp_irq_type;
 
 mp_irq_obj_t *mp_irq_new(const mp_irq_methods_t *methods, mp_obj_t parent);
 void mp_irq_init(mp_irq_obj_t *self, const mp_irq_methods_t *methods, mp_obj_t parent);
-mp_obj_t mp_irq_prepare_handler(mp_obj_t callback, mp_obj_t parent);
+mp_obj_t mp_irq_prepare_handler(mp_obj_t callback, mp_obj_t parent, bool ishard);
 int mp_irq_dispatch(mp_obj_t handler, mp_obj_t parent, bool ishard);
 void mp_irq_handler(mp_irq_obj_t *self);