The previous implementation had some pretty short cycles and two fixed
points (1711463637 and 2389024350). If two keys hashed to one of these
values insertions and lookups would loop forever.
This version is based on a standard xorshift PRNG with period 2**32-1.
The all-zero state is usually forbidden, so we insert it into the cycle
at an arbitrary location.
This is just to allow removing the 'clang-format off' directive. This
concept is only used within this header, so it doesn't need to be in the
global namespace.
We have whittled away at the usages of these AK::Vector APIs in the
Kernel. This change disables them from being visible when building
the Kernel to make sure no new usages get added.
If we didn't define our own `move` and `forward` and inject it into the
`std` namespace, then we would error just after trying to `using` those,
if no one has included it before. Now, we will include `utility` from
the STD if we aren't replacing the `std`.
This was a premature optimization from the early days of SerenityOS.
The eternal heap was a simple bump pointer allocator over a static
byte array. My original idea was to avoid heap fragmentation and improve
data locality, but both ideas were rooted in cargo culting, not data.
We would reserve 4 MiB at boot and only ended up using ~256 KiB, wasting
the rest.
This patch replaces all kmalloc_eternal() usage by regular kmalloc().
This is an interface to downcast(), which degrades errors into runtime
errors, and allows seemingly-correct-but-not-quite constructs like the
following to appear to compile, but fail at runtime:
Variant<NonnullRefPtr<T>, U> foo = ...;
Variant<RefPtr<T>, U> bar = foo;
The expectation here is that `foo` is converted to a RefPtr<T> if it
contains one, and remains a U otherwise, but in reality, the
NonnullRefPtr<T> variant is simply dropped on the floor, and the
resulting variant becomes invalid, failing the assertion in downcast().
This commit adds a Variant<Ts...>(Variant<NewTs...>) constructor that
ensures that no alternative can be left out at compiletime, for the
users that were using this interface for merely increasing the number of
alternatives (for instance, LibSQL's Value class).
This is a raffinement of 49cbd4dcca.
Previously, the container was scanned to compute the size in the unhappy
path. Now, using `all_of` happy and unhappy path should be fast.
In order to reduce our reliance on __builtin_{ffs, clz, ctz, popcount},
this commit removes all calls to these functions and replaces them with
the equivalent functions in AK/BuiltinWrappers.h.
While watching Andreas' most recent video, I noticed that this function
only worked with 32 bit values, but was a serious performance
bottleneck for the kernel. As such, I reworked it to use `size_t`, so
it now can switch to 64-bit sweeps on 64-bit platforms. This caused
test-js to go from 12.5 seconds hot to 11.5 seconds hot on my machine
when running on KVM x86_64.
The goal of this file is to enable C++ overloaded functions for
standard builtin functions that we use. It contains fallback
implementations for systems that do not have the builtins available.
Before, if we couldn't read enough data out of the buffer, we would re-
fill the buffer and recursively call read(), which in turn reads data
from the buffer into the resliced target span. This incurs very
intensive superflous memmove's when large chunks of data are read from
a buffered stream.
This commit changes the behavior so that when we exhaust the buffer, we
first read any necessary additional data directly into the target, then
fill up the buffer again. Effectively, this results in drastically
reduced overhead from Buffered when reading large contiguous chunks.
Of course, Buffered is designed to speed up data access patterns with
small frequent reads, but it's nice to be able to combine both access
patterns on one stream without penalties either way.
The final performance gain is about an additional 80% of abench decoding
speed.
This unbreaks the /var/run/utmp system which starts out as an empty
string, and is then turned into an object by the first update.
This isn't necessarily the best way for this to work, but it's how
it used to work, so this just fixes the regression for now.
This fixes at least half of our LibC includes in the kernel. The source
of truth for errno codes and their description strings now lives in
Kernel/API/POSIX/errno.h as an enumeration, which LibC includes.
This is the same strategy that LLVM's compiler-rt uses to make sure that
each UBSAN error is only reported once, when UBSAN is *not* deadly.
Otherwise, each time we head through a UB codepath, we will log the same
error over and over. That behavior just adds noise to the logs and makes
it nearly impossible to run binaires that have some common code path
with flagged UB in them.
compiler-rt goes the extra step to make sure the "clear" action is
atomic, but we don't really have that many multi-threaded apps gettting
tested with UBSAN yet, so we can add that later.
This will allow us to avoid some potentially expensive type conversion
during lookup, like form String to StringView, which would allocate
memory otherwise.
Instead of checking __linux__ macro directly, the code should check if
this macro is defined. This is already done correctly a couple of lines
above.
I ran into this when trying to build libjs-test262 on MacOS where I got
the following error message
error: "__linux__" is not defined, evaluates to 0 [-Werror=undef]
This is a convenience template that implements reference count
forwarding. This means that an object forwards ref() and unref() to
another object.
We can use this when two ref-counted objects need to keep each other
alive. This situation poses two problems:
- Using 2x RefPtr would cause a ref cycle and leak both objects.
- Using 2x WeakPtr would allow one of them to be destroyed early.
With RefCountForwarder, only one of the objects has a ref count. The
object with the ref count points to the forwarding object by using a
non-counting smart pointer (OwnPtr or NonnullOwnPtr). Thus, both objects
are kept alive by the same ref count, and they can safely point to each
other without worrying about disjoint lifetimes.
Note that the return type for the non-const method error() changed. This
is most likely an accident, hidden by the fact that ErrorType typically
is Error.
This makes it an error to not do something with a returned smart
pointer, which should help prevent mistakes. In cases where you do need
to ignore the value, casting to void will placate the compiler.
I did have to add comments to disable clang-format on a couple of lines,
where it wanted to format the code like this:
```c++
private : NonnullRefPtr() = delete;
```
Previously, Vector::extend for a moved vector would move the other
vector into this vector if this vector was empty, thereby throwing away
existing allocated capacity. Therefore, this commit allows the move to
only happen if this vector's capacity is too small to fit the other
vector. This will also alleviate bugs where callers relied on the
capacity to never shrink with calls to unchecked_append, extend and the
like.
This mirrors the existence of append() for data pointers and is very
useful when the program needs to have a guarantee of no allocations,
as is necessary for real-time audio.
During the build process on macOS, multiple versions of <unistd.h> were
being included (Apple's version and GCC's version). It appears that
all other places #include the version from GCC, but in Platform.h the
Apple header was being used. GCC's <unistd.h> is wrapped in
`extern "C"`, while Apple's is not. This causes a conflicting
declaration, so we need to wrap the #include with extern "C".
Issue has been observed on macOS Mojave.
See https://github.com/microsoft/vcpkg/issues/11320 for a similar issue.
This creates an error that contains the name of the syscall that failed.
This allows error handlers to print out the name of the call if they
want to. :^)
Avoid including a per-translation unit copy of all these functions.
Also, drive-by two clang-tidy fixes for readability-qualified-auto and
readability-implicit-bool-conversion.
Currently TimeManagement wont compile on AARCH64, so it is not included.
This creates a link error since format.cpp now relies on functionality
in TimeManagement.cpp to add timestamps to log lines.
This PR disables that functionality for AARCH64 builds until
TimeManagement will compile.
This is a naive implementation based on the symmetry with `asin`.
Before, I'm not really sure what we were doing, but it was returning
wildly incorrect results.
This isn't a complete conversion to ErrorOr<void>, but a good chunk.
The end goal here is to propagate buffer allocation failures to the
caller, and allow the use of TRY() with formatting functions.
Also add slightly richer parse errors now that we can include a string
literal with returned errors.
This will allow us to use TRY() when working with JSON data.
This feels like it was a refactor transition kind of conversion. The
places that were relying on it can easily be changed to explicitly ask
for the ptr() or a new vaddr() method on Userspace<T*>.
FlatPtr can still implicitly convert to Userspace<T> because the
constructor is not explicit, but there's quite a few more places that
are relying on that conversion.
In AK::Function::CallableWrapper::init_and_swap(), clang-tidy wants us
to mark the destination argument as pointer to const, which doesn't work
because we use placement new to construct a move'd *this into.
cert-dcl50-cpp: No variadic functions, suppressed in RefCounted and
ThreadSafeRefCounted for implementing the magic one_ref_left and
will_be_destroyed functions.
cert-dcl58-cpp: No opening ::std, suppressed in the places we put names
in ::std to aid tools (move, forward, nullptr_t, align_val_t, etc).
Either not returning *this, or in the case of Variant, not checking for
self assignment. In AK::Atomic, we can't return *this due to the wrapper
semantics Atomic implements.
Using `l` for long double causes a clang-tidy warning, so use all caps
suffixes for all of the AK::abs() overloads for consistency. Also, avoid
leaking the internal __DEFINE_GENERIC_ABS macro.
The definition of VERIFY_NOT_REACHED() as `assert(false)` causes the
tool to suggest converting it to a static_assert. Which doesn't make
any sense in context for what the macro is trying to do: crash the
program at runtime.
Add a basic clang-tidy configuration that enables checks from the
following categories:
- bugprone
- cert
- clang-analyzer
- concurrency
- misc
- performance
- portability
- readability
The noisiest rules that have conflicts with the project style guide or
accepted practices have been turned off.
There's absolutely more work to be done here before we could consider
setting any of these warnings as errors and enforcing them in CI, but
committing a project clang-tidy configuration should help the rules
become more visible and let other contributors take a crack at tweaking
rules and/or finding possible bugs.
Sadly the cpp-core-guidelines and modernize categories are very, very
noisy. If we want to enable rules from these categories, they would need
to be on a rule by rule basis.
Previously if code attempted to use the format specifier somewhere
(Ex: "%#4.3Lg"), the specifier would get dropped and we would just
print "g" instead of any value. Now at least we print a value.
Same as Vector, ByteBuffer now also signals allocation failure by
returning an ENOMEM Error instead of a bool, allowing us to use the
TRY() and MUST() patterns.
Instead of signalling allocation failure with a bool return value
(false), we now use ErrorOr<void> and return ENOMEM as appropriate.
This allows us to use TRY() and MUST() with Vector. :^)
All the read-only methods of Bitmap simply defer to BitmapView. Let's
make this relationship official by using class inheritance. This might
even shave off a few instructions, although any sufficiently optimizing
compiler probably already optimized them away.
We now use AK::Error and AK::ErrorOr<T> in both kernel and userspace!
This was a slightly tedious refactoring that took a long time, so it's
not unlikely that some bugs crept in.
Nevertheless, it does pass basic functionality testing, and it's just
real nice to finally see the same pattern in all contexts. :^)
This variant returns ErrorOr<NonnullOwnPtr<T>> instead of KResultOr.
Eventually the KResultOr variant should go away once the kernel adopts
Error and ErrorOr<T>.
This is an alternative to ErrorOr<T>::release_value() that can be used
when converting code to signal that we're releasing the value without
error propagation as a way to move forward now.
This makes these cases much easier to find later on, once more paths for
error propagation are available.
The goal with these is to eventually replace AK::Result, KResult and
KResultOr<T> with something that works (and makes sense) in both kernel
and userspace.
This first cut of Error can be made from an errno code, or from a string
literal (StringView)
When I added this code in 1472f6d, I forgot to add tests for it. That's
why I didn't realize that the values were appended to the wrong
FormatBuilder object, so an empty string was returned instead of the
expected "nan"/"inf". This made debugging some FPU issues with the
ScummVM port significantly more difficult.
For some algorithms, such as bloom filters, it's possible to reuse a
hash function (rather than having different hashing functions) if the
seed is different each time the hash function is used.
Modify AK::string_hash() to take a seed parameter, which defaults to 0
(the value the hash value was originally initialized to).
In the long-term, we should probably have a way to signal decoding
failure. For now, it should suffice to at least not crash. This is
particularly relevant because apparently this can be triggered while
parsing a PEM certificate, which happens during every TLS connection.
Found by OSS Fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=38979
In particular, we implicitly required that the caller initializes the
returned instances themselves (solved by making
UniformBumpAllocator::allocate call the constructor), and BumpAllocator
itself cannot handle classes that are not trivially deconstructible
(solved by deleting the method).
Co-authored-by: Ali Mohammad Pur <ali.mpfard@gmail.com>
This fixes two bugs:
1. `end_offset` was missing the alignment that might have been
introduced while computing `base_ptr`.
2. Ignoring point 1, `end_offset` computed the offset of the first byte
that is outside the current chunk. However, this might be in the
middle of a (hypothetical) object! The loop treats `end_offset` as if
it points to the first byte beyond the last (valid) object. So if the
last few bytes of the chunk are unused, the loop iterates once too
often.
Found by OSS Fuzz, long-standing issue (since 2021-07-31)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=38733
(This probably also resolves some other issues that go through
RegexMatcher.)
See also: 0f1425c895
Creating a String object from the formatted data is unnecessary, as it
immediately gets turned into a StringView anyways.
With this change, we will no longer allocate on the heap when printing
`VirtualAddress`, `VirtualRange` and `InodeIdentifier` objects, which is
a step towards stronger OOM hardening.
Forcing the formatting to go through `Formatter<FormatString>` is
completely unnecessary, increases code size, performs a String
allocation and prevents us from using the formatting options available
on that type.
This commit also removes explicit formatters from
`BlockBasedFileSystem::BlockIndex` and `Kernel::InodeIndex`, as those
are already covered by the blanket implementation for all
`DistinctNumeric` types.
There is no need to have a user-defined copy constructor that simply
calls the base class's copy constructor. By having the compiler generate
it for us, Span is made trivially copyable, so it can be passed in
registers.
This function converts a single wide character into its multibyte
representation (UTF-8 in our case). It is called from libc++'s
`std::basic_ostream<wchar_t>::flush`, which gets called at program exit
from a global destructor in order to flush `std::wcout`.
The platform independent Processor.h file includes the shared processor
code and includes the specific platform header file.
All references to the Arch/x86/Processor.h file have been replaced with
a reference to Arch/Processor.h.
Both my approach and the previous approach were wrong for different
cases. I've changed the Iterators index from storage-relative to
queue-relative, and it's much simpler and more obviously correct.
fixes#10383
While I was working on LibWeb, I got a page fault at 0xe0e0e0e4.
This indicates a destroyed RefPtr if compiled with SANITIZE_PTRS
defined. However, the page fault handler didn't print out this
indication.
This makes the page fault handler print out a note if the faulting
address looks like a recently destroyed RefPtr, OwnPtr, NonnullRefPtr,
NonnullOwnPtr, ThreadSafeRefPtr or ThreadSafeNonnullRefPtr. It will
only do this if SANITIZE_PTRS is defined, as smart pointers don't get
scrubbed without it being defined.
Some time ago, automatic locking was added to the AK smart pointers to
paper over various race conditions in the kernel. Until we've actually
solved the issues in the kernel, we're stuck with the locking.
However, we don't need to punish single-threaded userspace programs with
the high cost of locking. This patch moves the thread-safe variants of
RefPtr, NonnullRefPtr, WeakPtr and RefCounted into Kernel/Library/.
Example failure:
IDAllocator.h only pulls in AK/Hashtable.h, so any compilation unit that
includes AK/IDAllocator.h without including AK/Traits.h before it used
to be doomed to fail with the cryptic error message "In instantiation of
'AK::HashTable<T, TraitsForT, IsOrdered>::Iterator AK::HashTable<T,
TraitsForT, IsOrdered>::find(const T&) [with T = int; TraitsForT =
AK::Traits: incomplete type 'AK::Traits<int>' used in nested name
specifier".
This change also applys to to_uint
On i686 u64 == long long but on x86_64 u64 == long. Therefor on either
arch, one of the instantiations is missed. This change makes sure that
all integer types have an instantiation.