[Triggers] Hard to understand error messages if signal is not of correct type

[Scafir]

Description

If one awaits a trigger with dut as the signal (e.g: await RisingEdge(dut)), then the error produced is incomprehensible:

./sample:error: NULL access dereferenced
in process .sample(structure).P0
XXX lineno: 293, opcode: 151
XXX lineno: 214, opcode: 166
make[1]: *** [/home/clyde/.local/lib/python3.11/site-packages/cocotb/share/makefiles/simulators/Makefile.ghdl:77: results.xml] Error 255

This makes a relatively simple user mistake seem like an internal bug. This behavior is partly due to dut having an attribute _handle, thus not producing the "usual" error due to a typo: AttributeError: 'XXX' object has no attribute '_handle'

Expected Behavior

A more understandable error should be raised (TypeError if signal is not of type cocotb.handle.ModifiableObject?)

How to reproduce?

-- sample.vhd
LIBRARY IEEE;
USE IEEE.std_logic_1164.ALL;
ENTITY sample IS
    PORT(
        a : in std_logic;
        b : out std_logic
    );
END sample;

ARCHITECTURE structure OF sample IS
BEGIN
    b <= a;
END structure;

# test_sample.py
import cocotb
from cocotb.triggers import RisingEdge
from cocotb.clock import Clock

@cocotb.test()
async def type_test(dut):
    clock = Clock(dut.a, 10, units='ns')
    cocotb.start_soon(clock.start())

    # No problem
    await RisingEdge(dut.b)

    # Reasonnable error
    # await RisingEdge(12)

    # Incomprehensible error
    await RisingEdge(dut)

# Makefile
VHDL_SOURCES = $(PWD)/sample.vhd
MODULE = test_sample
TOPLEVEL = sample
SIM = ghdl

include $(shell cocotb-config --makefiles)/Makefile.sim

Context

• Cocotb - 0.2.3
• GHDL - git version 65a40a067d1b153265eaf8bcadbc225c5eeac041 (5 Jan 2023)

[marlonjames]

I'm assuming that version 0.2.3 is the cocotb-test version, not cocotb.

That being said, this is a bug. Module handles are of type GpiObjHdl type, but not GpiSignalObjHdl, which gpi_register_value_change_callback() assumes and does a static_cast to:

https://github.com/cocotb/cocotb/blob/4b5b80eb409c6f7e2a816784fe1086c785fb928f/cocotb/share/lib/gpi/GpiCommon.cpp#L516-L519

There is no checking for this difference either in GPI or in Python.

[imphil]

@Scafir Thanks for your great bug report. Would you be interested in attempting a fix in a pull request?

[Scafir]

Hi @imphil , I am willing to try. A few questions: It seems that the type check can either happen in cocotb/triggers.py#L386 or GpiCommon.cpp#L519. It seems to me that the first one would be the best place to do that. Should I also implement it in GpiCommon? My todo list would be as follow:

• Implement type check (either in triggers or GpiCommon)
• Add that RisingEdge, FallingEdge and Edge can raise a TypeException in the documentation
• Ensure that the test suite passes with no regression

Does this looks good to you?

[eric-wieser]

I don't think there's any need to document that objects can throw TypeErrors, because that's true of pretty much everything if you use it incorrectly.

[ktbarrett]

Dupe of #1887. Could be solved with #2774. IMO RisingEdge, Edge, and FallingEdge have no business being free functions, but oh well.