[CLN-list] Segfault when using univpoly

[Richard B. Kreckel]

On Mon, 7 Mar 2005, Ralf Goertz wrote:
> I just thought to give cln a try, but I seem to have hit a bad entry point.

Indeed.

> The following program segfaults
>
> %---------------------------------
> #include<iostream>
> #include<cln/cln.h>
>
> using namespace cln;
> using namespace std;
>
> main(){
>         cl_modint_ring R=find_modint_ring(9811);
>         cl_univpoly_modint_ring P=find_univpoly_ring(R);
>         return 0;
> }
> %--------------------------------------
>
> Using the 1.1.9-debug.rpm on a SuSE 9.2 linux, this is the output of gdb
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x400fe50c in cln::cl_make_univpoly_ring (r=@0xbfffefb0) at object.h:475
> 475                     { return heappointer->type; }

Ouch!  This is an ugly bug and I'm not quite sure how to fix it.

The problem is with the descendants of struct cl_heap.  In
include/cln/object.h:164 there is:

struct cl_heap {
    int refcount;                  // reference count for garbage collection
    const struct cl_class * type;  // type tag for dynamic typing
};

But in include/cln/modinteger.h:209 there is:

class cl_heap_modint_ring : public cl_heap {
    // ...
// This class is intented to be subclassable, hence needs a virtual destructor.
    virtual ~cl_heap_modint_ring () {}
private:
    virtual void dummy ();
};

At this point, the two virtual functions introduce a vptr to each object
of class cl_heap_modint_ring.  By the way, note that the same happens to
class cl_heap_univpoly_ring.

Now, in your program you call find_univpoly_ring(const cl_ring& r) where
r is of dynamic type cl_modint_ring.  It determines that the ring does not
exist yet and calls cl_make_univpoly_ring(const cl_ring& r) with in turn
invokes r.pointer_type().  But that is actually:

const cl_class * cl_rcpointer::pointer_type() const
{ return heappointer->type; }

Since heappointer is of static type cl_heap but of dynamic type
cl_heap_modint_ring (with vptr!), referencing cl_heap::type member
variable is undefined.

Possible fixes that I'm able to think up in a hurry:

1) Kluge around by overriding *::pointer_type() for certain classes:
   const cl_class * cl_modint_ring::pointer_type () const
   { return (reinterpret_cast<cl_heap_modint_ring*>(heappointer))->type; }
   This might fix some such problems but cannot work in general since it
   assumes that it is always called with the right dynamic type, otherwise
   the (wrong) function from cl_heap will be called.  And we simply cannot
   tell statically whether it's a cl_heap_modint_ring or a, say,
   cl_heap_ring (without vptr!)  Also note that writing
   const cl_class * cl_modint_ring::pointer_type () const
   { return ((cl_heap_modint_ring*)(heappointer))->type; }
   would be subtly wrong.

2) Add a vptr to cl_heap by giving it a virtual destructor.
   Ugh!  There must be a reason why we aren't writing in Java in the
   first place, I assume.  Anyway, I don't think it's going to be solved
   trivially.

3) Remove the vptr from cl_heap_modint_ring (and also from
   cl_heap_univpoly_ring) for good.  I'm irritated by the comment in
   include/cln/modinteger.h:354: "This class is intented to be
   subclassable, hence needs a virtual destructor."  The classes that
   actually do subclass it don't do anything to the virtual functions
   (with the sole exception of the destructors.)  So, this quick and dirty
   patch seems the fix your problem, maybe at the risk of a small memory
   leak.  That must yet be analyzed.
-------------------------------------------------------------------
Index: include/cln/modinteger.h
===================================================================
RCS file: /home/cvs/cln/include/cln/modinteger.h,v
retrieving revision 1.5
diff -a -u -r1.5 modinteger.h
--- include/cln/modinteger.h    6 May 2002 10:29:18 -0000       1.5
+++ include/cln/modinteger.h    7 Mar 2005 23:41:22 -0000
@@ -352,9 +352,9 @@
 // Constructor.
        cl_heap_modint_ring (cl_I m, cl_modint_setops*, cl_modint_addops*, cl_modint_mulops*);
 // This class is intented to be subclassable, hence needs a virtual destructor.-       virtual ~cl_heap_modint_ring () {}
+       //virtual ~cl_heap_modint_ring () {}
 private:
-       virtual void dummy ();
+       //virtual void dummy ();
 };
 #define SUBCLASS_cl_heap_modint_ring() \
   SUBCLASS_cl_heap_ring()
Index: include/cln/univpoly.h
===================================================================
RCS file: /home/cvs/cln/include/cln/univpoly.h,v
retrieving revision 1.5
diff -a -u -r1.5 univpoly.h
--- include/cln/univpoly.h      1 Jan 2004 15:59:30 -0000       1.5
+++ include/cln/univpoly.h      7 Mar 2005 23:41:26 -0000
@@ -341,9 +341,9 @@
 // Constructor.
        cl_heap_univpoly_ring (const cl_ring& r, cl_univpoly_setops*, cl_univpoly_addops*, cl_univpoly_mulops*, cl_univpoly_modulops*, cl_univpoly_polyops*);
 // This class is intented to be subclassable, hence needs a virtual destructor.-       virtual ~cl_heap_univpoly_ring () {}
+       //virtual ~cl_heap_univpoly_ring () {}
 private:
-       virtual void dummy ();
+       //virtual void dummy ();
 };
 #define SUBCLASS_cl_heap_univpoly_ring() \
   SUBCLASS_cl_heap_ring()
Index: src/modinteger/cl_MI.cc
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI.cc,v
retrieving revision 1.4
diff -a -u -r1.4 cl_MI.cc
--- src/modinteger/cl_MI.cc     29 Jun 2003 12:32:51 -0000      1.4
+++ src/modinteger/cl_MI.cc     7 Mar 2005 23:41:31 -0000
@@ -62,7 +62,7 @@

 // This tells the compiler to put the `cl_heap_modint_ring' vtable
 // into this file.
-void cl_heap_modint_ring::dummy () {}
+//void cl_heap_modint_ring::dummy () {}


 static cl_boolean modint_equal (cl_heap_modint_ring* R, const _cl_MI& x, const _cl_MI& y)
Index: src/polynomial/elem/cl_UP.cc
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP.cc,v
retrieving revision 1.2
diff -a -u -r1.2 cl_UP.cc
--- src/polynomial/elem/cl_UP.cc        28 Aug 2000 22:08:21 -0000      1.2
+++ src/polynomial/elem/cl_UP.cc        7 Mar 2005 23:41:31 -0000
@@ -58,7 +58,7 @@

 // This tells the compiler to put the `cl_heap_univpoly_ring' vtable
 // into this file.
-void cl_heap_univpoly_ring::dummy () {}
+//void cl_heap_univpoly_ring::dummy () {}


 // Create a new univariate polynomial ring.
-------------------------------------------------------------------

Could you please play a bit with that patch and tell how it goes?

Also, it might be helpful to consider the polynomial sector in CLN as,
errhm, "unfinished".  :-/

> Also, the legendre.cc example segfaults, if it is called with two arguments

Same cause, same solution.  Interesting, though.  How could it have
worked?  Was the virtual stuff introduced later then that example?

Regards
  -richy.
-- 
Richard B. Kreckel
<http://www.ginac.de/~kreckel/>