Unverified Commit 82bf5e84 authored by Vitaly Buka's avatar Vitaly Buka Committed by GitHub
Browse files

[LowerTypeTests] Add debug info to jump table entries (#194493) 

[LowerTypeTests] Add debug info to jump table entries (#192736)
    
When Control Flow Integrity (CFI) is enabled, jump tables are used to
redirect indirect calls. Previously, these jump table entries lacked
debug information, making it difficult for profilers and debuggers to
attribute execution time correctly.

Now stack trace, when stopped on jump table entry will looks like this:
```
#0: c::c() (.cfi_jt) at sanitizer/ubsan_interface.h:0:0
#1: __ubsan_check_cfi_icall_jt at sanitizer/ubsan_interface.h:0
```

Following up on previous attempts #192736 and #193670, this PR is
essentially #192736 but with the `(.cfi_jt)` and
`__ubsan_check_cfi_icall_jt`
frames swapped. While the specific order of `__ubsan_check_cfi_icall_jt`
isn't strictly necessary, swapping them helps maintain existing
diagnostics
behavior.
Additionally, the diagnostics must remove `ubsan_interface.h` to allow
for a fallback to printing the module name.
See "Commits" tab for details.
parent a671e795

compiler-rt/lib/ubsan/ubsan_handlers.cpp

+29 −1
Original line number Diff line number Diff line
@@ -855,6 +855,32 @@ void __ubsan::__ubsan_handle_pointer_overflow_abort(PointerOverflowData *Data, 
  Die();

}



// Returns true if this is an artificial debug location created by the

// LowerTypeTests pass (see createJumpTableDebugInfo in LLVM).

static bool isArtificialStack(const SymbolizedStack *S) {

  static constexpr char kSuffix[] = "ubsan_interface.h";

  if (!S || !S->info.function || !S->info.file)

    return false;

  const char *File = S->info.file;

  uptr FileLen = internal_strlen(File);

  uptr SuffixLen = internal_strlen(kSuffix);

  if (FileLen < SuffixLen)

    return false;

  return internal_strcmp(File + FileLen - SuffixLen, kSuffix) == 0;

}



// Stripping the file name from artificial frames forces the UBSan Diag

// to fall back to module names. This preserves the original behavior

// of showing the module, while still allowing the symbolizer

// to include the helpful (.cfi_jt) function suffix.

static SymbolizedStack *removeArtificialFiles(SymbolizedStack *FS) {

  for (SymbolizedStack *S = FS; S; S = S->next) {

    if (isArtificialStack(S))

      S->info.file = nullptr;

  }

  return FS;

}



static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function,

                              ReportOptions Opts) {

  if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall)
@@ -875,7 +901,9 @@ static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function, 
       "control flow integrity check for type %0 failed during %1")

      << Data->Type << CheckKindStr;



  SymbolizedStackHolder FLoc(getSymbolizedLocation(Function));

  SymbolizedStackHolder FLoc(

      removeArtificialFiles(getSymbolizedLocation(Function)));



  const char *FName = FLoc.get()->info.function;

  if (!FName)

    FName = "(unknown)";

compiler-rt/test/cfi/cross-dso/icall/diag.cpp

+1 −1
Original line number Diff line number Diff line
@@ -113,7 +113,7 @@ int main(int argc, char *argv[]) { 
  void *p;

  if (argv[1][0] == 'i') {

    // ICALL-DIAG: runtime error: control flow integrity check for type 'void *(int)' failed during indirect function call

    // ICALL-DIAG-NEXT: dynamic.so+0x{{[[:xdigit:]]+}}): note: create_B() defined here

    // ICALL-DIAG-NEXT: dynamic.so+0x{{[[:xdigit:]]+}}): note: create_B() {{(\(.cfi_jt\) )?}}defined here

    // ICALL-NODIAG-NOT: runtime error: control flow integrity check {{.*}} during indirect function call

    p = ((void *(*)(int))create_B)(42);

  } else {

compiler-rt/test/cfi/cross-dso/icall/icall-from-dso.cpp

+1 −1
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ void f() { 
  // CHECK: =2=

  fprintf(stderr, "=2=\n");

  // CFI-DIAG: runtime error: control flow integrity check for type 'void (int)' failed during indirect function call

  // CFI-DIAG-NEXT: ({{.*}}exe+0x{{[[:xdigit:]]+}}): note: g() defined here

  // CFI-DIAG-NEXT: ({{.*}}exe+0x{{[[:xdigit:]]+}}): note: g() {{(\(.cfi_jt\) )?}}defined here

  ((void (*)(int))g)(42); // UB here

  // CHECK-DIAG: =3=

  // CHECK-NOT: =3=

compiler-rt/test/cfi/cross-dso/icall/icall.cpp

+1 −1
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ int main() { 
  // CHECK: =2=

  fprintf(stderr, "=2=\n");

  // CFI-DIAG: runtime error: control flow integrity check for type 'void (int)' failed during indirect function call

  // CFI-DIAG-NEXT: ({{.*}}dynamic.so+0x{{[[:xdigit:]]+}}): note: f() defined here

  // CFI-DIAG-NEXT: ({{.*}}dynamic.so+0x{{[[:xdigit:]]+}}): note: f() {{(\(.cfi_jt\) )?}}defined here

  ((void (*)(int))f)(42); // UB here

  // CHECK-DIAG: =3=

  // CHECK-NOT: =3=

compiler-rt/test/cfi/mfcall.cpp

+2 −2
Original line number Diff line number Diff line
@@ -63,12 +63,12 @@ int main(int argc, char **argv) { 
  switch (argv[1][0]) {

    case 'a':

      // A: runtime error: control flow integrity check for type 'int (S::*)()' failed during non-virtual pointer to member function call

      // A: note: S::f1() defined here

      // A: note: S::f1() {{(\(.cfi_jt\) )?}}defined here

      (s.*bitcast<S_int>(&S::f1))();

      break;

    case 'b':

      // B: runtime error: control flow integrity check for type 'int (T::*)()' failed during non-virtual pointer to member function call

      // B: note: S::f2() defined here

      // B: note: S::f2() {{(\(.cfi_jt\) )?}}defined here

      (t.*bitcast<T_int>(&S::f2))();

      break;

    case 'c':