If you've been hacking on SerenityOS for a while you've probably seen
backtraces like this:

USERSPACE(23) ASSERTION FAILED: index.internal_data()
../Libraries/LibGUI/FileSystemModel.cpp:325
0xdeadc0de  Kernel::Scheduler::context_switch(Kernel::Thread*) +183
0xdeadc0de  Kernel::Scheduler::pick_next() +1681
0xdeadc0de  Kernel::Processor::check_invoke_scheduler() +125
0xdeadc0de  Kernel::Thread::die_if_needed() +157
0xdeadc0de  syscall_handler +1386
0xdeadc0de  syscall_asm_entry +49
0x080d5f29  raise +22
0x080480bd  abort +13
0x080d2402  tolower +0
0x0806fe11  GUI::FileSystemModel::node(GUI::ModelIndex const&) const +63
0x0804a426  DirectoryView::update_statusbar() +844

I always thought it was suspicious to see tolower() between the asserting
function and abort(), but I never looked into it until now.

As it turns out, it's pretty simple. This is how we declare abort():

    __attribute__((noreturn)) void abort();

Since abort() is marked noreturn, GCC simplifies some things when emitting
a call to it.

Here's the corresponding assembly for the crash above:

080d23cf <__assertion_failed>:
 80d23cf:       55                      push   ebp
 80d23d0:       89 e5                   mov    ebp,esp
 80d23d2:       53                      push   ebx
 80d23d3:       50                      push   eax
 80d23d4:       8b 5d 08                mov    ebx,DWORD PTR [ebp+0x8]
 80d23d7:       e8 5a 16 01 00          call   80e3a36 <getpid>
 80d23dc:       52                      push   edx
 80d23dd:       53                      push   ebx
 80d23de:       50                      push   eax
 80d23df:       68 90 d4 10 08          push   0x810d490
 80d23e4:       e8 e1 6c 00 00          call   80d90ca <dbgprintf>
 80d23e9:       83 c4 0c                add    esp,0xc
 80d23ec:       53                      push   ebx
 80d23ed:       68 9e d4 10 08          push   0x810d49e
 80d23f2:       ff 35 00 41 11 08       push   DWORD PTR ds:0x8114100
 80d23f8:       e8 36 9b 00 00          call   80dbf33 <fprintf>
 80d23fd:       e8 ae 5c f7 ff          call   80480b0 <abort>

080d2402 <tolower>:
 80d2402:       55                      push   ebp
 80d2403:       89 e5                   mov    ebp,esp
 80d2405:       8b 45 08                mov    eax,DWORD PTR [ebp+0x8]
 80d2408:       8d 50 bf                lea    edx,[eax-0x41]
 80d240b:       83 fa 19                cmp    edx,0x19
 80d240e:       77 03                   ja     80d2413 <tolower+0x11>
 80d2410:       83 c8 20                or     eax,0x20
 80d2413:       5d                      pop    ebp
 80d2414:       c3                      ret

As you can see, if abort() actually *did* return, it would return straight
into the start of tolower(). This never happens since the process kills
itself instead, but this is why the backtrace looks the way it does.

So now we all know that!

It would actually be nice if we could think of a clever way to prune
all the stack frames starting with tolower and above, since they are
irrelevant..

> It would actually be nice if we could think of a clever way to prune
> all the stack frames starting with tolower and above

Why do all that when we can just put a 'ret' inside
`__assertion_failed()`?
see patch below for a (questionable) way of doing so:

diff --git a/Libraries/LibC/assert.cpp b/Libraries/LibC/assert.cpp
index ae4ddfe7d..23e51e80c 100644
--- a/Libraries/LibC/assert.cpp
+++ b/Libraries/LibC/assert.cpp
@@ -32,11 +32,17 @@
 extern "C" {

 #ifdef DEBUG
+extern void* __assert_hide_fn_value(void*);
+
 void __assertion_failed(const char* msg)
 {
     dbgprintf("USERSPACE(%d) ASSERTION FAILED: %s\n", getpid(), msg);
     fprintf(stderr, "ASSERTION FAILED: %s\n", msg);
-    abort();
+    ((void (*)())__assert_hide_fn_value((void*)&abort))();
+    asm volatile(".byte 0xc3" ::
+                     : "eax");
+    for (;;)
+        ;
 }
 #endif
 }

where `__assert_hide_fn_value()` just...hides the actual value of
&abort,
so that it's seen as an indirect fnptr call, and not a call to abort().

And since putting a return inside a noreturn function makes a warning
(and that's obviously bad!), we'll sneak a ret there instead!

That only fixes up one stack frame, I'm saying it'd be nice if the asserting
function moved to the top of the trace.

Also, I think your approach could be simplified:

    asm volatile("call abort; ret");

No need for the awkward function pointer trickery. :)

> That only fixes up one stack frame

The only real problem with that trace (to me) is that it lies about
'tolower()', the rest of it seems reasonable to have.
 
> it'd be nice if the asserting
> function moved to the top of the trace.

That'd be nicer to see, yeah

> asm volatile("call abort; ret");

I was pretty sure that any mention of 'ret' in a noreturn function
caused a warning, but I'm probably wrong :shrug:

> No need for the awkward function pointer trickery

But it's fun!

Random idea... what about removing abort from the assert completely?

Instead serenity could use an unused software interrupt code to
trigger the assert crash.
Then you can just inline the assert everywhere.

Windows does something similar to allow processes to force terminate
by just running a single instruction.
https://docs.microsoft.com/en-us/cpp/intrinsics/fastfail?view=vs-2019