Madhu critical fixes 2023 02 1 #94
Conversation
This cleans up some code introduced in commit 95c96a2
When transfer is :everything and the object is not floating the object does not get reffed, and crashes glib when its finalizer runs.
(slot-value length-arg 'value) => (length (slot-value length-arg 'value)) HISTORY: "Second Madhu patch. Fix for length-arg." commit 95c96a2 (kalimehtar 2018-06-24) was just wrong. (sorry) "Fix array length detection in in-arg-clear" commit 82a8759 (andy128k 2019-12-31) was also wrong. Hopefully this fixes it.
class object-pointer-type has superclaasses (pointer-type interface-type). the (initialize-copy object-pointer-type) method uses (call-next-method) to copy superclass slots - However the (initialize-copy interface-type) method never gets called, and we end up with an error like: Slot GIR::NAMESPACE is unbound in #<GIR::OBJECT-POINTER-TYPE (although the slot has a NIL iniform!) This is because the pointer-type class has a superclass freeable-type. And the (initialize-copy freeable-type) method did not invoke call-next-method. Although freeable-type has no super-classes, this method has to invoke call-next-method so that other initialize-copy methods which may exist can run. Make it so. BTW (trace intiazlize-copy copy-instance) at least on CCL does not show the problem as the final calls via (call-next-method) are not shown for some reason.
This fixes a crash when GValues are allocated during function-invocation through g-i. The GValue is directly allocated rather than allocated via make-gvalue (which takes care to initialize it)
SHARED-INITIALIZE gets called via UPDATE-INSTANCE-FOR-REDEFINED-TYPE
when the class gets redefined and MAKE-INSTANCES-OBSOLETE gets called
on extant objects. In this case we should not reinitialize the object
since we are not creating it for the first time and don't have access
to the initargs which are supplied at creation time.
This bug is a shortcoming of the design which uses SHARED-INITIALIZE
for the purpose.
--
The error thrown by ccl is obscure.
If you recompile all files then objects such as
(setq *gobject* (require-namespace "GObject"))
become obsolete. ccl sets slots to nil.
Faslload shows this:
NIL has no slot named GIR::PTR.
[Condition of type SIMPLE-ERROR]
1: (SLOT-VALUE NIL GIR::PTR)
2: (#<STANDARD-METHOD CFFI:TRANSLATE-TO-FOREIGN (T GIR::INFO-FFI)> NIL #<GIR::INFO-FFI #x3020017796CD>)
3: (#:G-OBJECT-INFO-GET-PARENT NIL)
4: (GIR:OBJECT-INFO-GET-PARENT NIL)
5: (#<STANDARD-METHOD SHARED-INITIALIZE :AFTER (GIR:OBJECT-CLASS T)> #<error printing GIR:OBJECT-CLASS #x30200171571D> NIL :INFO NIL)
9: (CCL::UPDATE-OBSOLETE-INSTANCE #<error printing GIR:OBJECT-CLASS #x30200171571D>)
* (require-namespace "GObject")
NIL is not a Lisp string or pointer.
[Condition of type SIMPLE-ERROR]
Backtrace:
0: (#<STANDARD-METHOD CFFI:TRANSLATE-TO-FOREIGN (T CFFI::FOREIGN-STRING-TYPE)> NIL #<FOREIGN-STRING-TYPE :UTF-8>)
1: (G-IREPOSITORY-REQUIRE NIL NIL #<A Null Foreign Pointer> NIL #<A Foreign Pointer [stack-allocated] #x7EFD04742B70>)
2: (REPOSITORY-REQUIRE NIL NIL #<A Null Foreign Pointer> NIL)
3: (#<STANDARD-METHOD SHARED-INITIALIZE :AFTER (NAMESPACE T)> #N<GObject(2.0)> NIL :NAME NIL :VERSION NIL)
4: (CCL::%%BEFORE-AND-AFTER-COMBINED-METHOD-DCODE (NIL #<STANDARD-METHOD SHARED-INITIALIZE :AFTER (NAMESPACE T)> . 17453100824688))
5: (CCL::%%STANDARD-COMBINED-METHOD-DCODE (NIL (#<STANDARD-METHOD SHARED-INITIALIZE :AFTER (NAMESPACE T)>) #<CCL::STANDARD-KERNEL-METHOD SHARED-INITIALIZE (STANDARD-OBJECT T)>) 17453100824688)
6: (NIL #<Unknown Arguments>)
7: (CCL::UPDATE-OBSOLETE-INSTANCE #N<GObject(2.0)>)
;madhu 220430 - missed a spot:
ccl calls CCL::UPDATE-OBSOLETE-INSTANCE #E<SignalMatchType> in
signal.lisp which calls shared-initialize, and this fails
NIL has no slot named PTR.
[Condition of type SIMPLE-ERROR]
Restarts:
0: [LOAD-SOURCE] Load "home:cl;extern;cl-gobject-introspection;src;signal.lisp" instead of "/home/madhu/fasl/cl-gobject-introspection/binary-ccl/Version 1.12.1 (v1.12.1-8-gc2e8457) LinuxX8664/src/signal.lx64fsl"
1: [RECOMPILE] Compile "home:cl;extern;cl-gobject-introspection;src;signal.lisp" into "/home/madhu/fasl/cl-gobject-introspection/binary-ccl/Version 1.12.1 (v1.12.1-8-gc2e8457) LinuxX8664/src/signal.lx64fsl" then load "/home/madhu/fasl/cl-gobject-introspection/binary-ccl/Version 1.12.1 (v1.12.1-8-gc2e8457) LinuxX8664/src/signal.lx64fsl" again
5: (CCL::%%STANDARD-COMBINED-METHOD-DCODE (NIL (#<STANDARD-METHOD SHARED-INITIALIZE :AFTER (ENUM-DESC T)>) #<CCL::STANDARD-KERNEL-METHOD SHARED-INITIALIZE (STANDARD-OBJECT T)>) 17471111621578)
6: (NIL #<Unknown Arguments>)
7: (CCL::UPDATE-OBSOLETE-INSTANCE #E<SignalMatchType>)
Locals:
INSTANCE = #E<SignalMatchType>
ADDED = NIL
DISCARDED = NIL
PLIST = NIL
8: (CCL::%SLOT-ID-REF-OBSOLETE #E<SignalMatchType> #<SLOT-ID for VALUES-DICT/970 #x3020013BCFAD>)
9: ((:INTERNAL BUILD-ENUM) :ID)
Locals:
NAME = :ID
NAME* = "id"
#:G4163 = #E<SignalMatchType>
10: (CCL::$FASL-LFUNCALL #<CCL::FASLSTATE #x7F1E7B15A5CD>)
* src/object.lisp: (param-spect-setup-gc): new, with supporting functions. call this instead of object-setup-gc when creating a `ParamSpec' objects.
…ects" * src/object.lisp: (gobject): opt out entirely from the fake-object-class code-paths and create object-instances (for classes whose gtypes aren't in g-i, i.e. not in the gir file) with the gtype of first interface in the class interface list. ;230206 * src/object.lisp: (use-fake-objects): new variable. defualts to nil. set to t to enable the code path with fake objects
;230212 add an assert in commented out code to make sure you do not overwrite an existing print-object around method
…gs without crashing lisp the `with-foreign-alloc'ed `values-out' in build-function was being used incorrectly with inout arguments. fix a memory-overflow in in-arg-setup by always allocating memory, before letting in-arg-setup stomp it. * src/function.lisp: (mem-alloc): methods for struct-type and union-type. (make-arg): allocate memory at voutp on values-out array, and point the giargs-in and giargs-out to it. * src/function.lisp: (out-arg->value) try to free the giarg allocated for an "inout" argument before returning. NOTE: out-args are not generally freed except for what happens via mem-get in not out-arg->value. This needs further work. Also see the commit message in c27fdb6
| @@ -174,17 +215,25 @@ | |||
| :name (cffi:foreign-funcall "g_type_name" | |||
| :ulong gtype :string))))))) | |||
|
|
|||
| (defvar use-fake-objects nil) | |||
There was a problem hiding this comment.
What is the purpose of this variable?
There was a problem hiding this comment.
Commit 95c96a2 ("Second Madhu patch. Fix for length-arg") introduced a code path that created "fake-objects" for types that are not exposed through g-i. e.g. what is returned by (gir:invoke (gio "File" "new_for_path") "/etc/passwd"). - g_file_new_for_path is defined as returning an GFile which is GInterface, the actual type is some implementation dependent LocalFile type which is not available through g-i.
However I thought this code path may not be necessary , taking into account. Roman's commit 12f8910
if it were suitably extended. This patch implements the code path which Roman probably intended
to be the default behaviour, while retaining the fake-object creation code path.
this variable provides a way to toggle the codepath -- I still think
the fake-object code path may be necessary in some situations but I don't know what they are
yet.
I tried explaining this briefly in the commit message.
There was a problem hiding this comment.
There are problems with this fix: while it fixes some buffer overflows
involving in-out args it breaks common cases such as Gtk3.init
#F<init(#V<argv: (SEQUENCE STRING)>): (#V<RETURN-VALUE: VOID>
#V<argv: (SEQUENCE STRING)>)>
When it is invoked
as (invoke (gtk "init") nil)
which can be patched
but there are deeper issues with the annotations for this function
which will lead to problems
with (invoke (gtk "init") '("foo "bar" "car"))
where I can see no good way to solve the mem-alloc issue.where
a c-array has to be allocated before the arg-lengths can be fixed up.
So I think it was a good idea keeping this pull request on hold
any advice will be appreciated
|
@enometh I have cherry-picked most of your changes. They are already in When it comes to passing arrays, I was not able to reproduce any issue (gtk init works on my machine, he-he). |
Hello, I've cherry-picked these commits from my experimental branch for merging , as I think these patches
are critical - They haven't been tested in isolation (Some of this fixes earlier code committed on my behalf.).
Please review..