dbus: Scheme closure passed where function pointer is expected and triple compilation

[sjamaan]

Recently we ran Salmonella with the option -specialize, to find problems with the scrutinizer (flow analysis) and bugs in eggs, and it found a problem with dbus:
​http://parenteses.org/mario/misc/specialize-report/install/dbus.html

Due to the fact that dbus builds with -O3 you can also see this in action in the latest salmonella run based on the git master branch of Chicken: ​http://tests.call-cc.org/master/linux/x86/2012/05/17/salmonella-report/install/dbus.html

The warning is due to the fact that the vtable-message_function is declared as type c-pointer, but a plain Scheme closure object is set to that pointer. This is not correct, and should usually cause a segfault since Scheme objects are not generally usable as pointers. In this case it happens to extract a pointer to the procedure correctly because closures are represented similarly to pointer objects, but Scheme closures expect an argument count and a continuation as the first two parameters.

If you want to pass a C function pointer correctly, I think you'll need to declare a function in C (or use define-external or something similar) and use foreign-value with type c-pointer to reify it as a pointer.

Finally, it looks like the egg's Make recipe in the .setup-file is all wrong; the same file gets compiled up to three times!

This is due to the fact that the rule for dbus.so doesn't use dbus.c but dbus.scm, making the step to generate the .c-file unneccessary. Actually, this file even gets deleted after this is compiled. That causes the dbus.import.scm line to trigger compilation of dbus.c via that line yet again, even though the import file already exists.

Here's a patch to do it more or less correctly, if you really need to split up compilation into two stages (which is completely unneccessary):

Index: dbus.setup
===================================================================
--- dbus.setup	(revision 26708)
+++ dbus.setup	(working copy)
@@ -1,14 +1,14 @@
 ;;;; dbus.setup -*- Scheme -*-
 
-(use files utils)
-
 (make (
-		("dbus.import.so" ("dbus.c")
+		("dbus.import.so" ("dbus.import.scm")
 			(compile -s -O3 -d0 dbus.import.scm))
 		("dbus.so" ("dbus.c")
-			(compile -s -O3 -d1 dbus.scm -C "`pkg-config --cflags dbus-1`" -L "`pkg-config --libs dbus-1`"))
+			(compile -s -O3 -d1 dbus.c -C "`pkg-config --cflags dbus-1`" -L "`pkg-config --libs dbus-1`"))
+                ("dbus.import.scm" ("dbus.scm")
+			(compile -A dbus.scm -X easyffi -j dbus))
 		("dbus.c" ("dbus.scm")
-			(compile -t dbus.scm -O3 -d1 -X easyffi -C -g -j dbus)))
+			(compile -t dbus.scm -O3 -d1 -X easyffi)))
 	'("dbus.so" "dbus.import.so"))
 
 (install-extension 'dbus

I've removed the "(use files utils)" line since it isn't using anything from there.

Notice the fact that to do this, we still need to run the csc command twice on dbus.scm, because when you want the output dbus.c normally a side-effect of running the command is that dbus.import.scm is emitted (via the -j switch). If you want to split it up like this, you need to run the compilation twice. This means the compiler gets invoked twice and has to do all that work again.

Normally you use "make" to avoid building things more than once or over and over again, but the irony is that this setup-file is actually causing it to do exactly that!

You don't need the "make" form at all (which, by the way, is deprecated and ​moved into an egg now).

You can simplify dbus.setup into this (that's the entire file):

;;;; dbus.setup -*- Scheme -*-

(compile -s -O3 -d1 dbus.scm -C "`pkg-config --cflags dbus-1`" -L "`pkg-config --libs dbus-1`" -j dbus))
(compile -s -O3 -d1 dbus.import.scm))

(install-extension 'dbus
	`("dbus.so" "dbus.import.so")
	`((version "0.91")))

[sjamaan]

This should happen with a plain 4.8.0 build now, too.

[Shawn Rutledge]

OK about the .setup file, that's easy. However I'm not sure how to fix the vtable thing.

​http://dbus.freedesktop.org/doc/api/html/group__DBusConnection.html#ga24730ca6fd2e9132873962a32df7628c

explains about being able to register a handler by putting it in a vtable and then calling dbus_connection_register_object_path. My goal is to take a user-provided function taking 3 parameters (because dbus will provide those 3 when it calls the callback) and make a C function pointer out of it. This must be done anonymously - which is why define-external won't work. I can write a function which calls C_callback to invoke the user-provided callback, but that function would have to be given the Scheme closure to be able to call it; but I'm not going to call it, rather dbus is going to call it.

Furthermore I don't have any use for this functionality, there aren't any provided examples that use it, and I wonder if there are any users trying to use this feature at all.

But it ought to be easy to fix, in theory, if the whole FFI wasn't so perpetually confusing and easily-forgotten for me.

salmonella --specialize seems to be undocumented? What does it do? I tried running it in the egg directory, and the output was the same as it was without the --specialize parameter.

~/prj/chicken-eggs/release/4/dbus/trunk
[i7][10:29:06 PM] salmonella --specialize
Using /home/rutledge/prj/chicken-eggs/release/4/dbus/trunk/salmonella-tmp-d24dc as temporary directory

dbus (1 of 1)

Fetching........................................[ ok ] 0s
Reading .meta...................................[ ok ] 0s
Checking dependencies...........................[ ok ] 0s
Checking category...............................[ ok ] 0s
Checking license................................[ ok ] 0s
Checking author.................................[ ok ] 0s
Installing......................................[ ok ] 27s
Checking version................................[ -- ]
Testing.........................................[ -- ]
Checking documentation..........................[ ok ] 1s

*

Summary

Total eggs: 1

Installation

Ok: 1
Failed: 0

Tests

Ok: 0
Failed: 0
No tests: 1

Documentation

Documented: 1
Undocumented: 0

Total run time

28s

[sjamaan]

-specialize is an option to csc (you can pass it in the environment via CSC_OPTIONS), which is now the default on the git master branch.

[sjamaan]

Looks like the egg ​installs fine now