[gjs/wip/smcv/issue319: 2/2] function: Don't assume FFI argument always matches GIArgument




commit 4bcb5c6c479c24baacf29c3213fd9b5ec9e94939
Author: Simon McVittie <smcv debian org>
Date:   Sun Aug 23 18:13:17 2020 +0100

    function: Don't assume FFI argument always matches GIArgument
    
    Everywhere else in gjs_callback_closure() that writes into result
    does so by first converting the JavaScript object into a GIArgument
    according to GIArgument conventions, and then copying the GIArgument
    into result according to FFI conventions. In particular, for small
    integers like enums and flags, the value copied into the GIArgument
    is 32-bit, but the value pointed to by result is typically
    pointer-sized, which is larger on 64-bit platforms.
    
    However, the code path for vfuncs that fail at the JavaScript level, but
    have to return *something* at the C level, was instead treating result as
    being a pointer to a GIArgument. Writing a small integer to a GIArgument
    only sets the first few bits (the first 32 for enums and flags),
    leaving the next 32 bits of a pointer-sized quantity on a 64-bit
    platform uninitialized. On little-endian CPUs, if the next 32 bits
    happen to already be all-zeroes, the right thing would happen anyway,
    but we can't count on that.
    
    In particular, this resulted in the test-case
    "Wrong virtual functions marshals an enum return value" in GIMarshalling
    failing on big-endian LP64 architectures like s390x, after I fixed the
    other code paths involving GArgument and enums/flags for s390x in
    commit 1ba19d63 "function: Use GIArgument.v_int for enum and flags types".
    
    Resolves: https://gitlab.gnome.org/GNOME/gjs/-/issues/319
    Signed-off-by: Simon McVittie <smcv debian org>

 gi/function.cpp | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index b6f6fe11..20d3f72d 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -502,9 +502,12 @@ out:
         }
 
         /* Fill in the result with some hopefully neutral value */
-        g_callable_info_load_return_type(trampoline->info, &ret_type);
-        gjs_gi_argument_init_default(&ret_type,
-                                     static_cast<GIArgument*>(result));
+        if (!ret_type_is_void) {
+            GIArgument argument = {};
+            g_callable_info_load_return_type(trampoline->info, &ret_type);
+            gjs_gi_argument_init_default(&ret_type, &argument);
+            set_return_ffi_arg_from_giargument(&ret_type, result, &argument);
+        }
 
         /* If the callback has a GError** argument and invoking the closure
          * returned an error, try to make a GError from it */


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]