Rather than duplicating the alignment checks when constructing a new
GVariant, re-use the alignment checks from GVariantSerialised. This
ensures that the same checks are done everywhere in the GVariant code.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1342
Otherwise the GVariant would later fail internal alignment checks,
aborting the program.
If unaligned data is provided to (for example)
g_variant_new_from_data(), it will copy the data into a new aligned
allocation. This is slow, but better than crashing. If callers want
better performance, they should provide aligned data in their call, and
it will not be copied or reallocated.
Includes a unit test.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1342
This was causing a crash, because we were first removing an item, freeing
both the instance itself and the key, and then trying to reuse those.
So, in this case, instead of reassigning an item, we can just return TRUE
as we have already the item at the right place, while it's not needed to
update the modified timestamp, since no modification happened in reality.
Fixes#1588
Synchronize access to random number generator `test_run_rand` with
a lock to ensure that `g_test_rand_*` family of functions is
thread-safe.
The reseeding taking place between test case runs is intentionally left
unsynchronized. It is an error to continue using random number generator
after test case has already finished running. Lack of synchronization
here will make such erroneous use readily apparent with thread
sanitizer.
This test isn't inherently slow, but it produces so much output that
it can take a minute or more on hardware with weak I/O performance.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This reverts commits:
• 9ddcc79502
• ae02adc3c3
g_date_time_format() supports a few non-standard format placeholders:
• %:z
• %::z
• %:::z
These are all gnulib strtime() extensions, and hence are not recognised
by the compiler when the function is annotated with G_GNUC_STRFTIME.
However, this wasn’t noticed when we originally merged this change
because the errors were disabled in the tests which covered those
placeholders.
This does not work, since g_date_time_format() supports
non-standard extensions such as %:::z, and this has
broken several consumers which use format errors, such
as ostree.
This is desirable both to get more detailed failure messages; and
because g_assert() is compiled out when compiling with G_DISABLE_ASSERT,
which renders the tests useless.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
gint is not the best type when looping from 0 to N > 0, which usually is
the case in loops. There are a few cases in this patch where guint is
used rather than gsize, this is when the index is used in a printf-like
function as this makes the format string easier to read
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Formatting code for `%z` specifier incorrectly assumed that sign of
offset from UTC can be recovered from the number of hours alone, which
is not true for offsets between -01:00 and +00:00.
Extract and format sign separately to avoid the problem.
Issue #1337.
Previously, the code which parsed comments in key files would append a
line break to the comment where there was none before; this was part of
the code for handling re-inserting line breaks into multi-line comments
after removing the ‘#’ prefix. Now, we don’t add a terminal line break.
This was slightly icky to implement because parse_value_as_comment() is
called once for each line of a multi-line comment.
This expands the existing test case to cover a single line comment, and
also fixes the documentation to correctly state that the leading ‘#’
*is* removed and mention the new line break behaviour.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/107
When g_date_set_parse was used with more than one locale it could
incorrectly retain information from previous one. Reinitialize all
locale specific data inside g_date_prepare_to_parse to avoid the issue.
g_source_set_callback() and g_source_set_callback_indirect() are both
safe to call zero or more times on attached sources. The change in
callback will take effect the next time the source is dispatched, after
the set_callback() call returns (it could block due to locking).
https://gitlab.gnome.org/GNOME/glib/issues/827
FreeBSD 12 adds a new header, sys/auxv.h, to declare a function, elf_aux_info,
for public use, which was considered an internal function in previous releases.
This new function provides similar functionality with glibc getauxval, which is
also declared in the same header, but their interfaces are not compatible. Since
the only usage of sys/auxv.h is in g_check_setuid and FreeBSD already has
issetugid to provide the required functionality, we fixes the compilation error
by adding a check for getauxval function to prevent g_check_setuid from calling
getauxval when sys/auxv.h is found but getauxval is not available.
https://reviews.freebsd.org/D12743https://reviews.freebsd.org/rS324815
Previously, the markup parsing test would load a given markup file and
try to parse it several ways. It would return as soon as one of the
attempts failed — meaning that bugs only seen with non-nul-terminated,
or differently chunked, parse runs could never be caught.
Rework the tests so that all markup files are tested all ways, and we
assert that all ways of parsing them give the same result.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Previously, the element name validation only happened if a start_element
callback was specified on the context. Element name validation should be
unconditional.
This was causing test-5.gmarkup to fail when run against the improved
tests in the following commit.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When extracting a UTF-8 character to put in an error message on parse
failure, pass the remaining buffer length to utf8_str() to avoid it
running off the end of the input buffer. It previously assumed that the
buffer was nul-terminated, which was the case in all the tests until
now.
A following commit will add test coverage for this.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When using GMarkup to parse a string, the string can be provided with an
explicit length specified, or with no length and a nul terminator
instead. Run all the GMarkup tests both ways, to catch problems with
length checks, or with nul terminator checks.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This doesn’t trigger any new failures, but is distinct from other tests
we have, so would be good to retain.
Related to commit cec7170540.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
These were callers which explicitly specified the string length to
g_utf8_validate(), when it couldn’t be negative, and hence should be
able to unconditionally benefit from the increased string handling
length.
At least one call site would have previously silently changed behaviour
if called with strings longer than G_MAXSSIZE in length.
Another call site was passing strlen(string) to g_utf8_validate(), which
seems pointless: just pass -1 instead, and let g_utf8_validate()
calculate the string length. Its behaviour on embedded nul bytes
wouldn’t change, as strlen() stops at the first one.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This is a variant of g_utf8_validate() which requires the length to be
specified, thereby allowing string lengths up to G_MAXSIZE rather than
just G_MAXSSIZE.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When validating a string to see if it’s valid UTF-8, we pass a gsize to
g_utf8_validate(), which only takes a gssize. For large gsize values,
this will result in the gssize actually being negative, which will
change g_utf8_validate()’s behaviour to stop at the first nul byte. That
would allow subsequent nul bytes through the string validator, against
its documented behaviour.
Add a test case.
oss-fuzz#10319
Signed-off-by: Philip Withnall <withnall@endlessm.com>
As with the previous commit, when getting a child from a serialised
tuple, check its offset against the length of the serialised data of the
tuple (excluding the length of the offset table). The offset was already
checked against the length of the entire serialised tuple (including the
offset table) — but a child should not be able to start inside the
offset table.
A test is included.
oss-fuzz#9803
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When getting a child from a serialised variable array, check its offset
against the length of the serialised data of the array (excluding the
length of the offset table). The offset was already checked against the
length of the entire serialised array (including the offset table) — but a
child should not be able to start inside the offset table.
A test is included.
oss-fuzz#9803
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Previously, GVariant has allowed ‘arbitrary’ recursion on GVariantTypes,
but this isn’t really feasible. We have to deal with GVariants from
untrusted sources, and the nature of GVariantType means that another
level of recursion (and hence, for example, another stack frame in your
application) can be added with a single byte in a variant type signature
in the input. This gives malicious input sources far too much leverage
to cause deep stack recursion or massive memory allocations which can
DoS an application.
Limit recursion to 128 levels (which should be more than enough for
anyone™), document it and add a test. This is, handily, also the limit
of 64 applied by the D-Bus specification (§(Valid Signatures)), plus a
bit to allow wrapping of D-Bus messages in additional layers of
variants.
oss-fuzz#9857
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When checking whether a serialised GVariant tuple is in normal form,
it’s possible for `offset_ptr -= offset_size` to underflow and wrap
around, resulting in gvs_read_unaligned_le() reading memory outside the
serialised GVariant bounds.
See §(Tuples) in gvariant-serialiser.c for the documentation on how
tuples are serialised. Briefly, all variable-length elements in the
tuple have an offset to their end stored in an array of offsets at the
end of the tuple. The width of each offset is in offset_size. offset_ptr
is added to the start of the serialised tuple to get the offset which is
currently being examined. The offset array is in reverse order compared
to the tuple elements, hence the subtraction.
The bug can be triggered if a tuple contains a load of variable-length
elements, each of whose length is actually zero (i.e. empty arrays).
Includes a unit test.
oss-fuzz#9801
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Emulated futexes are slower than real ones; if they were not, there
would be no point in using the real futexes. On some machines they
are sufficiently slow to cause test timeouts.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Fedora is using https://fedoraproject.org/wiki/Changes/Annobin
to try to ensure that all objects are built with hardening flags.
Pass down `CFLAGS` to ensure the SystemTap objects use them.
Without gatomic.h, build fails on:
In file included from garcbox.c:24:0:
garcbox.c: In function ‘g_atomic_rc_box_acquire’:
grefcount.h:101:13: error: implicit declaration of function ‘g_atomic_int_get’; did you mean ‘__atomic_store’? [-Werror=implicit-function-declaration]
(void) (g_atomic_int_get (rc) == G_MAXINT ? 0 : g_atomic_int_inc ((rc))); \
^
garcbox.c:292:3: note: in expansion of macro ‘g_atomic_ref_count_inc’
g_atomic_ref_count_inc (&real_box->ref_count);
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
All pool threads are named "pool" and this a bit annoying when looking
at system-wide traces or statistics for a system where several
applications use thread pools. Include the prgname in the thread names
to get a better default name. The total length including the "pool-"
prefix is limited to 16 bytes in order for it to work on all systems.
Change-Id: I473a9f534c4630f3e81da72ff96d8f593c60efac
A double paren forces the compiler to assume that the
statement is right. That may not be the case.
This is essentially reverting b44fba25fb.
See https://bugzilla.gnome.org/show_bug.cgi?id=760215.
It's more morth to allow find common mistakes (= instead of ==
in conditionals) than masking them to make some rarely used
code work.
We still clear the key/value on removal, but since we're growing the
arrays with realloc() now, we can't guarantee that incoming memory is
cleared. There's no reason it should be either, since we check the
hashes array (which is always in a defined state) before accessing the
other arrays.
When g_hash_table_resize() gets called, we clear out tombstones and grow
the table at the same time if needed. However, the threshold was set too
low, so we'd grow if the load was greater than .5 after subtracting
tombstones. Increase this threshold to ~.75.
When resizing, we were keeping both the old and new hash, key and value
arrays around while we reinserted entries, resulting in a peak memory
overhead of 50%. Using a temporary bookkeeping array with one bit per
entry we can now grow and shrink the main arrays using realloc() and an
eviction scheme, reducing the overhead to .625% (assuming 64-bit keys and
values). Tests show the CPU overhead is negligible.
If int is smaller than void * on our arch, we start out with
int-sized keys and values and resize to pointer-sized entries as
needed. This saves a good amount of memory when the HT is being
used with e.g. GUINT_TO_POINTER().
I’m fed up of trying to read these and having my head done in by mixed
tabs and spaces.
This introduces no functional changes.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This avoids the convenience library being treated as though it was
an installed static library (objects not included in the dependent
static library, and convenience library being listed in the pkg-config
metadata), both of which would make static linking impossible.
This is a workaround for meson not having
https://github.com/mesonbuild/meson/pull/3939 merged yet.
Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1536
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously, it was installed unmodified by the Meson build system.
We don't need to define @bindir@, because gtester-report never
actually referred to it. We also don't need a definition of
GTESTER_REPORT for use by GLib itself, because its last use was removed
in 2013 (https://bugzilla.gnome.org/show_bug.cgi?id=709995).
Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1544
Signed-off-by: Simon McVittie <smcv@collabora.com>
This makes it easier to debug test failures, by ensuring that g_debug()
and g_test_message() are printed as TAP diagnostics.
Fixes: https://gitlab.gnome.org/GNOME/glib/issues/1528
Signed-off-by: Simon McVittie <smcv@collabora.com>
The text about deallocation of GArrays with elements containing
dynamically-allocated memory was confusing. It initially mentioned
clear_func, but later said elements with dynamically allocated memory
"should be freed separately".
Clarify this by using the same structure as g_ptr_array_free —
highlight the need to set a clear_func by consolidating the text about
it in a separate paragraph.
https://gitlab.gnome.org/GNOME/glib/merge_requests/348
Previously, in the case where 'kf' has more than one ref, calling
g_key_file_free(kf) would break it. For example, calling
g_key_file_has_key(kf, ...) would hit the following assertion:
g_hash_table_lookup: assertion 'hash_table != NULL' failed
This is because g_key_file_free() calls g_key_file_clear() which sets
self->groups and other fields to NULL; most lookup functions assume
these fields are non-NULL.
One fix would be to call g_key_file_init() right after
g_key_file_clear() in g_key_file_free(). However, in the case where
there are no other refs to the keyfile, this would mean allocating
many new hash tables which will be immediately destroyed when
g_key_file_unref() removes the last ref. Instead, inline the unref, and
re-initialize the internal state when the keyfile is still alive.
1) Creating a directory with 0666 does not prevent
traversal on Windows (ACL determines the possibility
of traversal, and Windows mkdir() does not translate
permission bits into ACL). Don't do the traversal check on Windows.
2) Creating a file with 0555 also isn't translated into
read-only ACL, Windows sets the read-only attribute instead,
which blocks all changes, including changes to file times.
Add the write permissions on Windows before changing file times.
It turns out that UCRT (which is the C runtime that Visual Studio
uses by default these days) quietly changed the semantics for
stat() functions and its variants. Previously they provided data
for the symlink (if the file queried was a symlink), now they
provide data for the target of the symlink.
glib used to call _wstat64() to get certain stat-ish data about
files, such as st_dev, since it was deemed that computing that
data in glib using custom code would be pointless, as _wstat64()
worked just fine.
With UCRT this is no longer true. This commit drops _wstat64() in
favour of a bunch of custom calls that populate the stat buffer
manually. This way glib correctly gets information about a symlink,
not its target.
The _fstat64() call is still being used, as it does not suffer
from this problem and thus remains untouched.
Even if we're using a prime modulo for the initial probe, our table is
power-of-two-sized, meaning we can set the mask simply by subtracting one
from the size.
Sequential integers would be densely packed in the table, leaving the
high-index buckets unused and causing abnormally long probes for many
operations. This was especially noticeable with failed lookups and
when "aging" the table by repeatedly inserting and removing integers
from a narrow range using g_direct_hash() as the hashing function.
The solution is to multiply the hash by a small prime before applying
the modulo. The compiler optimizes this to a few left shifts and adds, so
the constant overhead is small, and the entries will be spread out,
yielding a lower average probe count.
Extended path prefix looks like "\\?\",
and NT object path prefix looks like "\??\".
Strip them only if they are followed by a character
(any character) and a colon (:), indicating that
it's a DOS path with a drive.
Otherwise stripping such prefix might result in a patch
that looks like a relative path.
For example, "\\?\Volume{GUID}\" becomes "Volume{GUID}\",
which is a valid directory name.
Currently it's up to the user to make sense of such paths.
When running a test as a subprocess and matching its output, it’s very
annoying for GLib to tell you that the output didn’t match your pattern,
*but not actually say what the output was*. Fix that.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
