_caseInsensGetattr() causes simulation failure: broken interaction with dut due to calling dir(obj)

[paul-demo]

I have a very strange but very repeatable problem running cocotb 1.8.1, cocotb_bus 0.2.1, and Verilator on [arm64] MacOS. Specifically, calling dir(obj) results in no DUT signals' values showing up in the resultant waves file. The signals generated by the testbench are still there, but basically dumping any signal that is part of the DUT to waves is somehow disturbed by calling dir(dut) and nothing gets generated.

I traced the problem to this somewhat specific add-on feature in bus.py which iterates through dir(obj) (where obj == dut in common cocotb testbench name conventions). The whole point is just to make something case insensitive for convenience but it's causing me a lot of problems because calling dir(obj) apparently has undesirable side effects.

    def _caseInsensGetattr(self, obj, attr):
        for a in dir(obj):
            if a.casefold() == attr.casefold():
                return getattr(obj, a)
        return None

I can recreate the issue not even involving cocotb_bus by just adding this at the top of my testbench:

_ = dir(dut)

I found that simply changing three characters in cocotb_bus/bus.py from dir() to vars() fixes the problem. I'm not sure why that is. But I think since the dut object created by Cocotb defines a __dir__() function, it should be used and it will be used if you call vars().

More thoughts:
Searching through dirs() is not even what the code should be doing because that would include iteration over all these private member variables and possibly all superclasses (one of which, I'm assuming is causing the problem):

    "_HierarchyObject__get_sub_handle_by_name", "__class__", "__delattr__", "__dict__", "__dir__", "__doc__", "__eq__", "__format__", "__ge__", "__getattr__", "__getattribute__", "__gt__", "__hash__", "__init__", "__init_subclass__", "__iter__", "__le__", "__len__", "__lt__", "__module__", "__ne__", "__new__", "__reduce__", "__reduce_ex__", "__repr__", "__setattr__", "__sizeof__", "__str__", "__subclasshook__", "__weakref__", "_child_path", "_compat_mapping", "_def_file", "_def_name", "_discover_all", "_discovered", "_fullname", "_handle", "_id", "_invalid_sub_handles", "_len", "_log", "_name", "_path", "_sub_handle_key", "_sub_handles", "_type"

The cocotb_bus code should intead be iterating through the names of the public member variables, which include signal names, etc. of the dut. I'm assuming that somewhere deep in the Verilator/Cocotb interface, that list is generated and made available by the function __dirs__() and hence by vars(obj) (contrary to what you might think from the name, dir(obj) does not call __dirs__()). Here's an explanation:

https://stackoverflow.com/questions/980249/difference-between-dir-and-vars-keys-in-python

Python objects usually store their instance variables in a dictionary
that belongs to the object (except for slots). vars(x) returns this
dictionary (as does x.__dict__).

dir(x), on the other hand, returns a dictionary of x's "attributes,
its class's attributes, and recursively the attributes of its class's
base classes."

I suspect that what is happening is that the base classes are some compiled C++/Swig/CPython magic at the interface of Verilator and Cocotb, and therefore recursively looking at all the base class attributes is resulting in accessing something illegally and breaking the simulation / waves export. Interestingly, my colleague said it doesn't happen in Linux; I have verified (rather extensively) that this is the cause of the problem on MacOS.

TL;DR: please change that line to vars(obj) !

[paul-demo]

Well shoot, vars() only solves the problem because it never iterates over the problematic member which causes the waves to disappear. Unfortunately this also results in the Bus not being created, since vars() only has the following members in it, and has none of the signals that we actually want to find in the case-insensitive-search function.

{'_def_file': '',
 '_def_name': '',
 '_discovered': False,
 '_fullname': 'mymodule(GPI_MODULE)',
 '_handle': <cocotb.simulator.gpi_sim_hdl at 0x600002c1d4d0>,
 '_invalid_sub_handles': set(),
 '_len': None,
 '_log': <SimBaseLog cocotb.mymodule (INFO)>,
 '_name': 'mymodule',
 '_path': 'mymodule',
 '_sub_handles': {'clk': ModifiableObject(mymodule.clk)},
 '_type': 'GPI_MODULE'}

[Elefseus]

In my case, dir(dut) does not show all signals, either.
Then I find a answer in https://stackoverflow.com/questions/22105701/python-method-dir-does-not-return-all-attributes-methods .
Therefore, before create the bus, I manually traverse all the dut's bus signals by using 'getattr', like this:

for i in signal_names:
    try:
    getattr(dut, i)
bus = AXI4Master(dut, ...)

[paul-demo]

My problem is not that dir(dut) does not show all signals. It's that the simulation is completely empty/broken due to calling dir(dut) in the case-insensitive function.

I'm using cocotbext-axi which derives some additional classes from the cocotb-bus Bus class, and my solution is just to monkeypatch the problematic "case insensitive" function, essentially removing that feature.

import cocotbext.axi

# Patch a problematic section of cocotb_bus (doesn't work on MacOS) by removing
# the call to dir(obj) (as well as the case-insensitivity feature, for convenience).
def _caseInsensGetattr(self, obj, attr):
    return getattr(obj, attr, None)

for attr in dir(cocotbext.axi):
    obj = getattr(cocotbext.axi, attr)
    if hasattr(obj, "_caseInsensGetattr"):
        setattr(obj, "_caseInsensGetattr", _caseInsensGetattr)

[paul-demo]

My colleague has reported that the case-insensitive feature is also causing issues using VCS as a backend on a RHEL platform; so this issue is actually not confined to just Verilator-on-MacOS as I originally thought!

Can the cocotb-bus authors turn off the case-insensitive feature? Or at least default to: case_insensitive=False everywhere?

[paul-demo]

This commit should not have changed the default behavior of cocotb from case-sensitive to case-insensitive. The proper way to add patches like this to python code is for the default behavior to be unmodified. The new parameter value should not automatically assume you want case-insensitivity.

1c6b2ac

Case insensitivity is a special situation that's only related to certain simulators; most simulators maintain case sensitive port names.

I propose, simply changing the case_insensitive default in two places to be False.

[cmarqu]

Related: #71, #45, #42, #39

[paul-demo]

So this seems like a widespread problem. I didn't know other people are encountering the same issue related to that one case-insensitivity function, so thanks for referencing all the reported issues, @cmarqu. I was actually surprised that someone in my same group at work also encountered an issue with that same function and needed to set case_insensitive = False.

I think until that feature works as intended in all situations, we should at the very least disable it by setting the default value to False. To that end, please review this trivial PR:

#74

I don't particularly know why that feature is needed in the first place, but imagine it's due to some archaic toolchain and simply for convenience. I trust Alex Forencich (@alexforencich) had good reason to do so, but I think it needs more development done before we should make it the default behavior for all users of cocotb-bus.

[paul-demo]

It seems the build pipeline for cocotb-bus is out of date so the CICD test results are not useful at the moment. Please consider the PR for submission regardless of this.

I see this warning:

Running attrs on Python 3.6 is deprecated & we intend to drop support soon

And this error:

../../designs/avalon_module/Makefile:48: /Makefile.sim: No such file or directory

Neither of which are related to my change, I think.

[paul-demo]

This was actually a reported problem with the case-insensitivity change just 2 weeks after it was introduced. The change was made March 4 2021 and by March 15 2021 it was clear that it was incompatible with some simulators:

cocotb/cocotb#2473 (comment)

I think we should roll it back or disable that feature until it works on more platforms and more/all backend simulators that are supported by Cocotb (ex: those listed in runner.py, for example).

[cmarqu]

It seems the build pipeline for cocotb-bus is out of date so the CICD test results are not useful at the moment.

I'm working on fixing CI right now.

[alexforencich]

Verilator's PLI has been very, very broken until quite recently. Seems like 5.020 is the minimum usable version; the cocotb docs should be updated to reflect this. There are still some problems, but dir() and case insensitive matching should work in 5.020. If not, then we need to figure out why. If you're using a release older than 5.020, then you need up upgrade.

[paul-demo]

Works with Verilator 5.022 now, per discussion in #74.

Closing this issue.

[alexforencich]

We should open a separate issue for VCS, though.

[cmarqu]

Verilator's PLI has been very, very broken until quite recently. Seems like 5.020 is the minimum usable version; the cocotb docs should be updated to reflect this.

This is done in cocotb/cocotb#3740