Probe inlined function call

Use the DebugInfo (DWARF) to locate every inlined instances of a
uprobe function and attach to all of them.
This behaviour is controled by the `probe_inline` config key.
Note that using this config disables access to the arguments, because we
cannot guarantee inlined function instances follow the System V ABI.

Also attach to uprobe functions after their prologue, once the stack
frame has been set, which makes collecting stack traces more reliable.
This behaviour is the new default for all binaries with DWARF.

CHANGELOG.md

@@ -34,6 +34,8 @@ and this project adheres to
   - [#3060](https://github.com/bpftrace/bpftrace/pull/3060)
 - Disable func builtin for kretprobes and uretprobes when `get_func_ip` feature is not available
   - [#2645](https://github.com/bpftrace/bpftrace/pull/2645)
+- Attach probe after function prologue and probe inlined functions
+  - [#3095](https://github.com/bpftrace/bpftrace/pull/3095)
 #### Deprecated
 #### Removed
 #### Fixed

man/adoc/bpftrace.adoc

@@ -1102,12 +1102,6 @@ For string arguments use the `str()` call to retrieve the value.
 | n/a
 | Value returned by the function being traced (kretprobe, uretprobe, kretfunc)
 
-| `sarg0`, `sarg1`, `...sargn`
-| int64
-| n/a
-| n/a
-| nth stack value of the function being traced (kprobes, uprobes)
-
 | tid
 | uint64
 | 4.2
@@ -1619,7 +1613,9 @@ This buffer can be printed in the canonical string format using the `%s` format
 
 ----
 kprobe:arp_create {
-  printf("SRC %s, DST %s\n", macaddr(sarg0), macaddr(sarg1));
+  $stack_arg0 = *(uint8*)(reg("sp") + 8);
+  $stack_arg1 = *(uint8*)(reg("sp") + 16);
+  printf("SRC %s, DST %s\n", macaddr($stack_arg0), macaddr($stack_arg1));
 }
 
 /*
@@ -2921,7 +2917,7 @@ kprobe:tcp_reset {
 }
 ----
 
-Function arguments are available through the `argN` and `sargN` builtins, for register args and stack args respectively.
+Function arguments are available through the `argN` for register args. Arguments passed on stack are available using the stack pointer, e.g. `$stack_arg0 = *(int64*)reg("sp") + 16`.
 Whether arguments passed on stack or in a register depends on the architecture and the number or arguments used, e.g. on x86_64 the first 6 non-floating point arguments are passed in registers and all following arguments are passed on the stack.
 Note that floating point arguments are typically passed in special registers which don't count as `argN` arguments which can cause confusion.
 Consider a function with the following signature:

src/ast/passes/semantic_analyser.cpp

@@ -364,6 +364,11 @@ void SemanticAnalyser::visit(Builtin &builtin)
     AddrSpace addrspace = find_addrspace(pt);
     for (auto &attach_point : *probe->attach_points) {
       ProbeType type = probetype(attach_point->provider);
+      if (type == ProbeType::uprobe &&
+          bpftrace_.config_.get(ConfigKeyBool::probe_inline))
+        LOG(ERROR, builtin.loc, err_)
+            << "The " + builtin.ident + " builtin can only be used when "
+            << "the probe_inline config is disabled.";
       if (type != ProbeType::kprobe && type != ProbeType::uprobe &&
           type != ProbeType::usdt && type != ProbeType::rawtracepoint)
         LOG(ERROR, builtin.loc, err_)
@@ -390,6 +395,11 @@ void SemanticAnalyser::visit(Builtin &builtin)
         LOG(ERROR, builtin.loc, err_)
             << "The " + builtin.ident
             << " builtin can only be used with 'kprobes' and 'uprobes' probes";
+      if (type == ProbeType::uprobe &&
+          bpftrace_.config_.get(ConfigKeyBool::probe_inline))
+        LOG(ERROR, builtin.loc, err_)
+            << "The " + builtin.ident + " builtin can only be used when "
+            << "the probe_inline config is disabled.";
       if (is_final_pass() &&
           (attach_point->address != 0 || attach_point->func_offset != 0)) {
         // If sargX values are needed when using an offset, they can be stored
@@ -438,6 +448,12 @@ void SemanticAnalyser::visit(Builtin &builtin)
              "\"probe1,probe2 {args}\" is not.";
     } else if (type == ProbeType::kfunc || type == ProbeType::kretfunc ||
                type == ProbeType::uprobe) {
+      if (type == ProbeType::uprobe &&
+          bpftrace_.config_.get(ConfigKeyBool::probe_inline))
+        LOG(ERROR, builtin.loc, err_)
+            << "The args builtin can only be used when "
+            << "the probe_inline config is disabled.";
+
       auto type_name = probe->args_typename();
       builtin.type = CreateRecord(type_name,
                                   bpftrace_.structs.Lookup(type_name));

src/attached_probe.cpp

@@ -379,7 +379,10 @@ static int sym_address_cb(const char *symname,
 {
   struct symbol *sym = static_cast<struct symbol *>(p);
 
-  if (sym->address >= start && sym->address < (start + size)) {
+  // When size is 0, then [start, start + size) = [start, start) = ø.
+  // So we need a special case when size=0, but address matches the symbol's
+  if (sym->address == start ||
+      (sym->address > start && sym->address < (start + size))) {
     sym->start = start;
     sym->size = size;
     sym->name = symname;

src/bpftrace.cpp

@@ -278,6 +278,32 @@ int BPFtrace::add_probe(ast::Probe &p)
             generateWatchpointSetupProbe(match_ap, p));
 
         resources.watchpoint_probes.emplace_back(std::move(probe));
+      } else if (probetype(attach_point->provider) == ProbeType::uprobe) {
+        bool locations_from_dwarf = false;
+
+        // If the user specified an address/offset, do not overwrite
+        // their choice with locations from the DebugInfo.
+        if (probe.address == 0 && probe.func_offset == 0) {
+          // Get function locations from the DebugInfo, as it skips the
+          // prologue and also returns locations of inlined function calls.
+          if (auto *dwarf = get_dwarf(probe.path)) {
+            const auto locations = dwarf->get_function_locations(
+                probe.attach_point, config_.get(ConfigKeyBool::probe_inline));
+            for (const auto loc : locations) {
+              // Clear the attach point, so the address will be used instead
+              Probe probe_copy = probe;
+              probe_copy.attach_point.clear();
+              probe_copy.address = loc;
+              resources.probes.push_back(std::move(probe_copy));
+
+              locations_from_dwarf = true;
+            }
+          }
+        }
+
+        // Otherwise, use the location from the symbol table.
+        if (!locations_from_dwarf)
+          resources.probes.push_back(std::move(probe));
       } else {
         resources.probes.push_back(probe);
       }

src/config.cpp

@@ -13,6 +13,7 @@ Config::Config(bool has_cmd, bool bt_verbose) : bt_verbose_(bt_verbose)
   config_map_ = {
     { ConfigKeyBool::cpp_demangle, { .value = true } },
     { ConfigKeyBool::lazy_symbolication, { .value = false } },
+    { ConfigKeyBool::probe_inline, { .value = false } },
     { ConfigKeyInt::log_size, { .value = (uint64_t)1000000 } },
     { ConfigKeyInt::max_bpf_progs, { .value = (uint64_t)512 } },
     { ConfigKeyInt::max_cat_bytes, { .value = (uint64_t)10240 } },

src/config.h

@@ -25,6 +25,7 @@ enum class ConfigSource {
 enum class ConfigKeyBool {
   cpp_demangle,
   lazy_symbolication,
+  probe_inline,
 };
 
 enum class ConfigKeyInt {
@@ -71,6 +72,7 @@ const std::map<std::string, ConfigKey> CONFIG_KEY_MAP = {
   { "max_strlen", ConfigKeyInt::max_strlen },
   { "max_type_res_iterations", ConfigKeyInt::max_type_res_iterations },
   { "perf_rb_pages", ConfigKeyInt::perf_rb_pages },
+  { "probe_inline", ConfigKeyBool::probe_inline },
   { "stack_mode", ConfigKeyStackMode::default_ },
   { "str_trunc_trailer", ConfigKeyString::str_trunc_trailer },
 };

src/dwarf_parser.cpp

@@ -50,6 +50,31 @@ std::unique_ptr<Dwarf> Dwarf::GetFromBinary(BPFtrace *bpftrace,
   }
 }
 
+std::vector<uint64_t> Dwarf::get_function_locations(const std::string &function,
+                                                    bool include_inlined)
+{
+  // Locating every inlined instances of a function is expensive,
+  // so we only do it if the user explicitly requests it.
+  if (!include_inlined) {
+    auto syms = target_.FindSymbols(function.c_str(),
+                                    lldb::SymbolType::eSymbolTypeCode);
+    // The given function name MUST identify a unique symbol!
+    if (syms.GetSize() != 1)
+      return {};
+    auto sym = syms.GetContextAtIndex(0).GetSymbol();
+    return { sym.GetStartAddress().GetFileAddress() +
+             sym.GetPrologueByteSize() };
+  } else {
+    auto bps = target_.BreakpointCreateByName(function.c_str());
+    std::vector<uint64_t> result(bps.GetNumLocations());
+    for (uint32_t i = 0; i < bps.GetNumLocations(); i++) {
+      auto loc = bps.GetLocationAtIndex(i);
+      result[i] = loc.GetAddress().GetFileAddress();
+    }
+    return result;
+  }
+}
+
 std::string Dwarf::get_type_name(lldb::SBType type)
 {
   std::string type_name = type.GetDisplayTypeName() ?: "<anonymous type>";

src/dwarf_parser.h

@@ -23,6 +23,8 @@ class Dwarf {
   static std::unique_ptr<Dwarf> GetFromBinary(BPFtrace *bpftrace,
                                               std::string file_path);
 
+  std::vector<uint64_t> get_function_locations(const std::string &function,
+                                               bool include_inlined);
   std::vector<std::string> get_function_params(const std::string &function);
   Struct resolve_args(const std::string &function);
 
@@ -70,6 +72,14 @@ class Dwarf {
     return nullptr;
   }
 
+  std::vector<uint64_t> get_function_locations(const std::string &function
+                                               __attribute__((unused)),
+                                               bool include_inlined
+                                               __attribute__((unused)))
+  {
+    return {};
+  }
+
   std::vector<std::string> get_function_params(const std::string &function
                                                __attribute__((unused)))
   {

src/utils.cpp

@@ -847,18 +847,22 @@ std::tuple<std::string, std::string> get_kernel_dirs(
 
 const std::string &is_deprecated(const std::string &str)
 {
-  std::vector<DeprecatedName>::iterator item;
-
-  for (item = DEPRECATED_LIST.begin(); item != DEPRECATED_LIST.end(); item++) {
-    if (str == item->old_name) {
-      if (item->show_warning) {
-        LOG(WARNING) << item->old_name
-                     << " is deprecated and will be removed in the future. Use "
-                     << item->new_name << " instead.";
-        item->show_warning = false;
-      }
+  for (auto &item : DEPRECATED_LIST) {
+    if (!item.matches(str)) {
+      continue;
+    }
 
-      return item->new_name;
+    if (item.show_warning) {
+      LOG(WARNING) << item.old_name
+                   << " is deprecated and will be removed in the future. Use "
+                   << item.new_name << " instead.";
+      item.show_warning = false;
+    }
+
+    if (item.replace_by_new_name) {
+      return item.new_name;
+    } else {
+      return str;
     }
   }
 

src/utils.h

@@ -105,6 +105,18 @@ struct DeprecatedName {
   std::string old_name;
   std::string new_name;
   bool show_warning = true;
+  bool replace_by_new_name = true;
+
+  bool matches(const std::string &name) const
+  {
+    // We allow a prefix match to match against builtins with number (argX)
+    if (old_name.back() == '*') {
+      std::string_view old_name_view{ old_name.c_str(), old_name.size() - 1 };
+      return name.rfind(old_name_view) == 0;
+    }
+
+    return name == old_name;
+  }
 };
 
 typedef std::unordered_map<std::string, std::unordered_set<std::string>>
@@ -121,7 +133,9 @@ struct KConfig {
   std::unordered_map<std::string, std::string> config;
 };
 
-static std::vector<DeprecatedName> DEPRECATED_LIST = {};
+static std::vector<DeprecatedName> DEPRECATED_LIST = {
+  { "sarg*", "*(reg(\"sp\") + <stack_offset>)", true, false }
+};
 
 static std::vector<std::string> UNSAFE_BUILTIN_FUNCS = {
   "system",

tests/runtime/builtin

@@ -59,34 +59,6 @@ EXPECT_REGEX ^SUCCESS 0x[0-9a-f]+$
 TIMEOUT 5
 AFTER ./testprogs/syscall read
 
-NAME sarg
-PROG uprobe:./testprogs/stack_args:too_many_args { printf("SUCCESS %d %d\n", sarg0, sarg1); exit(); }
-EXPECT SUCCESS 32 64
-ARCH x86_64
-TIMEOUT 5
-AFTER ./testprogs/stack_args
-
-NAME sarg
-PROG uprobe:./testprogs/stack_args:too_many_args { printf("SUCCESS %d %d\n", sarg0, sarg1); exit(); }
-EXPECT SUCCESS 128 256
-ARCH aarch64|ppc64|ppc64le
-TIMEOUT 5
-AFTER ./testprogs/stack_args
-
-NAME sarg
-PROG uprobe:./testprogs/stack_args:too_many_args { printf("SUCCESS %d %d\n", sarg0, sarg1); exit(); }
-EXPECT SUCCESS 16 32
-ARCH s390x
-TIMEOUT 5
-AFTER ./testprogs/stack_args
-
-NAME sarg
-PROG uprobe:./testprogs/stack_args:too_many_args { printf("SUCCESS %d %d\n", sarg0, sarg1); exit(); }
-EXPECT SUCCESS 8 16
-ARCH armv7l
-TIMEOUT 5
-AFTER ./testprogs/stack_args
-
 NAME retval
 PROG kretprobe:vfs_read { printf("SUCCESS %d\n", retval); exit(); }
 EXPECT_REGEX SUCCESS .*

tests/runtime/call

@@ -229,7 +229,7 @@ AFTER ./testprogs/uprobe_loop
 NAME ustack_stack_mode_env_bpftrace
 PROG u:./testprogs/uprobe_loop:uprobeFunction1 { printf("%s\n", ustack(1)); exit(); }
 ENV BPFTRACE_STACK_MODE=bpftrace
-EXPECT uprobeFunction1+0
+EXPECT_REGEX ^uprobeFunction1\+[0-9]+$
 TIMEOUT 5
 AFTER ./testprogs/uprobe_loop
 # This was debugged as far down as std::cout having badbit [0] set after a
@@ -249,7 +249,7 @@ SKIP_IF_ENV_HAS CI=true
 NAME ustack_stack_mode_env_perf
 PROG u:./testprogs/uprobe_loop:uprobeFunction1 { printf("%s\n", ustack(1)); exit(); }
 ENV BPFTRACE_STACK_MODE=perf
-EXPECT_REGEX \d+ uprobeFunction1\+0 \(.*/uprobe_loop\)
+EXPECT_REGEX ^[0-9a-f]+ uprobeFunction1\+[0-9]+ \(.*/uprobe_loop\)$
 TIMEOUT 5
 AFTER ./testprogs/uprobe_loop
 # See https://github.com/bpftrace/bpftrace/issues/3080
@@ -276,17 +276,8 @@ SKIP_IF_ENV_HAS CI=true
 NAME ustack_elf_symtable
 ENV BPFTRACE_CACHE_USER_SYMBOLS=PER_PROGRAM
 PROG uprobe:./testprogs/uprobe_symres_exited_process:test { print(ustack); exit(); }
-EXPECT_REGEX ^\s+test\+0\s+main\+[1-9][0-9]*
+EXPECT_REGEX ^\s+test\+[0-9]+\s+test2\+[0-9]+\s+main\+[0-9]+
 AFTER ./testprogs/disable_aslr ./testprogs/uprobe_symres_exited_process
-ARCH x86_64
-TIMEOUT 5
-
-NAME ustack_elf_symtable
-ENV BPFTRACE_CACHE_USER_SYMBOLS=PER_PROGRAM
-PROG uprobe:./testprogs/uprobe_symres_exited_process:test { print(ustack); exit(); }
-EXPECT_REGEX ^\s+test\+0\s+test2\+[1-9][0-9]*\s+main\+[1-9][0-9]*
-AFTER ./testprogs/disable_aslr ./testprogs/uprobe_symres_exited_process
-ARCH ppc64le
 TIMEOUT 5
 
 NAME cat