* [bug #25525] Segfault using goops
@ 2009-02-09 8:25 Michael Burschik
2009-02-09 23:17 ` Neil Jerram
0 siblings, 1 reply; 10+ messages in thread
From: Michael Burschik @ 2009-02-09 8:25 UTC (permalink / raw)
To: Michael Burschik, bug-guile
URL:
<http://savannah.gnu.org/bugs/?25525>
Summary: Segfault using goops
Project: Guile
Submitted by: burschik
Submitted on: Mon 09 Feb 2009 10:25:50 AM EET
Category: None
Severity: 3 - Normal
Item Group: None
Status: None
Privacy: Public
Assigned to: None
Open/Closed: Open
Discussion Lock: Any
_______________________________________________________
Details:
(use-modules (oop goops))
(define-class <test-class> (<class>)
name)
(make <test-class>)
This makes guile 1.8.5 crash with a segmentation fault on Linux.
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2009-02-09 8:25 [bug #25525] Segfault using goops Michael Burschik
@ 2009-02-09 23:17 ` Neil Jerram
2010-04-16 11:26 ` Stefan Israelsson Tampe
0 siblings, 1 reply; 10+ messages in thread
From: Neil Jerram @ 2009-02-09 23:17 UTC (permalink / raw)
To: Neil Jerram, Michael Burschik, bug-guile
Follow-up Comment #1, bug #25525 (project guile):
In case it helps until this is fixed properly...
This appears to be specific to the combination of
- the use of "name" in the definition of <test-class>
- the use of <class> as a superclass.
I would guess related to the fact that <class> already has a slot called
"name".
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2009-02-09 23:17 ` Neil Jerram
@ 2010-04-16 11:26 ` Stefan Israelsson Tampe
2010-04-16 11:36 ` Stefan Israelsson Tampe
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Israelsson Tampe @ 2010-04-16 11:26 UTC (permalink / raw)
To: Neil Jerram, Michael Burschik, Stefan Israelsson Tampe, bug-guile
Follow-up Comment #2, bug #25525 (project guile):
The reason is that code assume cpl to be a null terminated list but it's not
due to the bug, then a cdr will make the crash. Now this is a symptom and not
the cause. But we can avoid a segmentation fault by making sure that we check
for a pair although it will slow down the algorithm a little. Checking for a
pairs and silently move on if error yield an error in a layout check!
Better to fix the cause, right? Well to raise a question. by coding for
efficiency and assume that the code will be used correctly we will gain speed.
On the other hand this means that
any bug will, with maybe to high probability, crash the repl.
I would prefere to use fast and unsecure paths only for proven
datastructures. Or, next best, use checks that is on only under a debug
compile!
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2010-04-16 11:26 ` Stefan Israelsson Tampe
@ 2010-04-16 11:36 ` Stefan Israelsson Tampe
2010-06-07 10:00 ` Andy Wingo
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Israelsson Tampe @ 2010-04-16 11:36 UTC (permalink / raw)
To: Neil Jerram, Michael Burschik, Stefan Israelsson Tampe, bug-guile
Follow-up Comment #3, bug #25525 (project guile):
a cut and paste bug, the function is
build_slots_list in goops.c
Anyhow I saw that I can enable a pair test, with a debug option in the
compilation.
So, as always, people are smart.
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2010-04-16 11:36 ` Stefan Israelsson Tampe
@ 2010-06-07 10:00 ` Andy Wingo
2011-08-20 18:14 ` Stefan Israelsson Tampe
0 siblings, 1 reply; 10+ messages in thread
From: Andy Wingo @ 2010-06-07 10:00 UTC (permalink / raw)
To: Neil Jerram, Andy Wingo, Michael Burschik,
Stefan Israelsson Tampe, bug-guile
Follow-up Comment #4, bug #25525 (project guile):
This bug is still present in git.
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2010-06-07 10:00 ` Andy Wingo
@ 2011-08-20 18:14 ` Stefan Israelsson Tampe
2011-08-21 10:54 ` Stefan Israelsson Tampe
2011-09-02 17:57 ` Andy Wingo
0 siblings, 2 replies; 10+ messages in thread
From: Stefan Israelsson Tampe @ 2011-08-20 18:14 UTC (permalink / raw)
To: Neil Jerram, Andy Wingo, Michael Burschik,
Stefan Israelsson Tampe, bug-guile
Follow-up Comment #5, bug #25525 (project guile):
Ok, I did an attempt to make an acceptable fix to this.
The problem was that in <class> the name slot order of the layout
changed due to the new slot again called name. This cassed lookup code that
assumes a fixed layout to fetch the wrong values due to a shift of the layout.
The added fix will, at class creation, verify if the class is deducing
<class>. If so and there is an attempt to add a new slot named already defined
by <class>, an error will be signaled, see result below,
scheme@(guile-user)> (define-class <test-class> (<class>) name)
ERROR: In procedure %compute-slots:
ERROR: In procedure init-object: a predefined <class> inherrited field cannot
be redefined
Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]>
A patch includeing some solidification of the code by making sure
a pair is used where it is assumed it does is added as well.
See goopsdiff diff below.
(file #23840)
_______________________________________________________
Additional Item Attachment:
File name: goopsdiff.diff Size:4 KB
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2011-08-20 18:14 ` Stefan Israelsson Tampe
@ 2011-08-21 10:54 ` Stefan Israelsson Tampe
2011-09-05 12:25 ` Stefan Israelsson Tampe
2011-09-02 17:57 ` Andy Wingo
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Israelsson Tampe @ 2011-08-21 10:54 UTC (permalink / raw)
To: Neil Jerram, Andy Wingo, Michael Burschik,
Stefan Israelsson Tampe, bug-guile
Follow-up Comment #6, bug #25525 (project guile):
I recognized that the layout problem appears as well if you inherit
from first <class> and then a class <b> which does not contain <class> I did
fix this. By postponing the addition of <class> variables to the last in order
for them to always get the correct
index in the layout.
So, now
(1) you can inheret meta objects from both <class> and non <class> objects
(2) it's not possible to inherit or add variables that have the same name as
<class> variables. If that is tried an error is signaled.
For a git-diff, see goopsdiff2 below
(file #23843)
_______________________________________________________
Additional Item Attachment:
File name: goopsdiff2.diff Size:4 KB
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [bug #25525] Segfault using goops
2011-08-20 18:14 ` Stefan Israelsson Tampe
2011-08-21 10:54 ` Stefan Israelsson Tampe
@ 2011-09-02 17:57 ` Andy Wingo
1 sibling, 0 replies; 10+ messages in thread
From: Andy Wingo @ 2011-09-02 17:57 UTC (permalink / raw)
To: Stefan Israelsson Tampe; +Cc: bug-guile, Michael Burschik, Neil Jerram
Hi Stefan,
I appreciate very much your work on the bugs! Your fixes have been
thoughtful and mostly correct, it seems to me.
In this case, your approach sounds about right. There are a number of
style errors that need fixing, though:
diff --git a/libguile/goops.c b/libguile/goops.c
index c2eb88f..c33bf0c 100644
--- a/libguile/goops.c
+++ b/libguile/goops.c
@@ -193,6 +193,7 @@ static SCM scm_make_extended_class_from_symbol (SCM type_name_sym,
int applicablep);
+
Please remove this empty-line insertion.
@@ -437,16 +438,88 @@ remove_duplicate_slots (SCM l, SCM res, SCM slots_already_seen)
return remove_duplicate_slots (SCM_CDR (l), res, slots_already_seen);
}
+static int has_class_super(SCM cplp)
"static int" on its own line, and a space before the paren, please.
+{
+ /* check to see if it is derived from <class> */
+ for( ; scm_is_pair(cplp) ; cplp = SCM_CDR(cplp))
No spaces before the semicolons, and spaces before the parens.
+ {
+ if(SCM_CAR(cplp) == scm_class_class)
+ {
+ return 1;
+ }
+ }
+ return 0;
+}
No tabs, only spaces please.
+
+
+static void check_cpl(SCM slots, SCM bslots)
Same, and no need for two blank lines.
+{
+ for(;scm_is_pair(bslots) ; bslots = SCM_CDR(bslots))
+ {
+ SCM pt;
+ SCM b = SCM_CAAR(bslots);
+
+ for(pt = slots ;scm_is_pair(pt) ; pt = SCM_CDR(pt))
+ {
+ if(SCM_CAAR(pt) == b)
+ scm_misc_error ("init-object", "a predefined <class> inherrited field cannot be redefined", SCM_EOL);
+
+ }
+ }
+}
Here we need spaces *after* each paren, and try to wrap lines to 72
chars, with a maximum of 80. It is "inherited".
There are more style errors of this sort. Can you post another patch
with these errors fixed?
Thanks,
Andy
--
http://wingolog.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug #25525] Segfault using goops
2011-08-21 10:54 ` Stefan Israelsson Tampe
@ 2011-09-05 12:25 ` Stefan Israelsson Tampe
2011-10-20 22:32 ` bug#9815: " Andy Wingo
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Israelsson Tampe @ 2011-09-05 12:25 UTC (permalink / raw)
To: Neil Jerram, Andy Wingo, Michael Burschik,
Stefan Israelsson Tampe, bug-guile
Follow-up Comment #7, bug #25525 (project guile):
added goops4.diff, a git diff that fixes c stylish errors and use
scm_is_eq in stead of a direct comparision.
/Stefan
(file #23943)
_______________________________________________________
Additional Item Attachment:
File name: goops4.diff Size:3 KB
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9815: [bug #25525] Segfault using goops
2011-09-05 12:25 ` Stefan Israelsson Tampe
@ 2011-10-20 22:32 ` Andy Wingo
0 siblings, 0 replies; 10+ messages in thread
From: Andy Wingo @ 2011-10-20 22:32 UTC (permalink / raw)
To: neil, wingo, Michael.Burschik, stefan.itampe, 9815
Update of bug #25525 (project guile):
Status: None => Fixed
Open/Closed: Open => Closed
_______________________________________________________
Follow-up Comment #8:
Thank you for the fix, Stefan. I have pushed it to git along with a test
case.
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/bugs/?25525>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-10-20 22:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-09 8:25 [bug #25525] Segfault using goops Michael Burschik
2009-02-09 23:17 ` Neil Jerram
2010-04-16 11:26 ` Stefan Israelsson Tampe
2010-04-16 11:36 ` Stefan Israelsson Tampe
2010-06-07 10:00 ` Andy Wingo
2011-08-20 18:14 ` Stefan Israelsson Tampe
2011-08-21 10:54 ` Stefan Israelsson Tampe
2011-09-05 12:25 ` Stefan Israelsson Tampe
2011-10-20 22:32 ` bug#9815: " Andy Wingo
2011-09-02 17:57 ` Andy Wingo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).