Testing this in a normal testcaes is a bit tricky, since
triggering a non-fatal assertion has the side-effect of
marking the test as failed.
So just don't run any testcases here, but check the side-effect
manually. Since we don't produce TAP output when not using
g_test_run(), tell meson that we're using the exitcode protocol.
There is a race between releasing and re-acquiring an interned
GRefString if this happens on two threads at the same time. This can
result in already freed memory to be returned from
g_ref_string_new_intern().
| Thread 1 | Thread 2 |
| ------------------------------ | ----------------------------- |
| g_ref_string_release() | g_ref_string_new_intern() |
| g_atomic_rc_box_release_full() | g_mutex_lock() |
| | g_hash_table_lookup() |
| remove_if_interned() | g_ref_string_acquire() |
| g_mutex_lock() | g_mutex_unlock() |
| g_hash_table_remove() | |
| g_mutex_unlock() | |
| g_free() | |
| | return res; // this is freed |
This use-after-free usually also gives a critical warning because
g_atomic_ref_count_inc() checks for the refcount having been 0
before incrementing.
It is not possible to safely implement weak references via garcbox.
To avoid this race do not implement weak references via garcbox but
instead implement the allocation of the string manually with a manually
managed reference count. This allows to safely resurrect the interned
string if the above race happens, and also avoids other races.
As a side-effect this also
* reduces the allocation size in addition to the actual string length
from 32 bytes to 16 bytes on 64 bit platforms and keeps it at 16 bytes
on 32 bit platforms,
* doesn't lock a mutex when freeing non-interned GRefStrings.
We have a mechanism for turning on optional features of the GLib
test harness by passing options to g_test_init(). Use it for the
non-fatal assertions as well.
The documentation for glibc's pthread_setname_np states:
The thread name is a meaningful C language string,
whose length is restricted to 16 characters,
including the terminating null byte ('\0').
The documentation for Solaris' pthread_setname_np states:
The thread name is a string of length 31 bytes or less,
UTF-8 encoded.
Failing to respect this length limitation may lead to no name being
set, which is confusing, since the thread then shows up under the
binary name in gdb. This was happening for the pango worker thread
with the name "[pango] fontconfig".
For g_auto(GVariantBuilder) one needs to initialize it before the
function returns, so it's best to do it when the variable is declared.
G_VARIANT_BUILDER_INIT exists but requires specifying a GVariantType in
the declaration which moves the type away from the usage of the builder
which often results in less readable code. G_VARIANT_BUILDER_INIT also
mentions that it's possible to explicitly zero the variable but this is
hard to find and writing `g_auto(GVariantBuilder) builder = {0,};` is
kind of ugly.
This introduces G_VARIANT_BUILDER_INIT_UNSET which zero initializes the
variable being declared. This gives us documentation and hides the
explicitly zeroing detail:
auto(GVariantBuilder) builder = G_VARIANT_BUILDER_INIT_UNSET ();
Every usage in GLib ensures this but theoretically external code might
pass something else. As this is only meant to be used internally from
GLib, don't support the other case but at least avoid potential out of
bound reads.
The length might be passed explicitly in the field instead, and the
string might not have a NUL-terminator as happens for example when
passed from the Rust bindings.
This might lead to out of bounds reads.
Thanks to Sebastian Wiesner for noticing this.
This fixes commit aac56f1618 — I missed
this while reviewing it, but the unit tests were partially changed to
call the new APIs, without being fully changed. This caused the build to
succeed on Linux, but fail on macOS due to using a deprecated API.
Actually, a better approach for the unit tests would be to consistently
call the *old* APIs, as they all immediately call the new APIs. Then we
get coverage of both old and new for free, at the cost of putting
`G_GNUC_BEGIN_IGNORE_DEPRECATIONS` at the top of the test file.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3492
UnixMountEntry: Deprecate g_unix_mount_* API in favor of g_unix_mount_entry_* API for GUnixMountEntry methods
Closes#3492
See merge request GNOME/glib!4337
This issue arises because the g_unix_mount_* naming convention does not match
the GUnixMountEntry instance type, confusing the introspection generator.
To resolve this, we are deprecating the g_unix_mount_* API functions that take
a GUnixMountEntry parameter and introducing equivalent g_unix_mount_entry_*
functions that correctly associate with the GUnixMountEntry instance. This change
ensures that introspection data correctly treats these as instance methods and
that documentation reflects proper ownership of returned data.
(Some minor tweaks by Philip Withnall.)
Fixes: #3492
It appears not to work, and nobody interested in FreeBSD has picked it
up to investigate yet.
Rather than have a completely broken implementation of
`g_file_query_exists()` on FreeBSD, let’s settle for using the old one.
It’s slightly slower than the new one, but has worked fine for people
for years.
This essentially reverts commit 65ad41d8a4
on FreeBSD. This commit can be reverted when a FreeBSD person
investigates what’s going wrong with the `faccessat()`-based
implementation.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3495
valgrind spotted this one: a read off the end of the `stop_chars` buffer
when `stop_chars_len == -1`, due to the fuzzing test not sticking to the
requirement from `g_data_input_stream_read_upto()` that `stop_chars`
must be nul-terminated if `stop_chars_len < 0`.
This can happen when reading `fuzzing/README.md`, which is done as a
smoketest when the fuzzing tests are run without the fuzzer, as normal
unit tests. In this case, it made smoke.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Just like how commit ad572e7780 added an
ifunc resolver for `g_utf8_validate()`, we also need to add one for
`g_str_is_ascii()`, as it also calls into the c-utf8 SIMD validation
code which causes false-positive buffer read overflow warnings from
valgrind and asan.
I thought about just adding the `strlen()` call into `g_str_is_ascii()`
unconditionally, as a simpler fix, but from a quick
codesearch.debian.net, it appears `g_str_is_ascii()` is used quite
widely, so this would have an unacceptable performance impact.
This should fix the valgrind failures on the `search-utils` test seen
here: https://gitlab.gnome.org/GNOME/glib/-/jobs/4423753.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
As suggested by Michael Catanzaro, this should make the return type of
the resolve function a bit easier for people to parse.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
It looks like these might get more complex in future, as compilers claim
to support the attribute (`__has_attribute(ifunc)` is true) but then
raise errors at compile time if the target architecture doesn’t support
ifuncs.
For example, see #3511.
This doesn’t fix#3511 (I don’t have time to test on musl right now), but
it should make it easier to update the platform preprocessor conditions
in future.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3511
This adds various additional tests to cover branches of `gunidecomp.c`
which are not already covered, bringing our branch coverage of that file
up to 100% (if you ignore `g_utf8_normalize()`, which is tested by
`unicode-normalize.c` and I’m counting it separately).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3470
