From 95fa8de2c3e9ab36282e42678fcd1c8f9208c5dc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 21:57:12 +0100 Subject: [PATCH 01/12] gmodule: Enable -Wsign-conversion for gmodule subdirectory There was only one `-Wsign-conversion` warning in the whole subdirectory, so that was easy. Signed-off-by: Philip Withnall Helps: #3405 --- gmodule/gmodule.c | 2 +- gmodule/meson.build | 2 +- gmodule/tests/meson.build | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gmodule/gmodule.c b/gmodule/gmodule.c index f0ba81b2e..8bdcd9da0 100644 --- a/gmodule/gmodule.c +++ b/gmodule/gmodule.c @@ -481,7 +481,7 @@ g_module_open_full (const gchar *file_name, _g_module_debug_init (); if (module_debug_flags & G_MODULE_DEBUG_BIND_NOW_MODULES) - flags &= ~G_MODULE_BIND_LAZY; + flags &= (unsigned) ~G_MODULE_BIND_LAZY; if (!file_name) { diff --git a/gmodule/meson.build b/gmodule/meson.build index f91924635..37a8fa42d 100644 --- a/gmodule/meson.build +++ b/gmodule/meson.build @@ -96,7 +96,7 @@ libgmodule = library('gmodule-2.0', install : true, include_directories : [configinc, gmoduleinc], dependencies : [libdl_dep, libglib_dep], - c_args : ['-DG_LOG_DOMAIN="GModule"', '-DGMODULE_COMPILATION'], + c_args : ['-DG_LOG_DOMAIN="GModule"', '-DGMODULE_COMPILATION', warning_sign_conversion_args], gnu_symbol_visibility : 'hidden', link_args : [glib_link_flags], ) diff --git a/gmodule/tests/meson.build b/gmodule/tests/meson.build index 5374c1c2a..e7d7b3dd1 100644 --- a/gmodule/tests/meson.build +++ b/gmodule/tests/meson.build @@ -75,7 +75,7 @@ test_env.set('G_TEST_SRCDIR', meson.current_source_dir()) test_env.set('G_TEST_BUILDDIR', meson.current_build_dir()) test_deps = [libm, thread_dep, libglib_dep, libgmodule_dep] -test_cargs = ['-DG_LOG_DOMAIN="GModule"', '-UG_DISABLE_ASSERT'] +test_cargs = ['-DG_LOG_DOMAIN="GModule"', '-UG_DISABLE_ASSERT', warning_sign_conversion_args] test_cpp_args = test_cargs foreach test_name, extra_args : gmodule_tests From 4b7f0a8f50d52b695bba1c3f53fe8f54f8e5e7ff Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 21:58:33 +0100 Subject: [PATCH 02/12] gthread: Enable -Wsign-conversion for gthread subdirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with previous commits, we’re enabling `-Wsign-conversion` piecemeal for all of glib.git. It turns out that gthread didn’t have any `-Wsign-conversion` warnings at all, so this was straightforward. Signed-off-by: Philip Withnall Helps: #3405 --- gthread/meson.build | 2 +- gthread/tests/meson.build | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gthread/meson.build b/gthread/meson.build index abb6f9151..9b492eed6 100644 --- a/gthread/meson.build +++ b/gthread/meson.build @@ -19,7 +19,7 @@ libgthread = library('gthread-2.0', darwin_versions : darwin_versions, install : true, dependencies : [libglib_dep], - c_args : ['-DG_LOG_DOMAIN="GThread"', glib_c_args_internal], + c_args : ['-DG_LOG_DOMAIN="GThread"', glib_c_args_internal, warning_sign_conversion_args], gnu_symbol_visibility : 'hidden', link_args : glib_link_flags, ) diff --git a/gthread/tests/meson.build b/gthread/tests/meson.build index 41fad1de4..66b50caf7 100644 --- a/gthread/tests/meson.build +++ b/gthread/tests/meson.build @@ -7,7 +7,7 @@ test_env.set('G_TEST_SRCDIR', meson.current_source_dir()) test_env.set('G_TEST_BUILDDIR', meson.current_build_dir()) test_deps = [thread_dep, libglib_dep, libgthread_dep] -test_cargs = ['-DG_LOG_DOMAIN="GLib-GThread"', '-UG_DISABLE_ASSERT'] +test_cargs = ['-DG_LOG_DOMAIN="GLib-GThread"', '-UG_DISABLE_ASSERT', warning_sign_conversion_args] test_cpp_args = test_cargs foreach test_name, extra_args : gthread_tests From f44511bc54326f8b4a14619c33d7795e6dad33bb Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 22:04:09 +0100 Subject: [PATCH 03/12] girepository: Fix straightforward -Wsign-conversion warnings Signed-off-by: Philip Withnall Helps: #3405 --- girepository/giarginfo.c | 4 ++-- girepository/gicallableinfo.c | 8 ++++---- girepository/giinterfaceinfo.c | 14 +++++++------- girepository/giobjectinfo.c | 22 ++++++++++------------ girepository/girepository.c | 6 +++--- girepository/girmodule.c | 2 +- girepository/giroffsets.c | 2 +- girepository/gitypelib.c | 11 ++++++----- girepository/giunioninfo.c | 2 +- girepository/givfuncinfo.c | 3 ++- 10 files changed, 37 insertions(+), 37 deletions(-) diff --git a/girepository/giarginfo.c b/girepository/giarginfo.c index 8728fd0d6..5d048377e 100644 --- a/girepository/giarginfo.c +++ b/girepository/giarginfo.c @@ -286,7 +286,7 @@ gi_arg_info_get_closure_index (GIArgInfo *info, has_closure_index = (blob->closure >= 0); if (out_closure_index != NULL) - *out_closure_index = has_closure_index ? blob->closure : 0; + *out_closure_index = has_closure_index ? (unsigned int) blob->closure : 0; return has_closure_index; } @@ -317,7 +317,7 @@ gi_arg_info_get_destroy_index (GIArgInfo *info, has_destroy_index = (blob->destroy >= 0); if (out_destroy_index != NULL) - *out_destroy_index = has_destroy_index ? blob->destroy : 0; + *out_destroy_index = has_destroy_index ? (unsigned int) blob->destroy : 0; return has_destroy_index; } diff --git a/girepository/gicallableinfo.c b/girepository/gicallableinfo.c index 551fe961d..48b1fc679 100644 --- a/girepository/gicallableinfo.c +++ b/girepository/gicallableinfo.c @@ -80,7 +80,7 @@ signature_offset (GICallableInfo *info) g_assert_not_reached (); } if (sigoff >= 0) - return *(uint32_t *)&rinfo->typelib->data[rinfo->offset + sigoff]; + return *(uint32_t *)&rinfo->typelib->data[rinfo->offset + (unsigned) sigoff]; return 0; } @@ -354,8 +354,8 @@ gi_callable_info_get_n_args (GICallableInfo *info) uint32_t offset; SignatureBlob *blob; - g_return_val_if_fail (info != NULL, -1); - g_return_val_if_fail (GI_IS_CALLABLE_INFO (info), -1); + g_return_val_if_fail (info != NULL, 0); + g_return_val_if_fail (GI_IS_CALLABLE_INFO (info), 0); offset = signature_offset (info); blob = (SignatureBlob *)&rinfo->typelib->data[offset]; @@ -710,7 +710,7 @@ gi_callable_info_invoke (GICallableInfo *info, } for (i = 0; i < n_args; i++) { - int offset = (is_method ? 1 : 0); + size_t offset = (is_method ? 1 : 0); ainfo = gi_callable_info_get_arg ((GICallableInfo *)info, i); switch (gi_arg_info_get_direction (ainfo)) { diff --git a/girepository/giinterfaceinfo.c b/girepository/giinterfaceinfo.c index 1c5289ea5..3901d2a25 100644 --- a/girepository/giinterfaceinfo.c +++ b/girepository/giinterfaceinfo.c @@ -149,7 +149,7 @@ gi_interface_info_get_property (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + n * header->property_blob_size; return (GIPropertyInfo *) gi_base_info_new (GI_INFO_TYPE_PROPERTY, (GIBaseInfo*)info, @@ -207,7 +207,7 @@ gi_interface_info_get_method (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + blob->n_properties * header->property_blob_size + n * header->function_blob_size; @@ -239,7 +239,7 @@ gi_interface_info_find_method (GIInterfaceInfo *info, InterfaceBlob *blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + blob->n_properties * header->property_blob_size; return gi_base_info_find_method ((GIBaseInfo*)info, offset, blob->n_methods, name); @@ -296,7 +296,7 @@ gi_interface_info_get_signal (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + blob->n_properties * header->property_blob_size + blob->n_methods * header->function_blob_size + n * header->signal_blob_size; @@ -392,7 +392,7 @@ gi_interface_info_get_vfunc (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + blob->n_properties * header->property_blob_size + blob->n_methods * header->function_blob_size + blob->n_signals * header->signal_blob_size @@ -433,7 +433,7 @@ gi_interface_info_find_vfunc (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + blob->n_prerequisites % 2) * 2 + + (blob->n_prerequisites + blob->n_prerequisites % 2u) * 2u + blob->n_properties * header->property_blob_size + blob->n_methods * header->function_blob_size + blob->n_signals * header->signal_blob_size; @@ -492,7 +492,7 @@ gi_interface_info_get_constant (GIInterfaceInfo *info, blob = (InterfaceBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->interface_blob_size - + (blob->n_prerequisites + (blob->n_prerequisites % 2)) * 2 + + (blob->n_prerequisites + (blob->n_prerequisites % 2u)) * 2u + blob->n_properties * header->property_blob_size + blob->n_methods * header->function_blob_size + blob->n_signals * header->signal_blob_size diff --git a/girepository/giobjectinfo.c b/girepository/giobjectinfo.c index 68cee822f..9dbcb5690 100644 --- a/girepository/giobjectinfo.c +++ b/girepository/giobjectinfo.c @@ -70,7 +70,7 @@ gi_object_info_get_field_offset (GIObjectInfo *info, FieldBlob *field_blob; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2; + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u; for (size_t i = 0; i < n; i++) { @@ -383,7 +383,7 @@ gi_object_info_get_property (GIObjectInfo *info, blob = (ObjectBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + n * header->property_blob_size; @@ -444,7 +444,7 @@ gi_object_info_get_method (GIObjectInfo *info, offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size @@ -484,7 +484,7 @@ gi_object_info_find_method (GIObjectInfo *info, blob = (ObjectBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size; @@ -528,8 +528,7 @@ gi_object_info_find_method_using_interfaces (GIObjectInfo *info, if (result == NULL) { - int n_interfaces; - int i; + unsigned int n_interfaces, i; n_interfaces = gi_object_info_get_n_interfaces (info); for (i = 0; i < n_interfaces; ++i) @@ -588,7 +587,7 @@ object_get_signal_offset (GIObjectInfo *info, unsigned int n) ObjectBlob *blob = (ObjectBlob *) &rinfo->typelib->data[rinfo->offset]; return rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size @@ -723,7 +722,7 @@ gi_object_info_get_vfunc (GIObjectInfo *info, blob = (ObjectBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size @@ -771,7 +770,7 @@ gi_object_info_find_vfunc (GIObjectInfo *info, blob = (ObjectBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size @@ -822,8 +821,7 @@ gi_object_info_find_vfunc_using_interfaces (GIObjectInfo *info, if (result == NULL) { - int n_interfaces; - int i; + unsigned int n_interfaces, i; n_interfaces = gi_object_info_get_n_interfaces (info); for (i = 0; i < n_interfaces; ++i) @@ -902,7 +900,7 @@ gi_object_info_get_constant (GIObjectInfo *info, blob = (ObjectBlob *)&rinfo->typelib->data[rinfo->offset]; offset = rinfo->offset + header->object_blob_size - + (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * header->field_blob_size + blob->n_field_callbacks * header->callback_blob_size + blob->n_properties * header->property_blob_size diff --git a/girepository/girepository.c b/girepository/girepository.c index a6a81716d..ad1cec18d 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -976,12 +976,12 @@ gi_repository_get_n_infos (GIRepository *repository, GITypelib *typelib; unsigned int n_interfaces = 0; - g_return_val_if_fail (GI_IS_REPOSITORY (repository), -1); - g_return_val_if_fail (namespace != NULL, -1); + g_return_val_if_fail (GI_IS_REPOSITORY (repository), 0); + g_return_val_if_fail (namespace != NULL, 0); typelib = get_registered (repository, namespace, NULL); - g_return_val_if_fail (typelib != NULL, -1); + g_return_val_if_fail (typelib != NULL, 0); n_interfaces = ((Header *)typelib->data)->n_local_entries; diff --git a/girepository/girmodule.c b/girepository/girmodule.c index 87bb13ad3..96a878388 100644 --- a/girepository/girmodule.c +++ b/girepository/girmodule.c @@ -250,7 +250,7 @@ node_cmp_offset_func (const void *a, { const GIIrNode *na = a; const GIIrNode *nb = b; - return na->offset - nb->offset; + return (int) na->offset - (int) nb->offset; } static void diff --git a/girepository/giroffsets.c b/girepository/giroffsets.c index d3fe36a1d..60e6e341f 100644 --- a/girepository/giroffsets.c +++ b/girepository/giroffsets.c @@ -356,7 +356,7 @@ get_field_size_alignment (GIIrTypelibBuild *build, return success; } -#define GI_ALIGN(n, align) (((n) + (align) - 1) & ~((align) - 1)) +#define GI_ALIGN(n, align) (((n) + (align) - 1u) & ~((align) - 1u)) static gboolean compute_struct_field_offsets (GIIrTypelibBuild *build, diff --git a/girepository/gitypelib.c b/girepository/gitypelib.c index 57411a86b..68247dd36 100644 --- a/girepository/gitypelib.c +++ b/girepository/gitypelib.c @@ -87,7 +87,7 @@ get_dir_entry_checked (GITypelib *typelib, return FALSE; } - offset = header->directory + (index - 1) * header->entry_blob_size; + offset = header->directory + (index - 1u) * header->entry_blob_size; if (typelib->len < offset + sizeof (DirEntry)) { @@ -161,7 +161,8 @@ gi_typelib_get_dir_entry (GITypelib *typelib, { Header *header = (Header *)typelib->data; - return (DirEntry *)&typelib->data[header->directory + (index - 1) * header->entry_blob_size]; + /* this deliberately doesn’t check for underflow of @index; see get_dir_entry_checked() for that */ + return (DirEntry *)&typelib->data[header->directory + (index - 1u) * header->entry_blob_size]; } static Section * @@ -303,7 +304,7 @@ strsplit_iter_next (StrSplitIter *iter, if (next) { iter->s = next + iter->sep_len; - len = next - s; + len = (size_t) (next - s); } else { @@ -1718,7 +1719,7 @@ validate_object_blob (ValidateContext *ctx, } if (typelib->len < offset + sizeof (ObjectBlob) + - (blob->n_interfaces + blob->n_interfaces % 2) * 2 + + (blob->n_interfaces + blob->n_interfaces % 2u) * 2u + blob->n_fields * sizeof (FieldBlob) + blob->n_properties * sizeof (PropertyBlob) + blob->n_methods * sizeof (FunctionBlob) + @@ -1875,7 +1876,7 @@ validate_interface_blob (ValidateContext *ctx, return FALSE; if (typelib->len < offset + sizeof (InterfaceBlob) + - (blob->n_prerequisites + blob->n_prerequisites % 2) * 2 + + (blob->n_prerequisites + blob->n_prerequisites % 2u) * 2u + blob->n_properties * sizeof (PropertyBlob) + blob->n_methods * sizeof (FunctionBlob) + blob->n_signals * sizeof (SignalBlob) + diff --git a/girepository/giunioninfo.c b/girepository/giunioninfo.c index de9a34bed..8a1e28723 100644 --- a/girepository/giunioninfo.c +++ b/girepository/giunioninfo.c @@ -169,7 +169,7 @@ gi_union_info_get_discriminator_offset (GIUnionInfo *info, UnionBlob *blob = (UnionBlob *)&rinfo->typelib->data[rinfo->offset]; size_t discriminator_offset; - discriminator_offset = (blob->discriminated) ? blob->discriminator_offset : 0; + discriminator_offset = (blob->discriminated) ? (size_t) blob->discriminator_offset : 0; if (out_offset != NULL) *out_offset = discriminator_offset; diff --git a/girepository/givfuncinfo.c b/girepository/givfuncinfo.c index 728cca43c..4cd328bba 100644 --- a/girepository/givfuncinfo.c +++ b/girepository/givfuncinfo.c @@ -224,7 +224,8 @@ gi_vfunc_info_get_address (GIVFuncInfo *vfunc_info, GIObjectInfo *object_info; GIStructInfo *struct_info; GIFieldInfo *field_info = NULL; - int length, i, offset; + size_t offset; + unsigned int length, i; void *implementor_class, *implementor_vtable; void *func = NULL; From 8ab78a66a4d238e81b68c92814f699ff77f18f3e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 22:05:42 +0100 Subject: [PATCH 04/12] girepository: Fix -Wsign-conversion warnings with string arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are a few `g_strndup()` calls which use a length calculated from the return value of `strchr()` minus the original string. That’s fine, as long as `strchr()` doesn’t return `NULL`. Add some asserts to ensure that. Signed-off-by: Philip Withnall Helps: #3405 --- girepository/girepository.c | 13 ++++++++++--- girepository/girparser.c | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/girepository/girepository.c b/girepository/girepository.c index ad1cec18d..113e4edb4 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -607,7 +607,8 @@ load_dependencies_recurse (GIRepository *repository, const char *dependency_version; last_dash = strrchr (dependency, '-'); - dependency_namespace = g_strndup (dependency, last_dash - dependency); + g_assert (last_dash != NULL); /* get_typelib_dependencies() guarantees this */ + dependency_namespace = g_strndup (dependency, (size_t) (last_dash - dependency)); dependency_version = last_dash+1; if (!gi_repository_require (repository, dependency_namespace, dependency_version, @@ -785,7 +786,8 @@ get_typelib_dependencies_transitive (GIRepository *repository, /* Recurse for this namespace. */ last_dash = strrchr (dependency, '-'); - dependency_namespace = g_strndup (dependency, last_dash - dependency); + g_assert (last_dash != NULL); /* get_typelib_dependencies() guarantees this */ + dependency_namespace = g_strndup (dependency, (size_t) (last_dash - dependency)); typelib = get_registered (repository, dependency_namespace, NULL); g_return_if_fail (typelib != NULL); @@ -1742,7 +1744,12 @@ enumerate_namespace_versions (const char *namespace, name_end = strrchr (entry, '.'); last_dash = strrchr (entry, '-'); - version = g_strndup (last_dash+1, name_end-(last_dash+1)); + + /* These are guaranteed by the suffix and prefix checks above: */ + g_assert (name_end != NULL); + g_assert (last_dash != NULL); + + version = g_strndup (last_dash + 1, (size_t) (name_end - (last_dash + 1u))); if (!parse_version (version, &major, &minor)) { g_free (version); diff --git a/girepository/girparser.c b/girepository/girparser.c index ab8d46bcc..f164a01c3 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -707,7 +707,7 @@ parse_type_internal (GIIrModule *module, *str == ':') (str)++; - type->giinterface = g_strndup (start, str - start); + type->giinterface = g_strndup (start, (size_t) (str - start)); } if (next) From e6d34fa5a82b9c45432401f57185981c80b85505 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 22:09:08 +0100 Subject: [PATCH 05/12] girparser: Fix a potential buffer overflow with g_strndup() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If parsing a generic type which has no closing `>`, there was no check that the `strchr()` call succeeded, which could have resulted in a negative length being passed to `g_strndup()`, which would result in a long positive length after implicit type casting. Fix that by bringing an old error handling path back into use. This results in a `g_critical()` in the calling function, which is good enough for now. Potentially all this code could be reworked to use `GError`, but that’s a much bigger project (a lot more of the `girparser.c` code would need to change). Signed-off-by: Philip Withnall Helps: #3405 --- girepository/girparser.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/girepository/girparser.c b/girepository/girparser.c index f164a01c3..cb1ebf8a4 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -685,7 +685,9 @@ parse_type_internal (GIIrModule *module, (str)++; end = strchr (str, '>'); - tmp = g_strndup (str, end - str); + if (end == NULL) + goto error; + tmp = g_strndup (str, (size_t) (end - str)); type->errors = g_strsplit (tmp, ",", 0); g_free (tmp); @@ -716,7 +718,7 @@ parse_type_internal (GIIrModule *module, g_free (temporary_type); return type; -/* error: */ +error: gi_ir_node_free ((GIIrNode *)type); g_free (temporary_type); return NULL; From cff7783a323d573854df86182905697c335e0ce7 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 23:09:09 +0100 Subject: [PATCH 06/12] girparser: Fix error handling for type parsing Reworking the code to add proper `GError` handling for type parsing, rather than the existing `g_critical()`, turned out to actually be fairly straightforward. So now `gi_ir_parser_parse_string()` returns `G_MARKUP_ERROR_INVALID_CONTENT` on unparseable types, just like it does with various other bits of invalid GIR. Signed-off-by: Philip Withnall --- girepository/girparser.c | 62 ++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/girepository/girparser.c b/girepository/girparser.c index cb1ebf8a4..3b80f1e86 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -454,11 +454,12 @@ push_node (ParseContext *ctx, GIIrNode *node) ctx->node_stack = g_slist_prepend (ctx->node_stack, node); } -static GIIrNodeType * parse_type_internal (GIIrModule *module, - const char *str, - char **next, - gboolean in_glib, - gboolean in_gobject); +static GIIrNodeType * parse_type_internal (GIIrModule *module, + const char *str, + char **next, + gboolean in_glib, + gboolean in_gobject, + GError **error); typedef struct { const char *str; @@ -575,11 +576,12 @@ parse_basic (const char *str) } static GIIrNodeType * -parse_type_internal (GIIrModule *module, - const char *str, - char **next, - gboolean in_glib, - gboolean in_gobject) +parse_type_internal (GIIrModule *module, + const char *str, + char **next, + gboolean in_glib, + gboolean in_gobject, + GError **error) { const BasicTypeInfo *basic; GIIrNodeType *type; @@ -686,7 +688,12 @@ parse_type_internal (GIIrModule *module, end = strchr (str, '>'); if (end == NULL) - goto error; + { + g_set_error (error, G_MARKUP_ERROR, + G_MARKUP_ERROR_INVALID_CONTENT, + "Failed to parse type ‘%s’", type->unparsed); + goto error; + } tmp = g_strndup (str, (size_t) (end - str)); type->errors = g_strsplit (tmp, ",", 0); g_free (tmp); @@ -719,6 +726,7 @@ parse_type_internal (GIIrModule *module, return type; error: + g_assert (error == NULL || *error != NULL); gi_ir_node_free ((GIIrNode *)type); g_free (temporary_type); return NULL; @@ -793,7 +801,9 @@ is_pointer_or_disguised_structure (ParseContext *ctx, } static GIIrNodeType * -parse_type (ParseContext *ctx, const char *type) +parse_type (ParseContext *ctx, + const char *type, + GError **error) { GIIrNodeType *node; const BasicTypeInfo *basic; @@ -807,11 +817,11 @@ parse_type (ParseContext *ctx, const char *type) if (basic == NULL) type = resolve_aliases (ctx, type); - node = parse_type_internal (ctx->current_module, type, NULL, in_glib, in_gobject); + node = parse_type_internal (ctx->current_module, type, NULL, in_glib, in_gobject, error); if (node) g_debug ("Parsed type: %s => %d", type, node->tag); else - g_critical ("Failed to parse type: '%s'", type); + g_debug ("Failed to parse type: '%s'", type); return node; } @@ -1519,7 +1529,8 @@ start_field (GMarkupParseContext *context, } else { - field->type = parse_type (ctx, "gpointer"); + field->type = parse_type (ctx, "gpointer", NULL); + g_assert (field->type != NULL); /* parsing `gpointer` should never fail */ } ((GIIrNode *)field)->name = g_strdup (name); @@ -2278,7 +2289,9 @@ start_type (GMarkupParseContext *context, pointer_depth > 0) pointer_depth--; - typenode = parse_type (ctx, name); + typenode = parse_type (ctx, name, error); + if (typenode == NULL) + return FALSE; /* A "pointer" structure is one where the c:type is a typedef that * to a pointer to a structure; we used to call them "disguised" @@ -2322,14 +2335,17 @@ end_type_top (ParseContext *ctx) typenode->tag == GI_TYPE_TAG_GSLIST) { if (typenode->parameter_type1 == NULL) - typenode->parameter_type1 = parse_type (ctx, "gpointer"); + typenode->parameter_type1 = parse_type (ctx, "gpointer", NULL); + g_assert (typenode->parameter_type1 != NULL); /* parsing `gpointer` should never fail */ } else if (typenode->tag == GI_TYPE_TAG_GHASH) { if (typenode->parameter_type1 == NULL) { - typenode->parameter_type1 = parse_type (ctx, "gpointer"); - typenode->parameter_type2 = parse_type (ctx, "gpointer"); + typenode->parameter_type1 = parse_type (ctx, "gpointer", NULL); + g_assert (typenode->parameter_type1 != NULL); /* parsing `gpointer` should never fail */ + typenode->parameter_type2 = parse_type (ctx, "gpointer", NULL); + g_assert (typenode->parameter_type2 != NULL); /* same */ } } @@ -2979,6 +2995,7 @@ start_discriminator (GMarkupParseContext *context, const char *type; const char *offset; guint64 parsed_offset; + GIIrNodeType *discriminator_type = NULL; if (!(strcmp (element_name, "discriminator") == 0 && ctx->state == STATE_UNION)) @@ -2997,8 +3014,11 @@ start_discriminator (GMarkupParseContext *context, return FALSE; } - ((GIIrNodeUnion *)CURRENT_NODE (ctx))->discriminator_type - = parse_type (ctx, type); + discriminator_type = parse_type (ctx, type, error); + if (discriminator_type == NULL) + return FALSE; + + ((GIIrNodeUnion *)CURRENT_NODE (ctx))->discriminator_type = g_steal_pointer (&discriminator_type); if (g_ascii_string_to_unsigned (offset, 10, 0, G_MAXSIZE, &parsed_offset, error)) ((GIIrNodeUnion *)CURRENT_NODE (ctx))->discriminator_offset = parsed_offset; From debb6a9cc03dcccca6fdb05fdf32f5ba750f93b9 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 23:10:36 +0100 Subject: [PATCH 07/12] tests: Add a basic test for GIIrParser type parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This follows up from the previous two commits to add a unit test. It doesn’t attempt to cover the multitude of other possible type parsing conditions; at the moment it’s just a regression test for the previous two commits, and somewhere to hang new tests on in future. Signed-off-by: Philip Withnall --- girepository/tests/ir-parser.c | 106 +++++++++++++++++++++++++++++++++ girepository/tests/meson.build | 3 + 2 files changed, 109 insertions(+) create mode 100644 girepository/tests/ir-parser.c diff --git a/girepository/tests/ir-parser.c b/girepository/tests/ir-parser.c new file mode 100644 index 000000000..79b71ab88 --- /dev/null +++ b/girepository/tests/ir-parser.c @@ -0,0 +1,106 @@ +/* + * Copyright 2025 GNOME Foundation, Inc. + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, see . + * + * Authors: + * - Philip Withnall + */ + +#include "config.h" + +#include "girepository.h" +#include "girparser-private.h" + +static void +test_type_parsing (void) +{ + const char *buffer_template = "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + ""; + const struct + { + const char *type; + gboolean expected_success; + } + vectors[] = + { + { "GLib.Error", TRUE }, + { "GLib.Error", TRUE }, + { "GLib.Error Date: Fri, 11 Apr 2025 23:11:52 +0100 Subject: [PATCH 08/12] girparser: Stop setting blob length to -1 when no length is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When parsing a GIR or building a typelib, stop setting the array length field to `-1` as a default. That field is unsigned, so setting it to `-1` is actually equivalent to setting it to `G_MAXUINT`. I can’t find anywhere which treats `G_MAXUINT` or `-1` as a magic value there, so it’s probably better off left unset. Given the lack of documentation for the typelib code, though, there is a fair chance I’m making a mistake and this is actually an integral part of the format. Let’s see what breaks. This fixes a `-Wsign-conversion` warning, at least. Signed-off-by: Philip Withnall Helps: #3405 --- girepository/girnode.c | 2 -- girepository/girparser.c | 16 ++++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/girepository/girnode.c b/girepository/girnode.c index 65641625d..23a880fa6 100644 --- a/girepository/girnode.c +++ b/girepository/girnode.c @@ -1523,8 +1523,6 @@ gi_ir_node_build_typelib (GIIrNode *node, array->dimensions.length = type->length; else if (array->has_size) array->dimensions.size = type->size; - else - array->dimensions.length = -1; pos = *offset2 + G_STRUCT_OFFSET (ArrayTypeBlob, type); *offset2 += sizeof (ArrayTypeBlob); diff --git a/girepository/girparser.c b/girepository/girparser.c index 3b80f1e86..ea00d4500 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -2224,22 +2224,20 @@ start_type (GMarkupParseContext *context, size = find_attribute ("fixed-size", attribute_names, attribute_values); typenode->has_length = len != NULL; - if (!typenode->has_length) - typenode->length = -1; - else if (g_ascii_string_to_unsigned (len, 10, 0, G_MAXUINT, &parsed_uint, error)) + if (typenode->has_length && + g_ascii_string_to_unsigned (len, 10, 0, G_MAXUINT, &parsed_uint, error)) typenode->length = parsed_uint; - else + else if (typenode->has_length) { gi_ir_node_free ((GIIrNode *) typenode); return FALSE; } typenode->has_size = size != NULL; - if (!typenode->has_size) - typenode->size = -1; - else if (g_ascii_string_to_unsigned (size, 10, 0, G_MAXSIZE, &parsed_uint, error)) + if (typenode->has_size && + g_ascii_string_to_unsigned (size, 10, 0, G_MAXSIZE, &parsed_uint, error)) typenode->size = parsed_uint; - else + else if (typenode->has_size) { gi_ir_node_free ((GIIrNode *) typenode); return FALSE; @@ -2256,9 +2254,7 @@ start_type (GMarkupParseContext *context, } else { typenode->zero_terminated = FALSE; typenode->has_length = FALSE; - typenode->length = -1; typenode->has_size = FALSE; - typenode->size = -1; } } else From 7c06269eef3c346d2cec652c2bf09ba7d014e3b2 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 23:14:52 +0100 Subject: [PATCH 09/12] girparser: Limit the size of GIR files which can be parsed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to passing around file lengths variously as `gsize` or `gssize`, we can’t reliably handle files with length greater than `G_MAXSSIZE`, as some of the APIs in use need `-1` to indicate that their input is nul terminated. Add some checks for this, and gracefully return an error if an input file is too big, rather than just exploding. Signed-off-by: Philip Withnall Helps: #3405 --- girepository/girparser.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/girepository/girparser.c b/girepository/girparser.c index ea00d4500..52e6a92fa 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -3080,7 +3080,14 @@ parse_include (GMarkupParseContext *context, return FALSE; } - module = gi_ir_parser_parse_string (ctx->parser, name, girpath, buffer, length, &error); + if (length > G_MAXSSIZE) + { + g_printerr ("Input file ‘%s’ too big\n", girpath); + g_free (girpath); + return FALSE; + } + + module = gi_ir_parser_parse_string (ctx->parser, name, girpath, buffer, (gssize) length, &error); g_free (buffer); if (error != NULL) { @@ -3922,7 +3929,7 @@ cleanup (GMarkupParseContext *context, * @namespace: the namespace of the string * @filename: (nullable) (type filename): Path to parsed file, or `NULL` * @buffer: (array length=length): the data containing the XML - * @length: length of the data, in bytes + * @length: length of the data, in bytes, or `-1` if nul terminated * @error: return location for a [type@GLib.Error], or `NULL` * * Parse a string that holds a complete GIR XML file, and return a list of a @@ -4061,7 +4068,19 @@ gi_ir_parser_parse_file (GIIrParser *parser, return NULL; } - module = gi_ir_parser_parse_string (parser, namespace, filename, buffer, length, error); + if (length > G_MAXSSIZE) + { + g_free (namespace); + g_free (buffer); + + g_set_error (error, + G_MARKUP_ERROR, + G_MARKUP_ERROR_INVALID_CONTENT, + "Input file too big"); + return NULL; + } + + module = gi_ir_parser_parse_string (parser, namespace, filename, buffer, (gssize) length, error); g_free (namespace); From 3de99090d099caa10aa879839441d10d780ddc48 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Apr 2025 23:16:28 +0100 Subject: [PATCH 10/12] girepository: Enable -Wsign-conversion for girepository subdirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As with previous commits, we’re enabling `-Wsign-conversion` piecemeal for all of glib.git. The previous few commits have fixed all the `-Wsign-conversion` warnings in libgirepository, so let’s enable the warning by default for that directory to prevent regressions. Signed-off-by: Philip Withnall Helps: #3405 --- girepository/compiler/meson.build | 1 + girepository/decompiler/meson.build | 1 + girepository/inspector/meson.build | 1 + girepository/meson.build | 1 + girepository/tests/meson.build | 2 +- 5 files changed, 5 insertions(+), 1 deletion(-) diff --git a/girepository/compiler/meson.build b/girepository/compiler/meson.build index f724f32c5..eb704761e 100644 --- a/girepository/compiler/meson.build +++ b/girepository/compiler/meson.build @@ -3,6 +3,7 @@ custom_c_args = [ '-DG_LOG_DOMAIN="GLib-GirCompiler"', + warning_sign_conversion_args, ] if cc.get_id() != 'msvc' diff --git a/girepository/decompiler/meson.build b/girepository/decompiler/meson.build index cb4553262..b6090f61f 100644 --- a/girepository/decompiler/meson.build +++ b/girepository/decompiler/meson.build @@ -3,6 +3,7 @@ custom_c_args = [ '-DG_LOG_DOMAIN="GLib-GirDecompiler"', + warning_sign_conversion_args, ] if cc.get_id() != 'msvc' diff --git a/girepository/inspector/meson.build b/girepository/inspector/meson.build index 8948cedc8..4467e6df8 100644 --- a/girepository/inspector/meson.build +++ b/girepository/inspector/meson.build @@ -3,6 +3,7 @@ custom_c_args = [ '-DG_LOG_DOMAIN="GLib-GirInspector"', + warning_sign_conversion_args, ] if cc.get_id() != 'msvc' diff --git a/girepository/meson.build b/girepository/meson.build index 3010099aa..b08801891 100644 --- a/girepository/meson.build +++ b/girepository/meson.build @@ -89,6 +89,7 @@ gir_c_args = [ '-DGOBJECT_INTROSPECTION_LIBDIR="@0@"'.format(glib_libdir), '-DGOBJECT_INTROSPECTION_DATADIR="@0@"'.format(glib_datadir), '-DGOBJECT_INTROSPECTION_RELATIVE_LIBDIR="@0@"'.format(get_option('libdir')), + warning_sign_conversion_args, ] custom_c_args = [] diff --git a/girepository/tests/meson.build b/girepository/tests/meson.build index 84a0bac9c..0697c9fa2 100644 --- a/girepository/tests/meson.build +++ b/girepository/tests/meson.build @@ -94,7 +94,7 @@ test_env.set('G_TEST_SRCDIR', meson.current_source_dir()) test_env.set('G_TEST_BUILDDIR', meson.current_build_dir()) test_deps = [libm, thread_dep, libgirepository_dep] -test_cargs = ['-DG_LOG_DOMAIN="GIRepository"', '-UG_DISABLE_ASSERT'] +test_cargs = ['-DG_LOG_DOMAIN="GIRepository"', '-UG_DISABLE_ASSERT', warning_sign_conversion_args] test_cpp_args = test_cargs foreach test_name, extra_args : girepository_tests From 21d8e246e7b27d2c76d0601899acfe8b66a3a0ad Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Sat, 12 Apr 2025 16:24:32 +0100 Subject: [PATCH 11/12] girepository: More minor -Wsign-conversion fixes from CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These didn’t occur on my local machine, interestingly. Signed-off-by: Philip Withnall Helps: #3405 --- girepository/girnode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/girepository/girnode.c b/girepository/girnode.c index 23a880fa6..eaced1561 100644 --- a/girepository/girnode.c +++ b/girepository/girnode.c @@ -2195,7 +2195,8 @@ gi_ir_node_build_typelib (GIIrNode *node, blob->n_fields = 0; blob->n_functions = 0; - blob->discriminator_offset = union_->discriminator_offset; + g_assert (union_->discriminator_offset <= G_MAXINT32); + blob->discriminator_offset = (int32_t) union_->discriminator_offset; if (union_->copy_func) blob->copy_func = gi_ir_write_string (union_->copy_func, strings, data, offset2); From ecc1f86e92734482a1487399347686e762ca1d66 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Sat, 12 Apr 2025 16:25:03 +0100 Subject: [PATCH 12/12] gmodule: Fix a minor -Wsign-conversion error on Windows Spotted by CI: https://gitlab.gnome.org/pwithnall/glib/-/jobs/4968919 Signed-off-by: Philip Withnall Helps: #3405 --- gmodule/gmodule-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gmodule/gmodule-win32.c b/gmodule/gmodule-win32.c index 5748bef0f..3869d3dae 100644 --- a/gmodule/gmodule-win32.c +++ b/gmodule/gmodule-win32.c @@ -52,7 +52,7 @@ set_error (GError **error, gchar *message; va_list args; - win32_error = g_win32_error_message (GetLastError ()); + win32_error = g_win32_error_message ((gint) GetLastError ()); va_start (args, format); detail = g_strdup_vprintf (format, args);