[CLN-list] Segfault when using univpoly

[Richard B. Kreckel]

On Mon, 14 Mar 2005, I wrote:
> Of course, taking the cl_heap* part of a cl_heap_modint_ring* results in
> an offset the size of the vptr.  And for it to work it's required that the
> definitions are visible.  Otherwise the offset will be zero.

However, if one takes care that the pointer of a cl_rcpointer always
points to the cl_heap sub-part, one runs into serious trouble later.  In
src/base/cl_free.cc:

void cl_free_heap_object (cl_heap* pointer)
{
        // This is invoked when pointer->refcount gets decremented to 0.
        var const cl_class* type = pointer->type;
        if (type->destruct)
                type->destruct(pointer);
        free_hook(pointer);
}

This calls only the registered dtor of the subobject, which invokes
undefined behavior!  (In practice, "undefined" means a downright segfault,
in this case.)

I see no way to cure this problem without adding more indirection.

The best idea I'm currently having is removing the vptr from all classes
derived from cl_heap.  This is done in the attached patch.  It makes each
subclass of cl_heap_modint_ring have different cl_class* type, however, so
they cannot be compared any more!  For this reason, it is best to remove
the extern declaration of the static cl_class_modint_ring and rely on
cl_class::flags for run time type information.  (There goes a clear
design.)

If somebody wants to bang on the patch, I'm interested in hearing how it
goes.

  -richy.
-- 
Richard B. Kreckel
<http://www.ginac.de/~kreckel/>
-------------- next part --------------
Index: include/cln/modinteger.h
===================================================================
RCS file: /home/cvs/cln/include/cln/modinteger.h,v
retrieving revision 1.5
diff -r1.5 modinteger.h
352c352
< // Constructor.
---
> // Constructor / destructor.
354,357c354
< // This class is intented to be subclassable, hence needs a virtual destructor.
< 	virtual ~cl_heap_modint_ring () {}
< private:
< 	virtual void dummy ();
---
> 	~cl_heap_modint_ring () {}
365,367d361
< 
< // Runtime typing support.
< extern cl_class cl_class_modint_ring;
Index: include/cln/object.h
===================================================================
RCS file: /home/cvs/cln/include/cln/object.h,v
retrieving revision 1.10
diff -r1.10 object.h
171c171
< // Flags, to be ORed together.
---
> // Flags, may be ORed together.
177a178,179
> #define cl_class_flags_modint_ring       32  // all instances are rings whose
>                                              // elements belong to cl_MI
Index: include/cln/univpoly.h
===================================================================
RCS file: /home/cvs/cln/include/cln/univpoly.h,v
retrieving revision 1.5
diff -r1.5 univpoly.h
343,346c343
< // This class is intented to be subclassable, hence needs a virtual destructor.
< 	virtual ~cl_heap_univpoly_ring () {}
< private:
< 	virtual void dummy ();
---
> 	~cl_heap_univpoly_ring () {}
361,363d357
< 
< // Runtime typing support.
< extern cl_class cl_class_univpoly_ring;
Index: src/modinteger/cl_MI.cc
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI.cc,v
retrieving revision 1.4
diff -r1.4 cl_MI.cc
29a30,39
> static void cl_modint_ring_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring*)pointer).~cl_heap_modint_ring();
> }
> 
> cl_class cl_class_modint_ring = {
> 	cl_modint_ring_destructor,
> 	cl_class_flags_modint_ring
> };
> 
52,65d61
< 
< static void cl_modint_ring_destructor (cl_heap* pointer)
< {
< 	(*(cl_heap_modint_ring*)pointer).~cl_heap_modint_ring();
< }
< 
< cl_class cl_class_modint_ring = {
< 	cl_modint_ring_destructor,
< 	0
< };
< 
< // This tells the compiler to put the `cl_heap_modint_ring' vtable
< // into this file.
< void cl_heap_modint_ring::dummy () {}
Index: src/modinteger/cl_MI_fix16.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_fix16.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_fix16.h
69,71c69,70
< 	cl_heap_modint_ring_fix16 (const cl_I& m)
< 		: cl_heap_modint_ring (m, &std_setops, &fix16_addops, &fix16_mulops) {}
< 	// Virtual destructor.
---
> 	cl_heap_modint_ring_fix16 (const cl_I& m);
> 	// Destructor.
73a73,89
> 
> static void cl_modint_ring_fix16_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_fix16*)pointer).~cl_heap_modint_ring_fix16();
> }
> 
> cl_class cl_class_modint_ring_fix16 = {
> 	cl_modint_ring_fix16_destructor,
> 	cl_class_flags_modint_ring
> };
> 
> // Constructor.
> inline cl_heap_modint_ring_fix16::cl_heap_modint_ring_fix16(const cl_I& m)
> 	: cl_heap_modint_ring (m, &std_setops, &fix16_addops, &fix16_mulops)
> {
> 	type = &cl_class_modint_ring_fix16;
> }
Index: src/modinteger/cl_MI_fix29.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_fix29.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_fix29.h
76,78c76,77
< 	cl_heap_modint_ring_fix29 (const cl_I& m)
< 		: cl_heap_modint_ring (m, &std_setops, &fix29_addops, &fix29_mulops) {}
< 	// Virtual destructor.
---
> 	cl_heap_modint_ring_fix29 (const cl_I& m);
> 	// Destructor.
80a80,96
> 
> static void cl_modint_ring_fix29_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_fix29*)pointer).~cl_heap_modint_ring_fix29();
> }
> 
> cl_class cl_class_modint_ring_fix29 = {
> 	cl_modint_ring_fix29_destructor,
> 	cl_class_flags_modint_ring
> };
> 
> // Constructor.
> inline cl_heap_modint_ring_fix29::cl_heap_modint_ring_fix29(const cl_I& m)
> 	: cl_heap_modint_ring (m, &std_setops, &fix29_addops, &fix29_mulops)
> {
> 	type = &cl_class_modint_ring_fix29;
> }
Index: src/modinteger/cl_MI_fix32.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_fix32.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_fix32.h
78,80c78,79
< 	cl_heap_modint_ring_fix32 (const cl_I& m)
< 		: cl_heap_modint_ring (m, &std_setops, &fix32_addops, &fix32_mulops) {}
< 	// Virtual destructor.
---
> 	cl_heap_modint_ring_fix32 (const cl_I& m);
> 	// Destructor.
82a82,98
> 
> static void cl_modint_ring_fix32_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_fix32*)pointer).~cl_heap_modint_ring_fix32();
> }
> 
> cl_class cl_class_modint_ring_fix32 = {
> 	cl_modint_ring_fix32_destructor,
> 	cl_class_flags_modint_ring
> };
> 
> // Constructor.
> inline cl_heap_modint_ring_fix32::cl_heap_modint_ring_fix32(const cl_I& m)
> 	: cl_heap_modint_ring (m, &std_setops, &fix32_addops, &fix32_mulops)
> {
> 	type = &cl_class_modint_ring_fix32;
> }
Index: src/modinteger/cl_MI_int32.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_int32.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_int32.h
77,79c77,78
< 	cl_heap_modint_ring_int32 (const cl_I& m)
< 		: cl_heap_modint_ring (m, &std_setops, &int32_addops, &int32_mulops) {}
< 	// Virtual destructor.
---
> 	cl_heap_modint_ring_int32 (const cl_I& m);
> 	// Destructor.
81a81,97
> 
> static void cl_modint_ring_int32_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_int32*)pointer).~cl_heap_modint_ring_int32();
> }
> 
> cl_class cl_class_modint_ring_int32 = {
> 	cl_modint_ring_int32_destructor,
> 	cl_class_flags_modint_ring
> };
> 
> // Constructor.
> inline cl_heap_modint_ring_int32::cl_heap_modint_ring_int32(const cl_I& m)
> 	: cl_heap_modint_ring (m, &std_setops, &int32_addops, &int32_mulops)
> {
> 	type = &cl_class_modint_ring_int32;
> }
Index: src/modinteger/cl_MI_montgom.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_montgom.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_montgom.h
73c73
< 	// Virtual destructor.
---
> 	// Destructor.
80a81,90
> static void cl_modint_ring_montgom_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_montgom*)pointer).~cl_heap_modint_ring_montgom();
> }
> 
> cl_class cl_class_modint_ring_montgom = {
> 	cl_modint_ring_montgom_destructor,
> 	cl_class_flags_modint_ring
> };
> 
164c174,176
< {}
---
> {
> 	type = &cl_class_modint_ring_montgom;
> }
Index: src/modinteger/cl_MI_pow2.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_pow2.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_pow2.h
10c10
< 	// Virtual destructor.
---
> 	// Destructor.
139a140,149
> static void cl_modint_ring_pow2_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_pow2*)pointer).~cl_heap_modint_ring_pow2();
> }
> 
> cl_class cl_class_modint_ring_pow2 = {
> 	cl_modint_ring_pow2_destructor,
> 	cl_class_flags_modint_ring
> };
> 
142c152,155
< 	: cl_heap_modint_ring (m, &std_setops, &pow2_addops, &pow2_mulops), m1 (_m1) {}
---
> 	: cl_heap_modint_ring (m, &std_setops, &pow2_addops, &pow2_mulops), m1 (_m1)
> {
> 	type = &cl_class_modint_ring_pow2;
> }
Index: src/modinteger/cl_MI_pow2m1.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_pow2m1.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_pow2m1.h
10c10
< 	// Virtual destructor.
---
> 	// Destructor.
80a81,90
> static void cl_modint_ring_pow2m1_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_pow2m1*)pointer).~cl_heap_modint_ring_pow2m1();
> }
> 
> cl_class cl_class_modint_ring_pow2m1 = {
> 	cl_modint_ring_pow2m1_destructor,
> 	cl_class_flags_modint_ring
> };
> 
83c93,96
< 	: cl_heap_modint_ring (m, &std_setops, &pow2m1_addops, &pow2m1_mulops), m1 (_m1) {}
---
> 	: cl_heap_modint_ring (m, &std_setops, &pow2m1_addops, &pow2m1_mulops), m1 (_m1)
> {
> 	type = &cl_class_modint_ring_pow2m1;
> }
Index: src/modinteger/cl_MI_pow2p1.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_pow2p1.h,v
retrieving revision 1.2
diff -r1.2 cl_MI_pow2p1.h
10c10
< 	// Virtual destructor.
---
> 	// Destructor.
91a92,101
> static void cl_modint_ring_pow2p1_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_pow2p1*)pointer).~cl_heap_modint_ring_pow2p1();
> }
> 
> cl_class cl_class_modint_ring_pow2p1 = {
> 	cl_modint_ring_pow2p1_destructor,
> 	cl_class_flags_modint_ring
> };
> 
94c104,107
< 	: cl_heap_modint_ring (m, &std_setops, &pow2p1_addops, &pow2p1_mulops), m1 (_m1) {}
---
> 	: cl_heap_modint_ring (m, &std_setops, &pow2p1_addops, &pow2p1_mulops), m1 (_m1)
> {
> 	type = &cl_class_modint_ring_pow2p1;
> }
Index: src/modinteger/cl_MI_std.h
===================================================================
RCS file: /home/cvs/cln/src/modinteger/cl_MI_std.h,v
retrieving revision 1.3
diff -r1.3 cl_MI_std.h
348,349c348
< 	cl_heap_modint_ring_std (const cl_I& m)
< 		: cl_heap_modint_ring (m, &std_setops, &std_addops, &std_mulops) {}
---
> 	cl_heap_modint_ring_std (const cl_I& m);
352a352,368
> 
> static void cl_heap_modint_ring_std_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_ring_std*)pointer).~cl_heap_modint_ring_std();
> }
> 
> cl_class cl_class_modint_ring_std = {
> 	cl_heap_modint_ring_std_destructor,
> 	cl_class_flags_modint_ring
> };
> 
> // Constructor.
> inline cl_heap_modint_ring_std::cl_heap_modint_ring_std (const cl_I& m)
> 	: cl_heap_modint_ring (m, &std_setops, &std_addops, &std_mulops)
> {
> 	type = &cl_class_modint_ring_std;
> }
Index: src/polynomial/elem/cl_UP.cc
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP.cc,v
retrieving revision 1.2
diff -r1.2 cl_UP.cc
41,48d40
< cl_heap_univpoly_ring::cl_heap_univpoly_ring (const cl_ring& r, cl_univpoly_setops* setopv, cl_univpoly_addops* addopv, cl_univpoly_mulops* mulopv, cl_univpoly_modulops* modulopv, cl_univpoly_polyops* polyopv)
< 	: setops (setopv), addops (addopv), mulops (mulopv), modulops (modulopv), polyops (polyopv),
< 	  _basering (r)
< {
< 	refcount = 0; // will be incremented by the `cl_univpoly_ring' constructor
< 	type = &cl_class_univpoly_ring;
< }
< 
59,61c51,57
< // This tells the compiler to put the `cl_heap_univpoly_ring' vtable
< // into this file.
< void cl_heap_univpoly_ring::dummy () {}
---
> cl_heap_univpoly_ring::cl_heap_univpoly_ring (const cl_ring& r, cl_univpoly_setops* setopv, cl_univpoly_addops* addopv, cl_univpoly_mulops* mulopv, cl_univpoly_modulops* modulopv, cl_univpoly_polyops* polyopv)
> 	: setops (setopv), addops (addopv), mulops (mulopv), modulops (modulopv), polyops (polyopv),
> 	  _basering (r)
> {
> 	refcount = 0; // will be incremented by the `cl_univpoly_ring' constructor
> 	type = &cl_class_univpoly_ring;
> }
70c66
< 	else if (r.pointer_type() == &cl_class_modint_ring) {
---
> 	else if (r.pointer_type()->flags & cl_class_flags_modint_ring) {
Index: src/polynomial/elem/cl_UP_GF2.h
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP_GF2.h,v
retrieving revision 1.3
diff -r1.3 cl_UP_GF2.h
1044,1045c1044,1046
< 	cl_heap_gf2_univpoly_ring (const cl_ring& r)
< 		: cl_heap_univpoly_ring (r, &gf2_setops, &gf2_addops, &gf2_mulops, &gf2_modulops, &gf2_polyops) {}
---
> 	cl_heap_gf2_univpoly_ring (const cl_ring& r);
> 	// Destructor.
> 	~cl_heap_gf2_univpoly_ring () {}
1046a1048,1064
> 
> static void cl_heap_gf2_univpoly_ring_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_gf2_univpoly_ring*)pointer).~cl_heap_gf2_univpoly_ring();
> }
> 
> cl_class cl_class_gf2_univpoly_ring = {
> 	cl_heap_gf2_univpoly_ring_destructor,
> 	0
> };
> 
> // Constructor.
> inline cl_heap_gf2_univpoly_ring::cl_heap_gf2_univpoly_ring (const cl_ring& r)
> 	: cl_heap_univpoly_ring (r, &gf2_setops, &gf2_addops, &gf2_mulops, &gf2_modulops, &gf2_polyops)
> {
> 	type = &cl_class_gf2_univpoly_ring;
> }
Index: src/polynomial/elem/cl_UP_MI.h
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP_MI.h,v
retrieving revision 1.5
diff -r1.5 cl_UP_MI.h
484,485c484,486
< 	cl_heap_modint_univpoly_ring (const cl_ring& r)
< 		: cl_heap_univpoly_ring (r, &modint_setops, &modint_addops, &modint_mulops, &modint_modulops, &modint_polyops) {}
---
> 	cl_heap_modint_univpoly_ring (const cl_ring& r);
> 	// Destructor.
> 	~cl_heap_modint_univpoly_ring () {}
486a488,504
> 
> static void cl_heap_modint_univpoly_ring_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_modint_univpoly_ring*)pointer).~cl_heap_modint_univpoly_ring();
> }
> 
> cl_class cl_class_modint_univpoly_ring = {
> 	cl_heap_modint_univpoly_ring_destructor,
> 	0
> };
> 
> // Constructor.
> inline cl_heap_modint_univpoly_ring::cl_heap_modint_univpoly_ring (const cl_ring& r)
> 	: cl_heap_univpoly_ring (r, &modint_setops, &modint_addops, &modint_mulops, &modint_modulops, &modint_polyops)
> {
> 	type = &cl_class_modint_univpoly_ring;
> }
Index: src/polynomial/elem/cl_UP_gen.h
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP_gen.h,v
retrieving revision 1.5
diff -r1.5 cl_UP_gen.h
463,464c463,465
< 	cl_heap_gen_univpoly_ring (const cl_ring& r)
< 		: cl_heap_univpoly_ring (r, &gen_setops, &gen_addops, &gen_mulops, &gen_modulops, &gen_polyops) {}
---
> 	cl_heap_gen_univpoly_ring (const cl_ring& r);
> 	// Destructor
> 	~cl_heap_gen_univpoly_ring () {}
465a467,483
> 
> static void cl_heap_gen_univpoly_ring_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_gen_univpoly_ring*)pointer).~cl_heap_gen_univpoly_ring();
> }
> 
> cl_class cl_class_gen_univpoly_ring = {
> 	cl_heap_gen_univpoly_ring_destructor,
> 	0
> };
> 
> // Constructor.
> inline cl_heap_gen_univpoly_ring::cl_heap_gen_univpoly_ring (const cl_ring& r)
> 	: cl_heap_univpoly_ring (r, &gen_setops, &gen_addops, &gen_mulops, &gen_modulops, &gen_polyops)
> {
> 	type = &cl_class_gen_univpoly_ring;
> }
Index: src/polynomial/elem/cl_UP_number.h
===================================================================
RCS file: /home/cvs/cln/src/polynomial/elem/cl_UP_number.h,v
retrieving revision 1.5
diff -r1.5 cl_UP_number.h
466,467c466,468
< 	cl_heap_num_univpoly_ring (const cl_ring& r)
< 		: cl_heap_univpoly_ring (r, &num_setops, &num_addops, &num_mulops, &num_modulops, &num_polyops) {}
---
> 	cl_heap_num_univpoly_ring (const cl_ring& r);
> 	// Destructor.
> 	~cl_heap_num_univpoly_ring () {}
468a470,486
> 
> static void cl_heap_num_univpoly_ring_destructor (cl_heap* pointer)
> {
> 	(*(cl_heap_num_univpoly_ring*)pointer).~cl_heap_num_univpoly_ring();
> }
> 
> cl_class cl_class_num_univpoly_ring = {
> 	cl_heap_num_univpoly_ring_destructor,
> 	0
> };
> 
> // Constructor.
> inline cl_heap_num_univpoly_ring::cl_heap_num_univpoly_ring (const cl_ring& r)
> 	: cl_heap_univpoly_ring (r, &num_setops, &num_addops, &num_mulops, &num_modulops, &num_polyops)
> {
> 	type = &cl_class_num_univpoly_ring;
> }