Remove LOG(FATAL) (#3164)

Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: #3163

Co-authored-by: Jordan Rome <jordalgo@fedoraproject.org>

CHANGELOG.md

@@ -28,8 +28,6 @@ and this project adheres to
   - [#3047](https://github.com/bpftrace/bpftrace/pull/3047)
 - Reproducible Builds: Do not store timestamps in gzip header
   - [#3096](https://github.com/bpftrace/bpftrace/pull/3096)
-- Replace instances of std::runtime_error with LOG(FATAL)
-  - [#3091](https://github.com/bpftrace/bpftrace/pull/3091)
 - Parse DWARF using liblldb instead of libdw
   - [#3042](https://github.com/bpftrace/bpftrace/pull/3042)
 - Replace native 'bpf_get_stackid'

docs/coding_guidelines.md

@@ -29,10 +29,8 @@ Appropriate language features for this include:
 * `int`
 * `bool`
 
-If an error is **not** recoverable, prefer using `LOG(FATAL)` or `LOG(ERROR)` and `exit(1)` if there are multiple, separate errors. Broadly
-speaking, if you want to immediately terminate and show a message to the
-user, use `LOG(FATAL)`. Exceptions **should not** be used for recoverable
-errors.
+If an error is **not** recoverable, prefer throwing `FatalUserException`.
+Exceptions **should not** be used for recoverable errors.
 
 ### Examples
 
@@ -94,9 +92,14 @@ struct Bar {
 
 Below are details about when to use each kind of log level:
 
-- `DEBUG`: log info regardless of log level; like using stdout (comes with file and line number)
+- `DEBUG`: log info regardless of log level; like using stdout (comes with file
+and line number)
 - `V1`: log info only if verbose logging is enabled (-v)
-- `WARNING`: log info that might affect bpftrace behavior or output but allows the run to continue; like using stderr
-- `ERROR`: log info to indicate that the user did something invalid, which will (eventually) cause bpftrace to exit (via `exit(1)` or `LOG(FATAL)`); this should get used if there are multiple errors, otherwise use `FATAL `
-- `FATAL`: similar to `ERROR` but exits immediately after logging
-- `BUG`: exit and log info to indicate that there is an internal/unexpected issue (not caused by invalid user program code or CLI use)
+- `WARNING`: log info that might affect bpftrace behavior or output but allows
+the run to continue; like using stderr
+- `ERROR`: log info to indicate that the user did something invalid, which will
+(eventually) cause bpftrace to exit (via `exit(1)`); this should primarily get
+used in main.cpp after catching `FatalUserException` from deeper parts of the
+code base
+- `BUG`: abort and log info to indicate that there is an internal/unexpected
+issue (not caused by invalid user program code or CLI use)

src/arch/loongarch64.cpp

@@ -1,4 +1,5 @@
 #include "arch.h"
+#include "utils.h"
 
 #include <algorithm>
 #include <array>
@@ -149,7 +150,8 @@ std::string name()
 
 std::vector<std::string> invalid_watchpoint_modes()
 {
-  LOG(FATAL) << "Watchpoints are not supported on this architecture";
+  throw FatalUserException(
+      "Watchpoints are not supported on this architecture");
 }
 
 int get_kernel_ptr_width()

src/arch/mips64.cpp

@@ -1,4 +1,5 @@
 #include "arch.h"
+#include "utils.h"
 
 #include <algorithm>
 #include <array>
@@ -161,7 +162,8 @@ std::string name()
 
 std::vector<std::string> invalid_watchpoint_modes()
 {
-  LOG(FATAL) << "Watchpoints are not supported on this architecture";
+  throw FatalUserException(
+      "Watchpoints are not supported on this architecture");
 }
 
 int get_kernel_ptr_width()

src/arch/riscv64.cpp

@@ -1,4 +1,5 @@
 #include "arch.h"
+#include "utils.h"
 
 #include <algorithm>
 #include <array>
@@ -103,7 +104,8 @@ std::string name()
 
 std::vector<std::string> invalid_watchpoint_modes()
 {
-  LOG(FATAL) << "Watchpoints are not supported on this architecture";
+  throw FatalUserException(
+      "Watchpoints are not supported on this architecture");
 }
 
 int get_kernel_ptr_width()

src/arch/s390.cpp

@@ -1,4 +1,5 @@
 #include "arch.h"
+#include "utils.h"
 
 #include <algorithm>
 #include <array>
@@ -93,7 +94,8 @@ std::string name()
 
 std::vector<std::string> invalid_watchpoint_modes()
 {
-  LOG(FATAL) << "Watchpoints are not supported on this architecture";
+  throw FatalUserException(
+      "Watchpoints are not supported on this architecture");
 }
 
 int get_kernel_ptr_width()

src/ast/irbuilderbpf.cpp

@@ -253,8 +253,7 @@ llvm::Type *IRBuilderBPF::GetType(const SizedType &stype)
         ty = getInt8Ty();
         break;
       default:
-        LOG(FATAL) << stype.GetSize()
-                   << " is not a valid type size for GetType";
+        LOG(BUG) << stype.GetSize() << " is not a valid type size for GetType";
     }
   }
   return ty;
@@ -666,8 +665,8 @@ Value *IRBuilderBPF::CreateUSDTReadArgument(Value *ctx,
     int offset = 0;
     offset = arch::offset(argument->base_register_name);
     if (offset < 0) {
-      LOG(FATAL) << "offset for register " << argument->base_register_name
-                 << " not known";
+      LOG(BUG) << "offset for register " << argument->base_register_name
+               << " not known";
     }
 
     // bpftrace's args are internally represented as 64 bit integers. However,
@@ -682,8 +681,8 @@ Value *IRBuilderBPF::CreateUSDTReadArgument(Value *ctx,
     if (argument->valid & BCC_USDT_ARGUMENT_INDEX_REGISTER_NAME) {
       int ioffset = arch::offset(argument->index_register_name);
       if (ioffset < 0) {
-        LOG(FATAL) << "offset for register " << argument->index_register_name
-                   << " not known";
+        LOG(BUG) << "offset for register " << argument->index_register_name
+                 << " not known";
       }
       index_offset = CreateGEP(getInt8Ty(),
                                ctx,

src/ast/passes/codegen_llvm.cpp

@@ -61,7 +61,8 @@ CodegenLLVM::CodegenLLVM(Node *root, BPFtrace &bpftrace)
   std::string error_str;
   auto target = llvm::TargetRegistry::lookupTarget(LLVMTargetTriple, error_str);
   if (!target)
-    LOG(FATAL) << "Could not find bpf llvm target, does your llvm support it?";
+    throw FatalUserException(
+        "Could not find bpf llvm target, does your llvm support it?");
 
   target_machine_.reset(
       target->createTargetMachine(LLVMTargetTriple,
@@ -837,7 +838,7 @@ void CodegenLLVM::visit(Call &call)
     auto name = bpftrace_.get_string_literal(call.vargs->at(0));
     addr = bpftrace_.resolve_kname(name);
     if (!addr)
-      LOG(FATAL) << "Failed to resolve kernel symbol: " << name;
+      throw FatalUserException("Failed to resolve kernel symbol: " + name);
     expr_ = b_.getInt64(addr);
   } else if (call.func == "uaddr") {
     auto name = bpftrace_.get_string_literal(call.vargs->at(0));
@@ -846,8 +847,8 @@ void CodegenLLVM::visit(Call &call)
                                       &sym,
                                       current_attach_point_->target);
     if (err < 0 || sym.address == 0)
-      LOG(FATAL) << "Could not resolve symbol: "
-                 << current_attach_point_->target << ":" << name;
+      throw FatalUserException("Could not resolve symbol: " +
+                               current_attach_point_->target + ":" + name);
     expr_ = b_.getInt64(sym.address);
   } else if (call.func == "cgroupid") {
     uint64_t cgroupid;
@@ -999,7 +1000,8 @@ void CodegenLLVM::visit(Call &call)
     Value *octet;
     auto ret = inet_pton(af_type, addr.c_str(), &dst);
     if (ret != 1) {
-      LOG(FATAL) << "inet_pton() call returns " << ret;
+      throw FatalUserException("inet_pton() call returns " +
+                               std::to_string(ret));
     }
     for (int i = 0; i < addr_size; i++) {
       octet = b_.getInt8(dst[i]);
@@ -1014,7 +1016,7 @@ void CodegenLLVM::visit(Call &call)
     auto reg_name = bpftrace_.get_string_literal(call.vargs->at(0));
     int offset = arch::offset(reg_name);
     if (offset == -1) {
-      LOG(FATAL) << "negative offset on reg() call";
+      throw FatalUserException("negative offset on reg() call");
     }
 
     expr_ = b_.CreateRegisterRead(ctx_, offset, call.func + "_" + reg_name);
@@ -2697,13 +2699,13 @@ void CodegenLLVM::visit(Probe &probe)
       uint64_t max_bpf_progs = bpftrace_.config_.get(
           ConfigKeyInt::max_bpf_progs);
       if (probe_count_ > max_bpf_progs) {
-        LOG(FATAL) << "Your program is trying to generate more than "
-                   << std::to_string(probe_count_)
-                   << " BPF programs, which exceeds the current limit of "
-                   << std::to_string(max_bpf_progs)
-                   << ".\nYou can increase the limit through the "
-                      "BPFTRACE_MAX_BPF_PROGS "
-                      "environment variable.";
+        throw FatalUserException(
+            "Your program is trying to generate more than " +
+            std::to_string(probe_count_) +
+            " BPF programs, which exceeds the current limit of " +
+            std::to_string(max_bpf_progs) +
+            ".\nYou can increase the limit through the BPFTRACE_MAX_BPF_PROGS "
+            "environment variable.");
       }
 
       tracepoint_struct_ = "";