Use uint64_t instead of long for pointers represented in C

While long is generally correct, the conversion of pointers to
long, then int64_t (passed to Java), then back to pointers
results in poor sign-extension, giving us invalid pointers.
Instead, convert to uint64_t everywhere and things work.

diff --git a/gen_type_mapping.py b/gen_type_mapping.py
index d9bcf976776754ea1a9e7521f14e78d9e83b2c07..cbec2ee98a3fad009297d1aa275d87300fcd0a88 100644 (file)
--- a/gen_type_mapping.py
+++ b/gen_type_mapping.py
@@ -239,12 +239,12 @@ class TypeMappingGenerator:
                 elif not holds_ref and ty_info.is_ptr:
                     opaque_ret_conv_suf = opaque_ret_conv_suf + "// Warning: we may need a move here but no clone is available for " + ty_info.rust_obj + "\n"
 
-                opaque_ret_conv_suf = opaque_ret_conv_suf + "CHECK((((long)" + ty_info.var_name + "_var.inner) & 1) == 0); // We rely on a free low bit, malloc guarantees this.\n"
-                opaque_ret_conv_suf = opaque_ret_conv_suf + "CHECK((((long)&" + ty_info.var_name + "_var) & 1) == 0); // We rely on a free low bit, pointer alignment guarantees this.\n"
+                opaque_ret_conv_suf = opaque_ret_conv_suf + "CHECK((((uint64_t)" + ty_info.var_name + "_var.inner) & 1) == 0); // We rely on a free low bit, malloc guarantees this.\n"
+                opaque_ret_conv_suf = opaque_ret_conv_suf + "CHECK((((uint64_t)&" + ty_info.var_name + "_var) & 1) == 0); // We rely on a free low bit, pointer alignment guarantees this.\n"
                 if holds_ref:
-                    opaque_ret_conv_suf = opaque_ret_conv_suf + "long " + ty_info.var_name + "_ref = (long)" + ty_info.var_name + "_var.inner & ~1;"
+                    opaque_ret_conv_suf = opaque_ret_conv_suf + "uint64_t " + ty_info.var_name + "_ref = (uint64_t)" + ty_info.var_name + "_var.inner & ~1;"
                 else:
-                    opaque_ret_conv_suf = opaque_ret_conv_suf + "long " + ty_info.var_name + "_ref = (long)" + ty_info.var_name + "_var.inner;\n"
+                    opaque_ret_conv_suf = opaque_ret_conv_suf + "uint64_t " + ty_info.var_name + "_ref = (uint64_t)" + ty_info.var_name + "_var.inner;\n"
                     opaque_ret_conv_suf = opaque_ret_conv_suf + "if (" + ty_info.var_name + "_var.is_owned) {\n"
                     opaque_ret_conv_suf = opaque_ret_conv_suf + "\t" + ty_info.var_name + "_ref |= 1;\n"
                     opaque_ret_conv_suf = opaque_ret_conv_suf + "}"
@@ -296,7 +296,7 @@ class TypeMappingGenerator:
                         base_conv = base_conv + "\n" + "FREE((void*)" + ty_info.var_name + ");"
                     return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                         arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                        ret_conv = ret_conv, ret_conv_name = "(long)ret",
+                        ret_conv = ret_conv, ret_conv_name = "(uint64_t)ret",
                         to_hu_conv = ty_info.java_hu_ty + " ret_hu_conv = new " + ty_info.java_hu_ty + "(null, " + ty_info.var_name + ");\nret_hu_conv.ptrs_to.add(this);",
                         to_hu_conv_name = "ret_hu_conv",
                         from_hu_conv = (ty_info.var_name + " == null ? 0 : " + ty_info.var_name + ".ptr", "this.ptrs_to.add(" + ty_info.var_name + ")"))
@@ -313,7 +313,7 @@ class TypeMappingGenerator:
                     # underlying unlike Vecs, and it gives Java more freedom.
                     base_conv = base_conv + "\nFREE((void*)" + ty_info.var_name + ");"
                 if ty_info.rust_obj in self.complex_enums:
-                    ret_conv = ("long " + ty_info.var_name + "_ref = ((long)&", ") | 1;")
+                    ret_conv = ("uint64_t " + ty_info.var_name + "_ref = ((uint64_t)&", ") | 1;")
                     if not holds_ref:
                         ret_conv = (ty_info.rust_obj + " *" + ty_info.var_name + "_copy = MALLOC(sizeof(" + ty_info.rust_obj + "), \"" + ty_info.rust_obj + "\");\n", "")
                         if ty_info.requires_clone == True: # Set in object array mapping
@@ -323,7 +323,7 @@ class TypeMappingGenerator:
                                 ret_conv = (ret_conv[0] + "*" + ty_info.var_name + "_copy = ", "; // Warning: We likely need to clone here, but no clone is available for " + ty_inf.rust_obj + "\n")
                         else:
                             ret_conv = (ret_conv[0] + "*" + ty_info.var_name + "_copy = ", ";\n")
-                        ret_conv = (ret_conv[0], ret_conv[1] + "long " + ty_info.var_name + "_ref = (long)" + ty_info.var_name + "_copy;")
+                        ret_conv = (ret_conv[0], ret_conv[1] + "uint64_t " + ty_info.var_name + "_ref = (uint64_t)" + ty_info.var_name + "_copy;")
                     return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                         arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
                         ret_conv = ret_conv, ret_conv_name = ty_info.var_name + "_ref",
@@ -339,7 +339,7 @@ class TypeMappingGenerator:
                         ret_conv = (ty_info.rust_obj + "* " + ty_info.var_name + "_conv = MALLOC(sizeof(" + ty_info.rust_obj + "), \"" + ty_info.rust_obj + "\");\n*" + ty_info.var_name + "_conv = ", ";")
                     return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                         arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                        ret_conv = ret_conv, ret_conv_name = "(long)" + ty_info.var_name + "_conv",
+                        ret_conv = ret_conv, ret_conv_name = "(uint64_t)" + ty_info.var_name + "_conv",
                         to_hu_conv = ty_info.java_hu_ty + " " + ty_info.var_name + "_hu_conv = " + ty_info.java_hu_ty + ".constr_from_ptr(" + ty_info.var_name + ");",
                         to_hu_conv_name = ty_info.var_name + "_hu_conv", from_hu_conv = (ty_info.var_name + " != null ? " + ty_info.var_name + ".ptr : 0", ""))
                 if ty_info.rust_obj in self.tuple_types:
@@ -389,11 +389,11 @@ class TypeMappingGenerator:
                         return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                             arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
                             ret_conv = ret_conv,
-                            ret_conv_name = "(long)" + ty_info.var_name + "_ref",
+                            ret_conv_name = "(uint64_t)" + ty_info.var_name + "_ref",
                             to_hu_conv = to_hu_conv, to_hu_conv_name = ty_info.var_name + "_conv", from_hu_conv = (from_hu_conv + ")", from_hu_conv_sfx))
                     return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                         arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                        ret_conv = ("long " + ty_info.var_name + "_ref = (long)(&", ") | 1;"), ret_conv_name = ty_info.var_name + "_ref",
+                        ret_conv = ("uint64_t " + ty_info.var_name + "_ref = (uint64_t)(&", ") | 1;"), ret_conv_name = ty_info.var_name + "_ref",
                         to_hu_conv = to_hu_conv, to_hu_conv_name = ty_info.var_name + "_conv", from_hu_conv = (from_hu_conv + ")", from_hu_conv_sfx))
 
                 # The manually-defined types - TxOut and u5
@@ -408,10 +408,10 @@ class TypeMappingGenerator:
                 if not ty_info.is_ptr and not holds_ref:
                     ret_conv = ("LDKTxOut* " + ty_info.var_name + "_ref = MALLOC(sizeof(LDKTxOut), \"LDKTxOut\");\n*" + ty_info.var_name + "_ref = ", ";")
                 else:
-                    ret_conv = ("long " + ty_info.var_name + "_ref = ((long)&", ") | 1;")
+                    ret_conv = ("uint64_t " + ty_info.var_name + "_ref = ((uint64_t)&", ") | 1;")
                 return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                     arg_conv = base_conv, arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                    ret_conv = ret_conv, ret_conv_name = "(long)" + ty_info.var_name + "_ref",
+                    ret_conv = ret_conv, ret_conv_name = "(uint64_t)" + ty_info.var_name + "_ref",
                     to_hu_conv = ty_info.java_hu_ty + " " + ty_info.var_name + "_conv = new " +ty_info.java_hu_ty + "(null, " + ty_info.var_name + ");",
                     to_hu_conv_name = ty_info.var_name + "_conv", from_hu_conv = (ty_info.var_name + ".ptr", ""))
             elif ty_info.is_ptr:
@@ -420,7 +420,7 @@ class TypeMappingGenerator:
                     return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                         arg_conv = ty_info.rust_obj + "* " + ty_info.var_name + "_conv = (" + ty_info.rust_obj + "*)" + ty_info.var_name + ";",
                         arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                        ret_conv = ("long ret_" + ty_info.var_name + " = (long)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
+                        ret_conv = ("uint64_t ret_" + ty_info.var_name + " = (uint64_t)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
                         to_hu_conv = ty_info.java_hu_ty + " " + ty_info.var_name + "_hu_conv = " + ty_info.java_hu_ty + ".constr_from_ptr(" + ty_info.var_name + ");",
                         to_hu_conv_name = ty_info.var_name + "_hu_conv",
                         from_hu_conv = (ty_info.var_name + " == null ? 0 : " + ty_info.var_name + ".ptr & ~1", "this.ptrs_to.add(" + ty_info.var_name + ")"))
@@ -445,20 +445,20 @@ class TypeMappingGenerator:
                             arg_conv = arg_conv, arg_conv_name = arg_conv_name, arg_conv_cleanup = None,
                             ret_conv = (ty_info.rust_obj + " *" + ty_info.var_name + "_clone = MALLOC(sizeof(" + ty_info.rust_obj + "), \"" + ty_info.rust_obj + "\");\n" +
                                 "*" + ty_info.var_name + "_clone = " + ty_info.rust_obj.replace("LDK", "") + "_clone(", ");"),
-                            ret_conv_name = "(long)" + ty_info.var_name + "_clone",
+                            ret_conv_name = "(uint64_t)" + ty_info.var_name + "_clone",
                             to_hu_conv = ty_info.java_hu_ty + " ret_hu_conv = new " + ty_info.java_hu_ty + "(null, " + ty_info.var_name + ");\nret_hu_conv.ptrs_to.add(this);",
                             to_hu_conv_name = "ret_hu_conv",
                             from_hu_conv = (ty_info.var_name + " == null ? 0 : " + ty_info.var_name + ".ptr", "this.ptrs_to.add(" + ty_info.var_name + ")"))
                     else:
                         return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                             arg_conv = arg_conv, arg_conv_name = arg_conv_name, arg_conv_cleanup = None,
-                            ret_conv = ("long ret_" + ty_info.var_name + " = (long)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
+                            ret_conv = ("uint64_t ret_" + ty_info.var_name + " = (uint64_t)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
                             to_hu_conv = ty_info.java_hu_ty + " ret_hu_conv = new " + ty_info.java_hu_ty + "(null, " + ty_info.var_name + ");\nret_hu_conv.ptrs_to.add(this);",
                             to_hu_conv_name = "ret_hu_conv",
                             from_hu_conv = (ty_info.var_name + " == null ? 0 : " + ty_info.var_name + ".ptr", "this.ptrs_to.add(" + ty_info.var_name + ")"))
                 return ConvInfo(ty_info = ty_info, arg_name = ty_info.var_name,
                     arg_conv = ty_info.rust_obj + "* " + ty_info.var_name + "_conv = (" + ty_info.rust_obj + "*)(" + ty_info.var_name + " & ~1);",
                     arg_conv_name = ty_info.var_name + "_conv", arg_conv_cleanup = None,
-                    ret_conv = ("long ret_" + ty_info.var_name + " = (long)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
+                    ret_conv = ("uint64_t ret_" + ty_info.var_name + " = (uint64_t)", ";"), ret_conv_name = "ret_" + ty_info.var_name,
                     to_hu_conv = "TODO 3", to_hu_conv_name = None, from_hu_conv = None) # its a pointer, no conv needed
             assert False # We should have handled every case by now.

diff --git a/genbindings.py b/genbindings.py
index 008084d29796f47cc0370b199d09107ae6f269ad..f35932ef39257637c61a3ce237fed7cf7dd7cb18 100755 (executable)
--- a/genbindings.py
+++ b/genbindings.py
@@ -695,7 +695,7 @@ with open(sys.argv[1]) as in_h, open(sys.argv[2], "w") as out_java:
                     write_c("\tret->" + e + " = " + e + ";\n")
                 if ty_info.arg_conv_cleanup is not None:
                     write_c("\t//TODO: Really need to call " + ty_info.arg_conv_cleanup + " here\n")
-        write_c("\treturn (long)ret;\n")
+        write_c("\treturn (uint64_t)ret;\n")
         write_c("}\n")
 
         for idx, ty_info in enumerate(ty_list):
@@ -855,7 +855,7 @@ with open(sys.argv[1]) as in_h, open(sys.argv[2], "w") as out_java:
                         if cleanup is not None:
                             write_c("\t\t" + cleanup + ";\n")
                         write_c("\t}\n")
-                        write_c("\treturn (long)ret;\n")
+                        write_c("\treturn (uint64_t)ret;\n")
                         write_c("}\n")
 
                     if ty_info.is_native_primitive:

diff --git a/java_strings.py b/java_strings.py
index 1987c10e58db6d3915cdf7e2ba7f8fe1aafb1a2f..a616a24a38f8de72c9f1ab0643ba9d06f0a5c899 100644 (file)
--- a/java_strings.py
+++ b/java_strings.py
@@ -126,7 +126,7 @@ void __attribute__((constructor)) spawn_stderr_redirection() {
 
         if not DEBUG:
             self.c_file_pfx = self.c_file_pfx + """#define MALLOC(a, _) malloc(a)
-#define FREE(p) if ((long)(p) > 1024) { free(p); }
+#define FREE(p) if ((uint64_t)(p) > 1024) { free(p); }
 #define DO_ASSERT(a) (void)(a)
 #define CHECK(a)
 """
@@ -256,7 +256,7 @@ static void alloc_freed(void* ptr) {
        __real_free(it);
 }
 static void FREE(void* ptr) {
-       if ((long)ptr < 1024) return; // Rust loves to create pointers to the NULL page for dummys
+       if ((uint64_t)ptr < 1024) return; // Rust loves to create pointers to the NULL page for dummys
        alloc_freed(ptr);
        __real_free(ptr);
 }
@@ -345,14 +345,14 @@ JNIEXPORT int64_t impl_bindings_bytes_1to_1u8_1vec (JNIEnv * env, jclass _b, jby
        vec->datalen = (*env)->GetArrayLength(env, bytes);
        vec->data = (uint8_t*)MALLOC(vec->datalen, "LDKCVec_u8Z Bytes");
        (*env)->GetByteArrayRegion (env, bytes, 0, vec->datalen, vec->data);
-       return (long)vec;
+       return (uint64_t)vec;
 }
 JNIEXPORT jbyteArray JNICALL Java_org_ldk_impl_bindings_txpointer_1get_1buffer (JNIEnv * env, jclass _b, jlong ptr) {
        LDKTransaction *txdata = (LDKTransaction*)ptr;
        LDKu8slice slice;
        slice.data = txdata->data;
        slice.datalen = txdata->datalen;
-       return Java_org_ldk_impl_bindings_get_1u8_1slice_1bytes(env, _b, (long)&slice);
+       return Java_org_ldk_impl_bindings_get_1u8_1slice_1bytes(env, _b, (uint64_t)&slice);
 }
 JNIEXPORT int64_t JNICALL Java_org_ldk_impl_bindings_new_1txpointer_1copy_1data (JNIEnv * env, jclass _b, jbyteArray bytes) {
        LDKTransaction *txdata = (LDKTransaction*)MALLOC(sizeof(LDKTransaction), "LDKTransaction");
@@ -360,7 +360,7 @@ JNIEXPORT int64_t JNICALL Java_org_ldk_impl_bindings_new_1txpointer_1copy_1data
        txdata->data = (uint8_t*)MALLOC(txdata->datalen, "Tx Data Bytes");
        txdata->data_is_owned = false;
        (*env)->GetByteArrayRegion (env, bytes, 0, txdata->datalen, txdata->data);
-       return (long)txdata;
+       return (uint64_t)txdata;
 }
 JNIEXPORT void JNICALL Java_org_ldk_impl_bindings_txpointer_1free (JNIEnv * env, jclass _b, jlong ptr) {
        LDKTransaction *tx = (LDKTransaction*)ptr;
@@ -375,7 +375,7 @@ JNIEXPORT jlong JNICALL Java_org_ldk_impl_bindings_vec_1slice_1len (JNIEnv * env
        _Static_assert(offsetof(LDKCVec_u8Z, datalen) == offsetof(LDKCVec_EventZ, datalen), "Vec<*> needs to be mapped identically");
        _Static_assert(offsetof(LDKCVec_u8Z, datalen) == offsetof(LDKCVec_C2Tuple_usizeTransactionZZ, datalen), "Vec<*> needs to be mapped identically");
        LDKCVec_u8Z *vec = (LDKCVec_u8Z*)ptr;
-       return (long)vec->datalen;
+       return (uint64_t)vec->datalen;
 }
 JNIEXPORT int64_t JNICALL Java_org_ldk_impl_bindings_new_1empty_1slice_1vec (JNIEnv * env, jclass _b) {
        // Check sizes of a few Vec types are all consistent as we're meant to be generic across types
@@ -386,7 +386,7 @@ JNIEXPORT int64_t JNICALL Java_org_ldk_impl_bindings_new_1empty_1slice_1vec (JNI
        LDKCVec_u8Z *vec = (LDKCVec_u8Z*)MALLOC(sizeof(LDKCVec_u8Z), "Empty LDKCVec");
        vec->data = NULL;
        vec->datalen = 0;
-       return (long)vec;
+       return (uint64_t)vec;
 }
 
 // We assume that CVec_u8Z and u8slice are the same size and layout (and thus pointers to the two can be mixed)
@@ -889,7 +889,7 @@ import java.util.Arrays;
             else:
                 out_c = out_c + ", " + var[1]
         out_c = out_c + ");\n"
-        out_c = out_c + "\treturn (long)res_ptr;\n"
+        out_c = out_c + "\treturn (uint64_t)res_ptr;\n"
         out_c = out_c + "}\n"
 
         return (out_java, out_java_trait, out_c)