Re: gEDA-user: Icarus bug?

[Benjamin Ylvisaker <ben8@cs.washington.edu>]

On Tue, 15 May 2007 21:01:40 -0400
Mike Jarabek <mjarabek@sentex.ca> wrote:

> Hi,
> 
> On Tue, 2007-05-15 at 15:47 -0700, Benjamin Ylvisaker wrote:
> > I believe I found a bug in the Icarus VPI implementation, but I wanted
> > to check before filing a bug report.
> > 
> > I have code that works with ModelSim under Linux, but I want to
> > develop on my primary computer, which is a PowerBook, so I'm trying to
> > get it working with Icarus under OS X.  Here's the troublesome bit:
> > 
> > void register() {
> >   s_cb_data init;
> >   vpiHandle cbHandle;
> > 
> >   init.reason = cbStartOfSimulation;
> >   init.cb_rtn = initializeSim;
> >   init.obj = NULL;
> >   init.time = NULL;
> >   init.value = NULL;
> >   init.user_data = NULL;
> > 
> >   cbHandle = vpi_register_cb(&init);
> > /*  vpi_free_object(cbHandle);*/
> > }
> > 
> > void (*vlog_startup_routines[ ] ) () = {
> >    register,
> >    0
> > };
> > 
> 
> Be careful here, you are passing a pointer to an auto variable into the
> vpi_register() function.  The lifetime of the variable may need to be
> longer than the end of your function.  The Verilog standard itself is
> pretty loosy-goosy on the subject of who allocates and frees these
> things.
> 
> I'd recommend declaring the 'init' as:
> 
> static s_cb_data init;
> 
> instead.  Also, anything that the structure refers to should also be
> declared static, or malloc()'ed.  In fact, all of the examples in the
> Verilog standard follow this 'static' convention.

This sounds like good advice in general, though my very limited
testing suggests that it works either way (static or stack allocated).

> > When I run it this way, it seems to work okay, but when I uncomment
> > the call to "vpi_free_object", it causes Icarus to crash with a "Bus
> > Error".  The Verilog PLI Handbook says that it's good style to free
> > the callback handle, unless you need to use it somewhere else.
> 
> It might not be a good idea to free the callback object yet, since the
> callback has not happened. The example in the Verilog standard does not
> free the object either.
> 
> Normally you use the vpi_free*() functions to free things that are
> returned to you from the simulator kernel.

Section 6.5 of the 2nd edition of the Verilog PLI Handbook states in
no uncertain terms that the callback handle should be freed after
registering a callback to avoid space leaks.  However(!), I downloaded
the PLI Book code from the website and found that all the free calls
after callback registrations were removed.  It seems that the
confusion is about whether the free call should free the callback data
structure itself or just the handle to it.  I'm not worried about
leaking one or two handles in the whole application, so I'll err on
the safe but possibly leaky side for now.

> > Does this look like a bug to anyone else?
> 
> In some ways, yes.  But it looks like a classic auto variable that gets
> a pointer dereference that leads to a bus error, or worse, random stack
> trampling.  Those bugs are the most fun to find.

The new PLI Book code still doesn't use static allocation for the
callback struct, and it seems to work in both ModelSim and Icarus.  I
don't want to get into too much of a habit of declaring things static
when they don't need to be.  Does anyone know if Icarus and/or any
other simulators make a copy of the callback data structure, so that
it doesn't need to hang around after the registration call?

Benjamin


_______________________________________________
geda-user mailing list
geda-user@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-user