Fix: parsing unsigned 64 bit positional params

Previously either of these would fail ``` $ bpftrace -e 'BEGIN {printf("%lu", $1);}' 0xffffffffffffffff $ bpftrace -e 'BEGIN {printf("%lu", $1);}' "18446744073709551615" ``` Because we were only using `stoll` which expects signed ints. This change falls back to using `stoull` and changes the type accordingly if `stoll` fails. Issue: #3335
bpftrace · Jul 30, 2024 · c13be5d · c13be5d
1 parent 775dad0
commit c13be5d
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,8 @@ and this project adheres to
   - [#3322](https://github.com/bpftrace/bpftrace/pull/3322)
 - Fix lldb support in appimage builds
   - #[3339](https://github.com/bpftrace/bpftrace/pull/3339)
+- Fix parsing large unsigned int strings as positional params
+  - [#3336](https://github.com/bpftrace/bpftrace/pull/3336)
 #### Security
 #### Docs
 - Remove mention of unsupported character literals

diff --git a/src/ast/passes/codegen_llvm.cpp b/src/ast/passes/codegen_llvm.cpp
@@ -126,7 +126,11 @@ void CodegenLLVM::visit(PositionalParameter &param)
     case PositionalParameterType::positional: {
       std::string pstr = bpftrace_.get_param(param.n, param.is_in_str);
       if (!param.is_in_str) {
-        expr_ = b_.getInt64(std::stoll(pstr, nullptr, 0));
+        if (param.type.IsSigned()) {
+          expr_ = b_.getInt64(std::stoll(pstr, nullptr, 0));
+        } else {
+          expr_ = b_.getInt64(std::stoull(pstr, nullptr, 0));
+        }
       } else {
         Constant *const_str = ConstantDataArray::getString(
             module_->getContext(), pstr, true);

diff --git a/src/ast/passes/semantic_analyser.cpp b/src/ast/passes/semantic_analyser.cpp
@@ -57,11 +57,15 @@ void SemanticAnalyser::visit(PositionalParameter &param)
             << "$" << std::to_string(param.n) + " is not a valid parameter";
       if (is_final_pass()) {
         std::string pstr = bpftrace_.get_param(param.n, param.is_in_str);
-        if (!is_numeric(pstr) && !param.is_in_str) {
+        auto param_int = get_int_from_str(pstr);
+        if (!param_int.has_value() && !param.is_in_str) {
           LOG(ERROR, param.loc, err_)
               << "$" << param.n << " used numerically but given \"" << pstr
               << "\". Try using str($" << param.n << ").";
         }
+        if (std::holds_alternative<uint64_t>(*param_int)) {
+          param.type = CreateUInt64();
+        }
         // string allocated in bpf stack. See codegen.
         if (param.is_in_str)
           param.type.SetAS(AddrSpace::kernel);

diff --git a/src/bpftrace.cpp b/src/bpftrace.cpp
@@ -2050,14 +2050,18 @@ std::optional<int64_t> BPFtrace::get_int_literal(
                  expr)) {
       if (pos_param->ptype == PositionalParameterType::positional) {
         auto param_str = get_param(pos_param->n, false);
-        if (is_numeric(param_str))
-          return std::stoll(param_str);
-        else {
+        auto param_int = get_int_from_str(param_str);
+        if (!param_int.has_value()) {
           LOG(ERROR, pos_param->loc)
               << "$" << pos_param->n << " used numerically but given \""
               << param_str << "\"";
           return std::nullopt;
         }
+        if (std::holds_alternative<int64_t>(*param_int)) {
+          return std::get<int64_t>(*param_int);
+        } else {
+          return (int64_t)std::get<uint64_t>(*param_int);
+        }
       } else
         return (int64_t)num_params();
     }

diff --git a/src/utils.cpp b/src/utils.cpp
@@ -1077,15 +1077,47 @@ std::string str_join(const std::vector<std::string> &list,
   return str;
 }
 
-bool is_numeric(const std::string &s)
+std::optional<std::variant<int64_t, uint64_t>> get_int_from_str(
+    const std::string &s)
 {
-  std::size_t idx;
-  try {
-    std::stoll(s, &idx, 0);
-  } catch (...) {
-    return false;
+  if (s.size() == 0) {
+    return std::nullopt;
+  }
+
+  if (s.starts_with("0x") || s.starts_with("0X")) {
+    // Treat all hex's as unsigned
+    std::size_t idx;
+    try {
+      uint64_t ret = std::stoull(s, &idx, 0);
+      if (idx == s.size()) {
+        return ret;
+      } else {
+        return std::nullopt;
+      }
+    } catch (...) {
+      return std::nullopt;
+    }
+  }
+
+  char *endptr;
+  const char *s_ptr = s.c_str();
+  errno = 0;
+
+  if (s.at(0) == '-') {
+    int64_t ret = strtol(s_ptr, &endptr, 0);
+    if (endptr == s_ptr || *endptr != '\0' || errno == ERANGE ||
+        errno == EINVAL) {
+      return std::nullopt;
+    }
+    return ret;
+  }
+
+  uint64_t ret = strtoul(s_ptr, &endptr, 0);
+  if (endptr == s_ptr || *endptr != '\0' || errno == ERANGE ||
+      errno == EINVAL) {
+    return std::nullopt;
   }
-  return idx == s.size();
+  return ret;
 }
 
 bool symbol_has_cpp_mangled_signature(const std::string &sym_name)

diff --git a/src/utils.h b/src/utils.h
@@ -15,6 +15,7 @@
 #include <tuple>
 #include <unordered_map>
 #include <unordered_set>
+#include <variant>
 #include <vector>
 
 #include "filesystem.h"
@@ -222,7 +223,8 @@ std::string path_for_pid_mountns(int pid, const std::string &path);
 void cat_file(const char *filename, size_t, std::ostream &);
 std::string str_join(const std::vector<std::string> &list,
                      const std::string &delim);
-bool is_numeric(const std::string &str);
+std::optional<std::variant<int64_t, uint64_t>> get_int_from_str(
+    const std::string &s);
 bool symbol_has_cpp_mangled_signature(const std::string &sym_name);
 std::optional<pid_t> parse_pid(const std::string &str, std::string &err);
 std::string hex_format_buffer(const char *buf,

diff --git a/tests/runtime/other b/tests/runtime/other
@@ -129,6 +129,11 @@ RUN {{BPFTRACE}} -e '$1 { printf("hello world\n"); exit(); }' i:ms:1
 EXPECT hello world
 TIMEOUT 1
 
+NAME positional unsigned hex
+RUN {{BPFTRACE}} -e 'BEGIN {printf("%lu", $1);}' 0xffffffffffffffff
+EXPECT 18446744073709551615
+TIMEOUT 1
+
 NAME string compare map lookup
 RUN {{BPFTRACE}} -e 't:syscalls:sys_enter_openat /comm == "syscall"/ { @[comm] = 1; }' -c "./testprogs/syscall openat"
 EXPECT @[syscall]: 1

diff --git a/tests/semantic_analyser.cpp b/tests/semantic_analyser.cpp
@@ -2303,7 +2303,7 @@ TEST(semantic_analyser, positional_parameters)
   auto stmt = static_cast<ast::ExprStatement *>(
       driver.ctx.root->probes->at(0)->stmts->at(0));
   auto pp = static_cast<ast::PositionalParameter *>(stmt->expr);
-  EXPECT_EQ(CreateInt64(), pp->type);
+  EXPECT_EQ(CreateUInt64(), pp->type);
   EXPECT_TRUE(pp->is_literal);
 
   bpftrace.add_param("0999");