Fix some bugs with positional parameters

This invalid script used to compile:
    bpftrace -e 'BEGIN { str(0); @x = $1 }' a
Now bpftrace correctly recognises that $1 is not inside the str() call

This script did not function correctly since "234" was
treated as a numeric value despite being inside str():
    bpftrace -e 'BEGIN { @x = str($1) }' 234
I've just disabled using numeric values in str() because the
codegen is not set up for it, although this is something that
should be valid.

Positional parameters are no longer marked as literals because we're
not set up for this and it resulted some dodgy memory accesses. We
probably do want them to be capable of being used as literals, but
more work is required to support that.

src/ast/ast.h

@@ -41,8 +41,9 @@ class Integer : public Expression {
 
 class PositionalParameter : public Expression {
 public:
-  explicit PositionalParameter(long n) : n(n) { is_literal = true; }
+  explicit PositionalParameter(long n) : n(n) {}
   long n;
+  bool is_in_str = false;
 
   void accept(Visitor &v) override;
 };

src/ast/printer.cpp

@@ -15,7 +15,7 @@ void Printer::visit(Integer &integer)
 void Printer::visit(PositionalParameter &param)
 {
   std::string indent(depth_, ' ');
-  out_ << indent << "builtin: $" << param.n << std::endl;
+  out_ << indent << "param: $" << param.n << std::endl;
 }
 
 void Printer::visit(String &string)

src/ast/semantic_analyser.cpp

@@ -25,23 +25,24 @@ void SemanticAnalyser::visit(PositionalParameter &param)
   param.type = SizedType(Type::integer, 8);
   std::string pstr = bpftrace_.get_param(param.n);
   if (is_final_pass()) {
-    if (!bpftrace_.is_numeric(pstr)) {
-      if (!call_ || call_->func != "str")
-        /*
-         * call_ was added just for this test: ensuring a string parameter is
-         * only used inside str(). Without it, string parameters used as
-         * integers would return their buffer address. Maybe that's ok?
-         * If this behavior is changed, codegen needs to support it.
-         */
-        err_ << "$" << param.n << " used numerically, but given \"" << pstr << "\". Try using str($" << param.n << ")." << std::endl;
+    if (!bpftrace_.is_numeric(pstr) && !param.is_in_str) {
+      err_ << "$" << param.n << " used numerically, but given \"" << pstr
+           << "\". Try using str($" << param.n << ")." << std::endl;
+    }
+    if (bpftrace_.is_numeric(pstr) && param.is_in_str) {
+      // This is blocked due to current limitations in our codegen
+      err_ << "$" << param.n << " used in str(), but given numeric value: "
+           << pstr << ". Try $" << param.n << " instead of str($"
+           << param.n << ")." << std::endl;
     }
   }
 }
 
 void SemanticAnalyser::visit(String &string)
 {
   if (string.str.size() > STRING_SIZE-1) {
-    err_ << "String is too long (over " << STRING_SIZE << " bytes): " << string.str << std::endl;
+    err_ << "String is too long (over " << STRING_SIZE << " bytes): "
+         << string.str << std::endl;
   }
   string.type = SizedType(Type::string, STRING_SIZE);
 }
@@ -199,9 +200,6 @@ void SemanticAnalyser::visit(Call &call)
       << std::endl;
   }
 
-  // needed for positional parameters context:
-  call_ = &call;
-
   if (call.vargs) {
     for (Expression *expr : *call.vargs) {
       expr->accept(*this);
@@ -299,10 +297,11 @@ void SemanticAnalyser::visit(Call &call)
     if (check_varargs(call, 1, 2)) {
       check_arg(call, Type::integer, 0);
       call.type = SizedType(Type::string, bpftrace_.strlen_);
-      if (is_final_pass()) {
-        if (call.vargs->size() > 1) {
-          check_arg(call, Type::integer, 1, false);
-        }
+      if (is_final_pass() && call.vargs->size() > 1) {
+        check_arg(call, Type::integer, 1, false);
+      }
+      if (auto *param = dynamic_cast<PositionalParameter*>(call.vargs->at(0))) {
+        param->is_in_str = true;
       }
     }
   }

src/ast/semantic_analyser.h

@@ -67,7 +67,6 @@ class SemanticAnalyser : public Visitor {
   void check_stack_call(Call &call, Type type);
 
   Probe *probe_;
-  Call *call_;
   std::map<std::string, SizedType> variable_val_;
   std::map<std::string, SizedType> map_val_;
   std::map<std::string, MapKey> map_key_;

src/bpftrace.cpp

@@ -580,9 +580,7 @@ void BPFtrace::add_param(const std::string &param)
 
 std::string BPFtrace::get_param(size_t i) const
 {
-  if (i > 0 && i < params_.size() + 1)
-      return params_[i - 1];
-  return "0";
+  return params_.at(i-1);
 }
 
 void perf_event_lost(void *cb_cookie __attribute__((unused)), uint64_t lost)

tests/parser.cpp

@@ -45,7 +45,11 @@ TEST(Parser, builtin_variables)
   test("kprobe:f { func }", "Program\n kprobe:f\n  builtin: func\n");
   test("kprobe:f { probe }", "Program\n kprobe:f\n  builtin: probe\n");
   test("kprobe:f { args }", "Program\n kprobe:f\n  builtin: args\n");
-  test("kprobe:f { $1 }", "Program\n kprobe:f\n  builtin: $1\n");
+}
+
+TEST(Parser, positional_param)
+{
+  test("kprobe:f { $1 }", "Program\n kprobe:f\n  param: $1\n");
 }
 
 TEST(Parser, comment)

tests/semantic_analyser.cpp

@@ -85,7 +85,6 @@ TEST(semantic_analyser, builtin_variables)
   test("kretprobe:f { retval }", 0);
   test("kprobe:f { func }", 0);
   test("kprobe:f { probe }", 0);
-  test("kprobe:f { $1 }", 0);
   test("tracepoint:a:b { args }", 0);
   test("kprobe:f { fake }", 1);
 }
@@ -805,9 +804,15 @@ TEST(semantic_analyser, probe_short_name)
 
 TEST(semantic_analyser, positional_parameters)
 {
-  // $1 won't be defined, will be tested more in runtime.
-  test("kprobe:f { printf(\"%d\", $1); }", 0);
-  test("kprobe:f { printf(\"%s\", str($1)); }", 0);
+  BPFtrace bpftrace;
+  bpftrace.add_param("123");
+  bpftrace.add_param("hello");
+
+  test(bpftrace, "kprobe:f { printf(\"%d\", $1); }", 0);
+  test(bpftrace, "kprobe:f { printf(\"%s\", str($1)); }", 10);
+
+  test(bpftrace, "kprobe:f { printf(\"%s\", str($2)); }", 0);
+  test(bpftrace, "kprobe:f { printf(\"%d\", $2); }", 10);
 }
 
 TEST(semantic_analyser, macros)