StringView was used where possible. Some utilities still use libc
functions which expect null-terminated strings, so String objects were
used there instead.
The main event loop functionality was used in just two places where the
alternative is a bit simpler. Remove it in favor of referencing the
event loop directly, or just invoking `EventLoop::current()`.
Note that we don't need locking in the constructor since we're now only
modifying a thread-local `Vector`. We also don't need locking in the
old call sites to `::with_main_locked()` since we already lock the
event loop in the subsequent `::post_event()` invocation.
Previously we captured them by reference, but when the deferred code
finally runs from an event loop, the references may be stale.
In that case, value and max of the progressbar are set with random
numbers.
If we were especially unlucky, the `frame_count` turned into a negative
int, and would crash at `VERIFY(min <= max)`.
The idea of locking the process veil in CrashReproter is well
intentioned, but ultimately frought with issues.
The fundamental premise is a bit flawed, as we are using the crashing
program as input to dynamically add new paths to the process veil.
This means that an attacker can potentially produce a custom or
malformed binary to trick CrashReporter into allowing an arbitrary
path to be read.
We can't fiddle with GUI widgets off the main thread, so let's use
Core::EventLoop::deferred_invoke() to dispatch the work.
The progress bar doesn't visibly update yet, but at least we're not
crashing anymore.
Previously when opening a crash report for HackStudio, the
`unveil("/bin/HackStudio", "rx")` call was failing because of the
earlier `unveil(executable_path.characters(), "r")` call requesting only
"r" permissions for it. This patch handles this specific case, so you
can crash HackStudio to your heart's content. :^)
Also, we were unveiling the executable path twice, once manually and
once implicitly as part of the coredump's libraries, so we now check for
the latter and avoid it.
Thanks to Daniel for noticing what was right in front of me and I didn't
see!
Co-authored-by: Daniel Bertalan <dani@danielbertalan.dev>
Previously we would create a temporary progress window to show a
progressbar while the coredump is processed. Since we're only waiting
on backtraces and CPU register states, we can move the progressbar
into the main window and show everything else immediately while the
slow parts are generated in a BackgroundAction.
Previously, when the --unlink flag was passed to CrashReporter, it
unlinked the coredump file immediately after reading it.
This change makes it so the coredump file is deleted when CrashReporter
exits.
Some coredumps take a long time to symbolicate, so let's show a simple
window with a progress bar while they are loading.
I'm not super happy with the factoring of this feature, but it's an
absolutely kickass feature that makes crashing feel 100% more responsive
than before, since you now get GUI feedback almost immediately after a
crash occurs. :^)
Before this patch, this is what would happen after something crashed:
1. CrashDaemon finds a new coredump in /tmp
2. CrashDaemon compresses the new coredump (gzip)
3. CrashDaemon parses the uncompressed coredump and prints a backtrace
4. CrashDaemon launches CrashReporter
5. CrashReporter parses the uncompressed coredump (again)
6. CrashReporter unlinks the uncompressed coredump
7. CrashReporter displays a GUI
This was taking quite a long time when dealing with large programs
crashing (like Browser's WebContent processes.)
The new flow:
1. CrashDaemon finds a new coredump in /tmp
2. CrashDaemon mmap()'s the (uncompressed) coredump
3. CrashDaemon launches CrashReporter
4. CrashDaemon goes to sleep for 3 seconds (hack alert!)
5. CrashReporter parses the (uncompressed) coredump
6. CrashReporter unlinks the (uncompressed) coredump
7. CrashReporter displays a GUI
8. CrashDaemon wakes up (after step 4)
9. CrashDaemon compresses the coredump (gzip)
TL;DR: we no longer parse the coredumps twice, and we also prioritize
launching the CrashReporter GUI immediately when a new coredump shows
up, instead of compressing and parsing it in CrashDaemon first.
The net effect of this is that you get a backtrace on screen much
sooner. That's pretty nice. :^)
This allows for typing [8] instead of [8, 8, 8, 8] to specify the same
margin on all edges, for example. The constructors follow CSS' style of
specifying margins. The added constructors are:
- Margins(int all): Sets the same margin on all edges.
- Margins(int vertical, int horizontal): Sets the first argument to top
and bottom margins, and the second argument to left and right margins.
- Margins(int top, int vertical, int bottom): Sets the first argument to
the top margin, the second argument to the left and right margins,
and the third argument to the bottom margin.
Depending on the values it might be difficult to figure out whether a
value is decimal or hexadecimal. So let's make this more obvious. Also
this allows copying and pasting those numbers into GNOME calculator and
probably also other apps which auto-detect the base.
The LexicalPath instance methods dirname(), basename(), title() and
extension() will be changed to return StringView const& in a further
commit. Due to this, users creating temporary LexicalPath objects just
to call one of those getters will recieve a StringView const& pointing
to a possible freed buffer.
To avoid this, static methods for those APIs have been added, which will
return a String by value to avoid those problems. All cases where
temporary LexicalPath objects have been used as described above haven
been changed to use the static APIs.
Since applications using Core::EventLoop no longer need to create a
socket in /tmp/rpc/, and also don't need to listen for incoming
connections on this socket, we can remove a whole bunch of pledges!
SPDX License Identifiers are a more compact / standardized
way of representing file license information.
See: https://spdx.dev/resources/use/#identifiers
This was done with the `ambr` search and replace tool.
ambr --no-parent-ignore --key-from-file --rep-from-file key.txt rep.txt *
Most coredumps contain large amounts of consecutive null bytes and as
such are a prime candidate for compression.
This commit makes CrashDaemon compress files once the kernel finishes
emitting them, as well as adds the functionality needed in LibCoreDump
to then parse them.
This was needlessly expecting the first backtrace entry function name to
start with '__assertion_failed', which is no longer the case - it's now
something from libsystem.so. Let's just check whether we have an
'assertion' key in the coredump's metadata, just like we do for pledge
violations.
Now that WindowServer broadcasts the system theme using an anonymous
file, we need clients to pledge "recvfd" so they can receive it.
Some programs keep the "shared_buffer" pledge since it's still used for
a handful of things.
This is in preparation of adding (much) more process information to
coredumps. As we can only have one null-terminated char[] of arbitrary
length in each struct it's now a single JSON blob, which is a great fit:
easily extensible in the future and allows for key/value pairs and even
nested objects, which will be used e.g. for the process environment, for
example.
