bt2: Add wrapper for bt_plugin_get_version

There is a subtle bug with our use of the "char **OUT" typemap for the
bt_plugin_get_version function.

THe SWIG wrapper does this:

    char *temp_value5;
    arg5 = &temp_value5;

The initial content of temp_value5 is unknown, assume that it points to
unreadable memory.  If the call to bt_plugin_get_version returns
PROPERTY_AVAILABILITY_NOT_AVAILABLE, the value of temp_value5 is
unchanged (but really, it could be anything, again we should assume it
points to unreadable memory).  However, the epilogue of the typemap will
still try to transform the C string to a Python str object.  Doing so
will cause a crash when trying to read the C string.

The fix is to make our own wrapper to bt_plugin_get_version, which
guarantees that if it returns PROPERTY_AVAILABILITY_NOT_AVAILABLE, it
will force the value of the output parameter to be NULL (which is
handled by the epilogue and transformed to None).

Setting the temporary variable to an invalid but non-zero pointer value
((void *) 1) makes this bug consistently reproducible and not rely on
random memory contents.  I suggest we keep it there, in order to quickly
find other instances of this bug in the future.

Change-Id: Idd12ca4c4d4d732b841c1e12420e46e6a5a25874
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1221
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>

diff --git a/bindings/python/bt2/bt2/native_bt.i b/bindings/python/bt2/bt2/native_bt.i
index b1e2d27df70382341801768cd98dc0ab918401d8..fe289b0bd0139d63f729589551538265b980008a 100644 (file)
--- a/bindings/python/bt2/bt2/native_bt.i
+++ b/bindings/python/bt2/bt2/native_bt.i
@@ -52,8 +52,16 @@ typedef int bt_bool;
 %rename("%(strip:[bt_])s", %$isvariable) "";
 %rename("%(strip:[BT_])s", %$isenumitem) "";
 
-/* Output argument typemap for string output (always appends) */
-%typemap(in, numinputs=0) (const char **OUT) (char *temp_value) {
+/*
+ * Output argument typemap for string output (always appends)
+ *
+ * We initialize the output parameter `temp_value` to an invalid but non-zero
+ * pointer value.  This is to make sure we don't rely on its initial value in
+ * the epilogue (where we call SWIG_Python_str_FromChar).  When they fail,
+ * functions on which we apply this typemap don't guarantee that the value of
+ * `temp_value` will be unchanged or valid.
+ */
+%typemap(in, numinputs=0) (const char **OUT) (char *temp_value = (void *) 1) {
        $1 = &temp_value;
 }
 

diff --git a/bindings/python/bt2/bt2/native_bt_plugin.i b/bindings/python/bt2/bt2/native_bt_plugin.i
index 640085beea076f433975d96fd8366cfd6acde92c..435d40f2ed7de5b86243d5e26d18bf6710d72baa 100644 (file)
--- a/bindings/python/bt2/bt2/native_bt_plugin.i
+++ b/bindings/python/bt2/bt2/native_bt_plugin.i
@@ -96,3 +96,36 @@ extern const bt_plugin *bt_plugin_set_borrow_plugin_by_index_const(
 extern void bt_plugin_set_get_ref(const bt_plugin_set *plugin_set);
 
 extern void bt_plugin_set_put_ref(const bt_plugin_set *plugin_set);
+
+/* Helpers */
+
+bt_property_availability bt_plugin_get_version_wrapper(
+               const bt_plugin *plugin, unsigned int *OUT,
+               unsigned int *OUT, unsigned int *OUT, const char **OUT);
+
+%{
+
+/*
+ * This wrapper ensures that when the API function fails, the `*extra` output
+ * parameter is set to NULL.  This is necessary because the epilogue of the
+ * "char **OUT" typemap will use that value to make a Python str object.  We
+ * can't rely on the initial value of `*extra`, it could point to unreadable
+ * memory.
+ */
+
+bt_property_availability bt_plugin_get_version_wrapper(
+               const bt_plugin *plugin, unsigned int *major,
+               unsigned int *minor, unsigned int *patch, const char **extra)
+{
+       bt_property_availability ret;
+
+       ret = bt_plugin_get_version(plugin, major, minor, patch, extra);
+
+       if (ret == BT_PROPERTY_AVAILABILITY_NOT_AVAILABLE) {
+               *extra = NULL;
+       }
+
+       return ret;
+}
+
+%}

diff --git a/bindings/python/bt2/bt2/plugin.py b/bindings/python/bt2/bt2/plugin.py
index 51f9a2ed85f1b7591a693ac02e0d48bf4a0fad10..39223a8ce6f255b9878fe2bbec3cf4c6ba5ded0a 100644 (file)
--- a/bindings/python/bt2/bt2/plugin.py
+++ b/bindings/python/bt2/bt2/plugin.py
@@ -198,7 +198,7 @@ class _Plugin(object._SharedObject):
 
     @property
     def version(self):
-        status, major, minor, patch, extra = native_bt.plugin_get_version(self._ptr)
+        status, major, minor, patch, extra = native_bt.plugin_get_version_wrapper(self._ptr)
 
         if status == native_bt.PROPERTY_AVAILABILITY_NOT_AVAILABLE:
             return