[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

Fri Sep 20 11:00:54 PDT 2019

ychen created this revision.
ychen added reviewers: rnk, chandlerc, zturner, sepavloff.
Herald added subscribers: llvm-commits, seiya, rupprecht, dexonsmith, steven_wu, hiraditya, mehdi_amini.
Herald added a project: LLVM.

`exit` does cleanup including calling class destructor whereas `abort`
does not do any cleanup. In multithreading environment such as ThinLTO or JIT,
threads may share states which mostly are ManagedStatic<>. If faulting thread
tearing down a class when another thread is using it, there are chances of
memory corruption. This is bad 1. It will stop error reporting like pretty
stack printer; 2. The memory corruption is distracting and nondeterministic in
terms of error message, and corruption type (depending one the timing, it
could be double free, heap free after use, etc.).

The cons of doing this are functions registered with atexit will not get
called. However I haven't see any such uses in the codebase. The
alternative is to use `install_fatal_error_handler` which will be called
in `report_fatal_error`.

There are many tests need to be fixed due to this change since `not`
tool (used in RUN lines) treat normal exit and crash differently. For
this patch, I have not fix up all the such tests. I will do it later if
reviewers agree this is the right thing to do.

Fixes PR35547.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67847

Files:
  llvm/lib/Support/ErrorHandling.cpp
  llvm/test/Bitcode/function-default-address-spaces.ll
  llvm/test/MachineVerifier/test_g_inttoptr.mir
  llvm/test/tools/llvm-lto2/X86/pipeline.ll
  llvm/test/tools/llvm-objdump/relocations-elf.test
  llvm/test/tools/llvm-readobj/arm64-many-epilogs.s
  llvm/test/tools/llvm-readobj/arm64-win-error2.s


Index: llvm/test/tools/llvm-readobj/arm64-win-error2.s
===================================================================

--- llvm/test/tools/llvm-readobj/arm64-win-error2.s
+++ llvm/test/tools/llvm-readobj/arm64-win-error2.s
@@ -6,7 +6,7 @@
 
 // REQUIRES: aarch64-registered-target
 // RUN: llvm-mc -filetype=obj -triple aarch64-windows %s -o - \
-// RUN:   | not llvm-readobj --unwind - 2>&1 | FileCheck %s
+// RUN:   | not --crash llvm-readobj --unwind - 2>&1 | FileCheck %s
 
 // CHECK: LLVM ERROR: Malformed unwind data
 
Index: llvm/test/tools/llvm-readobj/arm64-many-epilogs.s
===================================================================
--- llvm/test/tools/llvm-readobj/arm64-many-epilogs.s
+++ llvm/test/tools/llvm-readobj/arm64-many-epilogs.s
@@ -1,6 +1,6 @@
 // REQUIRES: aarch64-registered-target
 // RUN: llvm-mc -filetype=obj -triple aarch64-windows %s -o - \
-// RUN:   | not llvm-readobj --unwind - | FileCheck %s
+// RUN:   | not --crash llvm-readobj --unwind - 2>&1 | FileCheck %s
 
 // Older versions of LLVM had a bug where we would accidentally
 // truncate the number of epilogue scopes to a uint8_t; make
Index: llvm/test/tools/llvm-objdump/relocations-elf.test
===================================================================
--- llvm/test/tools/llvm-objdump/relocations-elf.test
+++ llvm/test/tools/llvm-objdump/relocations-elf.test
@@ -74,7 +74,7 @@
 ## Check we report an error if the relocated section identified by the
 ## sh_info field of a relocation section is invalid.
 # RUN: yaml2obj --docnum=2 %s > %t2
-# RUN: not llvm-objdump --reloc %t2 2>&1 | FileCheck %s --check-prefix=ERR
+# RUN: not --crash llvm-objdump --reloc %t2 2>&1 | FileCheck %s --check-prefix=ERR
 # ERR: LLVM ERROR: Invalid data was encountered while parsing the file
 
 --- !ELF
Index: llvm/test/tools/llvm-lto2/X86/pipeline.ll
===================================================================
--- llvm/test/tools/llvm-lto2/X86/pipeline.ll
+++ llvm/test/tools/llvm-lto2/X86/pipeline.ll
@@ -28,13 +28,13 @@
 ; CUSTOM-NEXT: }
 
 ; Check that invalid pipelines are caught as errors.
-; RUN: not llvm-lto2 run %t1.bc -o %t.o \
+; RUN: not --crash llvm-lto2 run %t1.bc -o %t.o \
 ; RUN:  -r %t1.bc,patatino,px -opt-pipeline foogoo 2>&1 | \
 ; RUN:  FileCheck %s --check-prefix=ERR
 
 ; ERR: LLVM ERROR: unable to parse pass pipeline description 'foogoo': unknown pass name 'foogoo'
 
-; RUN: not llvm-lto2 run %t1.bc -o %t.o \
+; RUN: not --crash llvm-lto2 run %t1.bc -o %t.o \
 ; RUN:  -r %t1.bc,patatino,px -aa-pipeline patatino \
 ; RUN:  -opt-pipeline loweratomic 2>&1 | \
 ; RUN:  FileCheck %s --check-prefix=AAERR
Index: llvm/test/MachineVerifier/test_g_inttoptr.mir
===================================================================
--- llvm/test/MachineVerifier/test_g_inttoptr.mir
+++ llvm/test/MachineVerifier/test_g_inttoptr.mir
@@ -1,4 +1,4 @@
-#RUN: not llc -o - -march=arm64 -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+#RUN: not --crash llc -o - -march=arm64 -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
 # REQUIRES: global-isel, aarch64-registered-target
 
 ---
Index: llvm/test/Bitcode/function-default-address-spaces.ll
===================================================================
--- llvm/test/Bitcode/function-default-address-spaces.ll
+++ llvm/test/Bitcode/function-default-address-spaces.ll
@@ -1,6 +1,6 @@
 ; RUN: llvm-as %s  -o - | llvm-dis - | FileCheck %s -check-prefixes CHECK,PROG-AS0
 ; RUN: llvm-as -data-layout "P200" %s  -o - | llvm-dis | FileCheck %s -check-prefixes CHECK,PROG-AS200
-; RUN: not llvm-as -data-layout "P123456789" %s -o /dev/null 2>&1 | FileCheck %s -check-prefix BAD-DATALAYOUT
+; RUN: not --crash llvm-as -data-layout "P123456789" %s -o /dev/null 2>&1 | FileCheck %s -check-prefix BAD-DATALAYOUT
 ; BAD-DATALAYOUT: LLVM ERROR: Invalid address space, must be a 24-bit integer
 
 ; PROG-AS0-NOT: target datalayout
Index: llvm/lib/Support/ErrorHandling.cpp
===================================================================
--- llvm/lib/Support/ErrorHandling.cpp
+++ llvm/lib/Support/ErrorHandling.cpp
@@ -122,7 +122,7 @@
   // files registered with RemoveFileOnSignal.
   sys::RunInterruptHandlers();
 
-  exit(1);
+  abort();
 }
 
 void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67847.221071.patch
Type: text/x-patch
Size: 4309 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190920/fe0e2934/attachment.bin>
