From fef0574d08589ea04d74b45d6d90d34a25bccbf1 Mon Sep 17 00:00:00 2001
From: Kyle Huey <khuey@kylehuey.com>
Date: Sun, 26 May 2024 14:50:47 -0700
Subject: [PATCH] review comments tranche 1

---
 CMakeLists.txt               | 10 +++++-----
 src/PerfCounters.cc          |  8 ++++----
 src/PerfCounters.h           |  4 ++--
 src/bpf/async_event_filter.c |  4 ++--
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 20185584799..f780fda8a6c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -237,15 +237,15 @@ if(NOT ANDROID)
   add_definitions(-DZSTD=1)
 endif()
 
-option(usebpf "Enable bpf acceleration")
+option(bpf "Enable bpf acceleration")
 
-if(usebpf)
-  add_definitions(-DUSEBPF=1)
+if(bpf)
+  add_definitions(-DBPF=1)
   set(REQUIRED_LIBS
     ${REQUIRED_LIBS}
     libbpf
   )
-endif(usebpf)
+endif(bpf)
 
 foreach(required_lib ${REQUIRED_LIBS})
   string(TOUPPER ${required_lib} PKG)
@@ -701,7 +701,7 @@ post_build_executable(rr)
 set(RR_BIN rr)
 add_dependencies(rr Generated)
 
-if(usebpf)
+if(bpf)
   add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/share/rr/async_event_filter.o
     DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/src/bpf/async_event_filter.c
     COMMAND clang -g -target bpf -Wall -O2 -c ${CMAKE_CURRENT_SOURCE_DIR}/src/bpf/async_event_filter.c -o ${CMAKE_CURRENT_BINARY_DIR}/share/rr/async_event_filter.o)
diff --git a/src/PerfCounters.cc b/src/PerfCounters.cc
index 67687ba608c..2c003e67b2e 100644
--- a/src/PerfCounters.cc
+++ b/src/PerfCounters.cc
@@ -17,7 +17,7 @@
 #include <unistd.h>
 #include <time.h>
 
-#ifdef USEBPF
+#ifdef BPF
 #include <bpf/libbpf.h>
 #include <linux/hw_breakpoint.h>
 #endif
@@ -1125,7 +1125,7 @@ Ticks PerfCounters::read_ticks(Task* t, Error* error) {
   return ret;
 }
 
-#ifdef USEBPF
+#ifdef BPF
 bool PerfCounters::accelerate_async_signal(const Registers& regs) {
   static int initialized;
   static struct perf_event_attr attr;
@@ -1158,7 +1158,7 @@ bool PerfCounters::accelerate_async_signal(const Registers& regs) {
       if (bpf_object__load(obj) < 0) {
         return false;
       }
-      int bpf_map_fd = ScopedFd(bpf_object__find_map_fd_by_name(obj, "registers"));
+      int bpf_map_fd = bpf_object__find_map_fd_by_name(obj, "registers");
       if (bpf_map_fd < 0) {
         return false;
       }
@@ -1174,7 +1174,7 @@ bool PerfCounters::accelerate_async_signal(const Registers& regs) {
       bpf_regs = (struct user_regs_struct*)
         mmap(NULL, 4096, PROT_READ | PROT_WRITE,
              MAP_SHARED, bpf_map_fd, 0);
-      if (!bpf_regs) {
+      if (bpf_regs == MAP_FAILED) {
         return false;
       }
 
diff --git a/src/PerfCounters.h b/src/PerfCounters.h
index cc24b8549f9..179e2bb42f0 100644
--- a/src/PerfCounters.h
+++ b/src/PerfCounters.h
@@ -179,10 +179,10 @@ class PerfCounters {
   /**
    * Try to use BPF to accelerate async signal processing
    */
-#ifdef USEBPF
+#ifdef BPF
   bool accelerate_async_signal(const Registers& regs);
 #else
-  inline bool accelerate_async_signal(const Registers&) {
+  bool accelerate_async_signal(const Registers&) {
     return false;
   }
 #endif
diff --git a/src/bpf/async_event_filter.c b/src/bpf/async_event_filter.c
index bd5d5cbb3c0..611042744b9 100644
--- a/src/bpf/async_event_filter.c
+++ b/src/bpf/async_event_filter.c
@@ -42,9 +42,9 @@ int match_registers(struct bpf_perf_event_data* event) {
     CHECK_REG(rdx)
     CHECK_REG(rsi)
     CHECK_REG(rdi)
-    CHECK_REG(rip)
+    CHECK_REG(rsp)
 
     return 1;
 }
 
-char _license[] SEC("license") = "GPL";
+char _license[] SEC("license") = "Dual MIT/GPL";