We could have unguarded crashes when calling strlen (NULL) or when passing
invalid GString's.
Also ensure that we are not using the macro `val` argument multiple times as
it may lead to an unwanted behavior when passing to it a variable value such
as `str[++i]`, as the value may be called multiple times.
C++ tests and Coverity were both underlining this.
Fixes: #2890
Now that the implementation of GSlice has been dropped, these tests for
the internals of the implementation are unnecessary.
We can keep `glib/tests/slice.c` as it tests the API rather than the
implementation.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1079
Keep the API for ABI compatibility.
See
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2935#note_1650099
for a summary of the reasoning for this change:
- The performance of system-provided allocators has improved since
GSlice was written, and they are now similarly as performant, or more
performant, than GSlice.
- The code is unmaintained and nobody understands it.
- It doesn’t integrate with tooling and system security features which
have been written for the system `malloc()` implementation (such as
sanitisers, valgrind, etc.).
- It’s confusing for developers: should they use `g_slice_new()` or
`g_new()`?
- GSlice is faster than the libc allocator for allocating and
(particularly) freeing linked lists, but since these are a rubbish
data structure, that’s not a great thing to optimise for.
For the cases where application performance is negatively impacted by
the implementation of GSlice being dropped (and we don’t think there’ll
be many), applications can use a drop-in `malloc()` replacement which is
more suited to their particular workload. Choosing an allocator in GLib
to suit all application workloads is not possible.
Including documentation updates and cleanups by Philip Withnall.
Fixes: #1079
Fix the tests, by allocating the structure.
==121338==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffe44018610 at pc 0x00000040ff71 bp 0x7ffe440178f0 sp 0x7ffe440178e8
READ of size 8 at 0x7ffe44018610 thread T0
#0 0x40ff70 in test_launch_uris_with_terminal ../gio/tests/desktop-app-info.c:1393
#1 0x7efd97b831e8 in test_case_run ../glib/gtestutils.c:2947
#2 0x7efd97b831e8 in g_test_run_suite_internal ../glib/gtestutils.c:3037
#3 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#4 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#5 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#6 0x7efd97b84189 in g_test_run_suite ../glib/gtestutils.c:3136
#7 0x7efd97b842c5 in g_test_run ../glib/gtestutils.c:2248
#8 0x4055bc in main ../gio/tests/desktop-app-info.c:1901
#9 0x7efd9564a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
#10 0x7efd9564a5c8 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x275c8)
#11 0x4059f4 in _start (/home/elmarco/src/gnome/glib/build/gio/tests/desktop-app-info+0x4059f4)
Address 0x7ffe44018610 is located in stack of thread T0 at offset 128 in frame
#0 0x404d1f in main ../gio/tests/desktop-app-info.c:1823
This frame has 6 object(s):
[48, 52) 'argc' (line 1821)
[64, 72) 'path' (line 1870)
[96, 104) 'argv' (line 1822)
[128, 144) '<unknown>' <== Memory access at offset 128 is inside this variable
[160, 176) '<unknown>'
[192, 288) 'supported_terminals' (line 1825)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There’s a kernel bug on the CI machines which is causing this test to
fail all the time and it’s getting my goat.
The test can be re-enabled later (by reverting this commit) when the
kernel on the CI VM host is fixed. I don’t know when that’s going to
happen.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2879
Although unlikely, apparently `message` may be `NULL` (the rest of the
code checks for it), so the code to convert newlines to spaces should
probably also check for `NULL`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1504054
Otherwise if, for whatever reason, the `app` loses its D-Bus name,
`g_application_quit()` is called from `name_was_lost()` before it’s
called from `quit_already()`, and then `quit_already()` does an invalid
read on `app`.
If the name was not meant to be lost at this point in the test, the
subsequent `g_assert_false (name_lost)` will catch that, so this change
shouldn’t cause the test to pass unnecessarily.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This should stop the following valgrind error:
```
==14506== 80 bytes in 1 blocks are definitely lost in loss record 1,098 of 1,463
==14506== at 0x484086F: malloc (vg_replace_malloc.c:381)
==14506== by 0x48C5D83: g_malloc (gmem.c:130)
==14506== by 0x48E7911: g_slice_alloc (gslice.c:1074)
==14506== by 0x48E7951: g_slice_alloc0 (gslice.c:1100)
==14506== by 0x493617D: g_system_thread_new (gthread-posix.c:1188)
==14506== by 0x48F9EAA: g_thread_new_internal (gthread.c:935)
==14506== by 0x48F9E29: g_thread_try_new (gthread.c:919)
==14506== by 0x48FA351: g_thread_pool_spawn_thread (gthreadpool.c:312)
==14506== by 0x48F9D2C: g_thread_proxy (gthread.c:831)
==14506== by 0x4EFA2A4: start_thread (in /usr/lib64/libpthread-2.33.so)
==14506== by 0x4D89322: clone (in /usr/lib64/libc-2.33.so)
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Now that we're using TAP parsing, this will show subtest failures in
details but without showing any logging error, that we'd still need to parse
from actual logs.
When we've a failure our TAP reporting just bails out without that is clear
what is the test that has failed.
So in this case, expose a "not ok" message if the test name is known, and
use it to report the error message too if available.
Otherwise just use the same Bail out! strategy.
This is something that a base test should decide, so if a subtest fails it's
up to the caller to decide whether to bail out the whole suite or just the
subtest.
When using verbose logging, all the printing and the glib info and debug
logs may end up being written in standard out, making meson TAP parser to be
confused and to fail with TAP parsing errors.
So, in such case write the subprocess output all at once using the the gtest
printer so that we add proper formatting.
Also do not write any gtest result message for subprocesses
When using TAP output in gtest, all the printed strings should be commented
not to break the spec, so enforce this at GTest level, by ensuring that
all the lines written via g_print() will be in the commented form and will
respect the subtest indentation.
As per this we can remove some custom code in g_test_log() as now there are
very few cases in which we need to use the default print handler which is
now private when testing.
g_set_print_handler() and g_set_printerr_handler() are supposed to return
the old printer handlers, but in case the default is used NULL is returned
instead.
This makes hard to override the default handler to e.g. decorate the output
with some minor content, without having to rewrite the low-level
implementation.
So, make the default printers to be functions and set them as the default
ones. Also this avoids having to check each time what function to use at
print time.
While checking the validity of a `GVariantTypeInfo` is good, this code
path is very hot, and I’ve never seen these assertions catch a bug in
practice.
Lean more towards the performance side of the performance/correctness
tradeoff in this case, by removing the assertions here.
They remain in place in a number of other `GVariantTypeInfo` code paths,
so invalid `GVariantTypeInfo` pointers should hopefully still be caught
quickly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
These functions were previously calling
`g_variant_serialised_n_children()` twice just to validate the input, in
the case that the input was a serialised variant.
That’s not necessary, and checking the number of children in a
serialised variant is not necessarily cheap.
Move the checks around so that the number of children is only checked
once on each code path. This doesn’t introduce any functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
As with commit 9ae59bd647, deeply nested
maybes in an array can be exploited by a malicious caller to cause a
geometric increase in processing time and number of `GVariant` instances
handled by the `g_variant_print()` code.
Optimise this by skipping recursing through most of the chain of maybes,
thus avoiding all the setup checks in each recursive call.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
oss-fuzz#54577
