From 9b6718ac995eec6ee3b8a2f8db5efb862f283b5d Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Thu, 30 May 2024 15:11:45 -0700 Subject: [PATCH] Improve backtraces We're now able to rewind the instruction pointer in x86 backtraces. This helps ensure addr2line cannot print information about unrelated adjacent code. I've restored -fno-schedule-insns2 in most cases because it really does cause unpredictable breakage for backtraces. --- build/definitions.mk | 1 + examples/crashreport.c | 19 +++++---- libc/integral/c.inc | 7 +--- libc/intrin/describebacktrace.c | 50 +++++++++++++++++++----- libc/intrin/describebacktrace.internal.h | 2 +- libc/intrin/iscall.c | 37 ++++++++++++++++++ libc/intrin/iscall.internal.h | 10 +++++ libc/log/backtrace2.c | 4 ++ libc/log/backtrace3.c | 3 ++ tool/cosmocc/bin/cosmocc | 6 ++- tool/cosmocc/bin/cosmocross | 3 ++ 11 files changed, 118 insertions(+), 24 deletions(-) create mode 100644 libc/intrin/iscall.c create mode 100644 libc/intrin/iscall.internal.h diff --git a/build/definitions.mk b/build/definitions.mk index 0ff0a2150..2683ffe33 100644 --- a/build/definitions.mk +++ b/build/definitions.mk @@ -60,6 +60,7 @@ TMPSAFE = $(TMPDIR)/ endif BACKTRACES = \ + -fno-schedule-insns2 \ -fno-optimize-sibling-calls \ -mno-omit-leaf-frame-pointer diff --git a/examples/crashreport.c b/examples/crashreport.c index 7ea9a5e81..4e41ebf7f 100644 --- a/examples/crashreport.c +++ b/examples/crashreport.c @@ -7,6 +7,7 @@ │ • http://creativecommons.org/publicdomain/zero/1.0/ │ ╚─────────────────────────────────────────────────────────────────*/ #endif +#include "libc/calls/calls.h" #include "libc/intrin/kprintf.h" #include "libc/math.h" #include "libc/runtime/runtime.h" @@ -26,6 +27,13 @@ * o//examples/crashreport.com */ +int Divide(int x, int y) { + volatile int z = 0; // force creation of stack frame + return x / y + z; +} + +int (*pDivide)(int, int) = Divide; + dontubsan int main(int argc, char *argv[]) { kprintf("----------------\n"); kprintf(" THIS IS A TEST \n"); @@ -34,12 +42,7 @@ dontubsan int main(int argc, char *argv[]) { ShowCrashReports(); - volatile double a = 0; - volatile double b = 23; - volatile double c = exp(b) / a; - (void)c; - - volatile int x = 0; - volatile int y = 1 / x; - return y; + pDivide(1, 0); + pDivide(2, 0); + pDivide(3, 0); } diff --git a/libc/integral/c.inc b/libc/integral/c.inc index 37bd92b86..301cca916 100644 --- a/libc/integral/c.inc +++ b/libc/integral/c.inc @@ -539,16 +539,13 @@ typedef struct { #ifdef __x86_64__ #define notpossible \ do { \ - __asm__("nop\n\t" \ - "ud2\n\t" \ - "nop"); \ + __asm__("ud2"); \ __builtin_unreachable(); \ } while (0) #elif defined(__aarch64__) #define notpossible \ do { \ - __asm__("udf\t#0\n\t" \ - "nop"); \ + __asm__("udf\t#0"); \ __builtin_unreachable(); \ } while (0) #else diff --git a/libc/intrin/describebacktrace.c b/libc/intrin/describebacktrace.c index 93107f884..d6bd8f8e0 100644 --- a/libc/intrin/describebacktrace.c +++ b/libc/intrin/describebacktrace.c @@ -17,34 +17,66 @@ │ PERFORMANCE OF THIS SOFTWARE. │ ╚─────────────────────────────────────────────────────────────────────────────*/ #include "libc/intrin/describebacktrace.internal.h" +#include "libc/intrin/iscall.internal.h" #include "libc/intrin/kprintf.h" #include "libc/intrin/weaken.h" -#include "libc/log/libfatal.internal.h" #include "libc/nexgen32e/stackframe.h" #define N 160 +static bool IsDangerous(const void *ptr) { + if (_weaken(kisdangerous)) + return _weaken(kisdangerous)(ptr); + return false; +} + +static char *FormatHex(char *p, unsigned long x) { + int k = x ? (__builtin_clzl(x) ^ 63) + 1 : 1; + k = (k + 3) & -4; + while (k > 0) + *p++ = "0123456789abcdef"[(x >> (k -= 4)) & 15]; + *p = '\0'; + return p; +} + dontinstrument const char *(DescribeBacktrace)(char buf[N], - struct StackFrame *fr) { + const struct StackFrame *fr) { char *p = buf; char *pe = p + N; bool gotsome = false; while (fr) { - if (_weaken(kisdangerous) && _weaken(kisdangerous)(fr)) { + if (IsDangerous(fr)) { + if (p + 1 + 1 + 1 < pe) { + if (gotsome) + *p++ = ' '; + *p = '!'; + if (p + 16 + 1 < pe) { + *p++ = ' '; + p = FormatHex(p, (long)fr); + } + } break; } - if (p + 16 + 1 + 1 <= pe) { - if (gotsome) { + if (p + 16 + 1 < pe) { + unsigned char *ip = (unsigned char *)fr->addr; +#ifdef __x86_64__ + // x86 advances the progrem counter before an instruction + // begins executing. return addresses in backtraces shall + // point to code after the call, which means addr2line is + // going to print unrelated code unless we fixup the addr + if (!IsDangerous(ip)) + ip -= __is_call(ip); +#endif + if (gotsome) *p++ = ' '; - } else { + else gotsome = true; - } - p = __hexcpy(p, fr->addr); + p = FormatHex(p, (long)ip); } else { break; } fr = fr->next; } - *p = 0; + *p = '\0'; return buf; } diff --git a/libc/intrin/describebacktrace.internal.h b/libc/intrin/describebacktrace.internal.h index 407057578..9c116cd25 100644 --- a/libc/intrin/describebacktrace.internal.h +++ b/libc/intrin/describebacktrace.internal.h @@ -4,7 +4,7 @@ #include "libc/nexgen32e/stackframe.h" COSMOPOLITAN_C_START_ -const char *DescribeBacktrace(char[160], struct StackFrame *) libcesque; +const char *DescribeBacktrace(char[160], const struct StackFrame *) libcesque; #define DescribeBacktrace(x) DescribeBacktrace(alloca(160), x) COSMOPOLITAN_C_END_ diff --git a/libc/intrin/iscall.c b/libc/intrin/iscall.c new file mode 100644 index 000000000..7907d8525 --- /dev/null +++ b/libc/intrin/iscall.c @@ -0,0 +1,37 @@ +/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│ +│ vi: set et ft=c ts=2 sts=2 sw=2 fenc=utf-8 :vi │ +╞══════════════════════════════════════════════════════════════════════════════╡ +│ Copyright 2024 Justine Alexandra Roberts Tunney │ +│ │ +│ Permission to use, copy, modify, and/or distribute this software for │ +│ any purpose with or without fee is hereby granted, provided that the │ +│ above copyright notice and this permission notice appear in all copies. │ +│ │ +│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL │ +│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED │ +│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE │ +│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL │ +│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR │ +│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER │ +│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ +│ PERFORMANCE OF THIS SOFTWARE. │ +╚─────────────────────────────────────────────────────────────────────────────*/ +#include "libc/intrin/iscall.internal.h" + +// returns true if `p` is preceded by x86 call instruction +// this is actually impossible to do but we'll do our best +dontinstrument int __is_call(const unsigned char *p) { + if (p[-5] == 0xe8) + return 5; // call Jvds + if (p[-2] == 0xff && (p[-1] & 070) == 020) + return 2; // call %reg + if (p[-4] == 0xff && (p[-3] & 070) == 020) + return 4; // call disp8(%reg,%reg) + if (p[-3] == 0xff && (p[-2] & 070) == 020) + return 3; // call disp8(%reg) + if (p[-7] == 0xff && (p[-6] & 070) == 020) + return 7; // call disp32(%reg,%reg) + if (p[-6] == 0xff && (p[-5] & 070) == 020) + return 6; // call disp32(%reg) + return 0; +} diff --git a/libc/intrin/iscall.internal.h b/libc/intrin/iscall.internal.h new file mode 100644 index 000000000..d290f2386 --- /dev/null +++ b/libc/intrin/iscall.internal.h @@ -0,0 +1,10 @@ +#ifndef COSMOPOLITAN_LIBC_INTRIN_ISCALL_H_ +#define COSMOPOLITAN_LIBC_INTRIN_ISCALL_H_ +COSMOPOLITAN_C_START_ + +// returns true if `p` is preceded by x86 call instruction +// this is actually impossible to do but we'll do our best +int __is_call(const unsigned char *); + +COSMOPOLITAN_C_END_ +#endif /* COSMOPOLITAN_LIBC_INTRIN_ISCALL_H_ */ diff --git a/libc/log/backtrace2.c b/libc/log/backtrace2.c index a68d90f05..b3eeac606 100644 --- a/libc/log/backtrace2.c +++ b/libc/log/backtrace2.c @@ -24,6 +24,8 @@ #include "libc/dce.h" #include "libc/errno.h" #include "libc/fmt/itoa.h" +#include "libc/intrin/describebacktrace.internal.h" +#include "libc/intrin/iscall.internal.h" #include "libc/intrin/kprintf.h" #include "libc/intrin/promises.internal.h" #include "libc/intrin/weaken.h" @@ -112,6 +114,8 @@ static int PrintBacktraceUsingAddr2line(int fd, const struct StackFrame *bp) { --gi; } while ((addr = garbage->p[gi].ret) == (uintptr_t)_weaken(__gc)); } + if (!kisdangerous((const unsigned char *)addr)) + addr -= __is_call((const unsigned char *)addr); #endif argv[i++] = buf + j; buf[j++] = '0'; diff --git a/libc/log/backtrace3.c b/libc/log/backtrace3.c index b82e12a4a..9b35ebd3d 100644 --- a/libc/log/backtrace3.c +++ b/libc/log/backtrace3.c @@ -20,6 +20,7 @@ #include "libc/calls/calls.h" #include "libc/cosmo.h" #include "libc/fmt/itoa.h" +#include "libc/intrin/iscall.internal.h" #include "libc/intrin/kprintf.h" #include "libc/intrin/weaken.h" #include "libc/log/backtrace.internal.h" @@ -76,6 +77,8 @@ dontinstrument dontasan int PrintBacktraceUsingSymbols( --gi; } while ((addr = garbage->p[gi].ret) == (intptr_t)_weaken(__gc)); } + if (!kisdangerous((const unsigned char *)addr)) + addr -= __is_call((const unsigned char *)addr); #endif if (addr) { if ((symbol = __get_symbol(st, addr)) != -1) { diff --git a/tool/cosmocc/bin/cosmocc b/tool/cosmocc/bin/cosmocc index 091d0d822..a6f350c2c 100755 --- a/tool/cosmocc/bin/cosmocc +++ b/tool/cosmocc/bin/cosmocc @@ -162,7 +162,7 @@ for x; do # tail calls — may opt not to create an entry in this list. As a # result, stack traces are always meaningful, even without debug # information." - x="-momit-leaf-frame-pointer -foptimize-sibling-calls" + x="-momit-leaf-frame-pointer -foptimize-sibling-calls -fschedule-insns2" elif [ x"$x" = x"-r" ] || [ x"$x" = x"-S" ] || [ x"$x" = x"-pie" ] || @@ -241,9 +241,13 @@ CFLAGS="-fportcosmo -fno-dwarf2-cfi-asm -fno-unwind-tables -fno-asynchronous-unw LDFLAGS="-static -nostdlib -no-pie -fuse-ld=bfd -Wl,-z,noexecstack -Wl,-z,norelro -Wl,--gc-sections" PRECIOUS="-fno-omit-frame-pointer" +# these features screw with backtraces so avoid them if [ x"$OPT" != x"-Os" ] && [ x"$MODE" != x"tiny" ]; then CFLAGS="$CFLAGS -fno-optimize-sibling-calls -mno-omit-leaf-frame-pointer" fi +if [ x"$OPT" != x"-O3" ]; then + CFLAGS="$CFLAGS -fno-schedule-insns2" +fi CC_X86_64="$BIN/x86_64-linux-cosmo-gcc" CC_AARCH64="$BIN/aarch64-linux-cosmo-gcc" diff --git a/tool/cosmocc/bin/cosmocross b/tool/cosmocc/bin/cosmocross index d8a3f58c1..644c109c9 100755 --- a/tool/cosmocc/bin/cosmocross +++ b/tool/cosmocc/bin/cosmocross @@ -200,6 +200,9 @@ if [ x"$OPT" != x"-Os" ] && # $OPT != "-Os" [ x"$MODE" != x"${MODE%tiny}" ]; then # endswith($MODE, "tiny") CFLAGS="$CFLAGS -fno-optimize-sibling-calls -mno-omit-leaf-frame-pointer" fi +if [ x"$OPT" != x"-O3" ]; then + CFLAGS="$CFLAGS -fno-schedule-insns2" +fi if [ $INTENT = cpp ]; then set -- "$CC" $PLATFORM $CPPFLAGS "$@"