Update dynamic logging to work with Go's 1.17+ calling convention (#2108)

Summary: Update dynamic logging to work with Go's 1.17+ calling
convention

Despite our earlier messaging that dynamic logging would be deprecated
in #2105, I decided that it was easier to handle the new ABI instead of
deprecating the Go parts of the dynamic tracer. My rationale for this is
that the blocker for achieving feature parity for the new Go ABI,
handling `STRUCT_BLOB` variables, is actually a concern for c/c++
tracepoint programs as well. I felt it was important to keep the dynamic
tracer rather than deprecate it entirely.

As mentioned above, this change does not have full feature parity with
the legacy ABI since `STRUCT_BLOB` variables don't work with register
based calling conventions -- these variables copy a blob of memory with
the assumption that a packed struct exists in a contiguous chunk of
memory. However, this isn't unique to Go binaries as c/c++ applications
can pass certain structs via
[registers](https://github.com/pixie-io/pixie/blob/b0001dab6288508295c54f36c81b3ed80c2677e1/src/stirling/obj_tools/testdata/cc/test_exe.cc#L66-L69)
as well.

This change replaces the previous Go `STRUCT_BLOB` tests with C
equivalents that pass the variable on the stack. When a struct variable
will be passed via a register, dynamic logging will now return an error
and suggest to the user to access struct fields individually. This error
and the test cases that assert the error is returned can be reverted to
their original form once #2106 is complete.

Relevant Issues: #2105

Type of change: /kind cleanup

Test Plan: Existing tests pass and new ones were added where coverage
was needed

Changelog Message: Update Go dynamic logging feature to support Go 1.17+
binaries

---------

Signed-off-by: Dom Del Nano <[email protected]>

Author: ddelnano

src/stirling/obj_tools/testdata/cc/test_exe.cc

@@ -72,6 +72,17 @@ ABCStruct64 ABCSumMixed(ABCStruct32 x, ABCStruct64 y, int32_t z_a, int64_t z_b,
   return ABCStruct64{x.a + y.a + z_a + w.a, x.b + y.b + z_b + w.b, x.c + y.c + z_c + w.c};
 }
 
+void OuterStructFunc(OuterStruct x) {
+  x.O0++;
+  x.O1.M0.L0 = !x.O1.M0.L0;
+  x.O1.M0.L1++;
+  x.O1.M0.L2++;
+  x.O1.M1 = !x.O1.M1;
+  x.O1.M2.L0 = !x.O1.M2.L0;
+  x.O1.M2.L1++;
+  x.O1.M2.L2++;
+}
+
 void SomeFunctionWithPointerArgs(int* a, ABCStruct32* x) {
   x->a = *a;
   a++;
@@ -113,6 +124,7 @@ int main() {
 
     sleep(1);
   }
+  OuterStructFunc(OuterStruct{1, MidStruct{{true, 2, nullptr}, false, {true, 3, nullptr}}});
 
   return 0;
 }

src/stirling/source_connectors/dynamic_tracer/BUILD.bazel

@@ -49,7 +49,9 @@ pl_cc_bpf_test(
     srcs = ["dynamic_trace_bpf_test.cc"],
     data = [
         "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_21_grpc_client",
         "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_21_grpc_server_with_certs",
     ],
     tags = [
         "cpu:16",
@@ -69,7 +71,7 @@ pl_cc_bpf_test(
     srcs = ["stirling_dt_bpf_test.cc"],
     data = [
         "//src/stirling/obj_tools/testdata/cc:test_exe",
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
         "//src/stirling/testing/dns:dns_hammer",
         "//src/stirling/testing/dns:dns_hammer_image.tar",
     ],

src/stirling/source_connectors/dynamic_tracer/dynamic_trace_bpf_test.cc

@@ -32,11 +32,12 @@
 
 constexpr std::string_view kClientPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
-    "golang_1_16_grpc_client";
+    "golang_1_21_grpc_client";
 constexpr std::string_view kServerPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-    "golang_1_16_grpc_server";
+    "golang_1_21_grpc_server";
 
+DECLARE_bool(debug_dt_pipeline);
 namespace px {
 namespace stirling {
 

src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/BUILD.bazel

@@ -45,7 +45,7 @@ pl_cc_test(
     name = "code_gen_test",
     srcs = ["code_gen_test.cc"],
     data = [
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
     ],
     deps = [
         ":cc_library",
@@ -56,7 +56,8 @@ pl_cc_test(
     name = "dwarvifier_test",
     srcs = ["dwarvifier_test.cc"],
     data = [
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
+        "//src/stirling/obj_tools/testdata/cc:test_exe",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
     ],
     deps = [
         ":cc_library",
@@ -67,7 +68,7 @@ pl_cc_test(
     name = "probe_transformer_test",
     srcs = ["probe_transformer_test.cc"],
     data = [
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
     ],
     deps = [
         ":cc_library",
@@ -79,7 +80,7 @@ pl_cc_test(
     name = "autogen_test",
     srcs = ["autogen_test.cc"],
     data = [
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
     ],
     deps = [
         ":cc_library",
@@ -91,9 +92,9 @@ pl_cc_test(
     name = "dynamic_tracer_test",
     srcs = ["dynamic_tracer_test.cc"],
     data = [
-        "//src/stirling/obj_tools/testdata/go:test_go_1_16_binary",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/obj_tools/testdata/go:test_go_1_21_binary",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_21_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_21_grpc_server_with_certs",
     ],
     tags = [
         "exclusive",

src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/autogen_test.cc

@@ -22,7 +22,7 @@
 #include "src/common/testing/testing.h"
 #include "src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/autogen.h"
 
-constexpr std::string_view kBinaryPath = "src/stirling/obj_tools/testdata/go/test_go_1_16_binary";
+constexpr std::string_view kBinaryPath = "src/stirling/obj_tools/testdata/go/test_go_1_21_binary";
 
 namespace px {
 namespace stirling {
@@ -111,8 +111,8 @@ tracepoints {
       fields: "i1"
       fields: "i2"
       fields: "i3"
-      fields: "__tilde__r6"
-      fields: "__tilde__r7"
+      fields: "__tilde__r0"
+      fields: "__tilde__r1"
       fields: "latency"
     }
     probes {
@@ -147,13 +147,15 @@ tracepoints {
       }
       ret_vals {
         id: "retval6"
-        expr: "~r6"
+        expr: "~r0"
       }
       ret_vals {
         id: "retval7"
-        expr: "~r7"
+        expr: "~r1"
+      }
+      function_latency {
+        id: "fn_latency"
       }
-      function_latency { id: "fn_latency" }
       output_actions {
         output_name: "main__d__MixedArgTypes_table"
         variable_names: "arg0"

src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/code_gen_test.cc

@@ -22,7 +22,7 @@
 
 #include "src/common/testing/testing.h"
 
-constexpr std::string_view kBinaryPath = "src/stirling/obj_tools/testdata/go/test_go_1_16_binary";
+constexpr std::string_view kBinaryPath = "src/stirling/obj_tools/testdata/go/test_go_1_21_binary";
 
 namespace px {
 namespace stirling {

src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/dwarvifier.cc

@@ -97,8 +97,9 @@ class Dwarvifier {
                                                       uint64_t offset, const TypeInfo& type_info);
 
   // Used by ProcessVarExpr() to handle a Struct variable.
-  Status ProcessStructBlob(const std::string& base, uint64_t offset, const TypeInfo& type_info,
-                           const std::string& var_name, ir::physical::Probe* output_probe);
+  Status ProcessStructBlob(const ArgInfo& arg_info, const std::string& base, uint64_t offset,
+                           const TypeInfo& type_info, const std::string& var_name,
+                           ir::physical::Probe* output_probe);
 
   // The input components describes a sequence of field of nesting structures. The first component
   // is the name of an input argument of a function, or an expression to describe the index of an
@@ -569,11 +570,11 @@ void Dwarvifier::AddEntryProbeVariables(ir::physical::Probe* output_probe) {
     auto* parm_ptr_var =
         AddVariable<ScalarVariable>(output_probe, kParmPtrVarName, ir::shared::VOID_POINTER);
     parm_ptr_var->set_reg(ir::physical::Register::SYSV_AMD64_ARGS_PTR);
+  } else if (language_ == ir::shared::GOLANG) {
+    auto* parm_ptr_var =
+        AddVariable<ScalarVariable>(output_probe, kParmPtrVarName, ir::shared::VOID_POINTER);
+    parm_ptr_var->set_reg(ir::physical::Register::GOLANG_ARGS_PTR);
   }
-  // TODO(oazizi): For Golang 1.17+, will need the following:
-  //  auto* parm_ptr_var =
-  //          AddVariable<ScalarVariable>(output_probe, kParmPtrVarName, ir::shared::VOID_POINTER);
-  //  parm_ptr_var->set_reg(ir::physical::Register::GOLANG_ARGS_PTR);
 }
 
 void Dwarvifier::AddRetProbeVariables(ir::physical::Probe* output_probe) {
@@ -585,6 +586,10 @@ void Dwarvifier::AddRetProbeVariables(ir::physical::Probe* output_probe) {
     auto* rc_ptr_var =
         AddVariable<ScalarVariable>(output_probe, kRCPtrVarName, ir::shared::VOID_POINTER);
     rc_ptr_var->set_reg(ir::physical::Register::RC_PTR);
+  } else if (language_ == ir::shared::GOLANG) {
+    auto* parm_ptr_var =
+        AddVariable<ScalarVariable>(output_probe, kParmPtrVarName, ir::shared::VOID_POINTER);
+    parm_ptr_var->set_reg(ir::physical::Register::GOLANG_ARGS_PTR);
   }
 }
 
@@ -788,11 +793,22 @@ Status Dwarvifier::ProcessGolangInterfaceExpr(const std::string& base, uint64_t
   return Status::OK();
 }
 
-Status Dwarvifier::ProcessStructBlob(const std::string& base, uint64_t offset,
-                                     const TypeInfo& type_info, const std::string& var_name,
+Status Dwarvifier::ProcessStructBlob(const ArgInfo& arg_info, const std::string& base,
+                                     uint64_t offset, const TypeInfo& type_info,
+                                     const std::string& var_name,
                                      ir::physical::Probe* output_probe) {
   PX_ASSIGN_OR_RETURN(std::vector<StructSpecEntry> struct_spec_entires,
                       dwarf_reader_->GetStructSpec(type_info.type_name));
+  VLOG(1) << arg_info.ToString();
+  // TODO(ddelnano): Remove once structs in registers are supported (gh#2106)
+  if (arg_info.location.loc_type == LocationType::kRegister) {
+    auto message = absl::Substitute(
+        "Structs variables from registers aren't supported yet. Add an expr to '$0' to access an "
+        "individual, non struct field (e.g. expr: \"struct_name.field_a\") until this is supported "
+        "(gh#2106).",
+        var_name);
+    return error::InvalidArgument(message);
+  }
   PX_ASSIGN_OR_RETURN(ir::physical::StructSpec struct_spec_proto,
                       CreateStructSpecProto(struct_spec_entires, language_));
 
@@ -917,7 +933,8 @@ Status Dwarvifier::ProcessVarExpr(const std::string& var_name, const ArgInfo& ar
       PX_RETURN_IF_ERROR(
           ProcessGolangInterfaceExpr(base, offset, type_info, var_name, output_probe));
     } else {
-      PX_RETURN_IF_ERROR(ProcessStructBlob(base, offset, type_info, var_name, output_probe));
+      PX_RETURN_IF_ERROR(
+          ProcessStructBlob(arg_info, base, offset, type_info, var_name, output_probe));
     }
   } else {
     return error::Internal("Expected struct or base type, but got type: $0", type_info.ToString());
@@ -936,13 +953,23 @@ Status Dwarvifier::ProcessArgExpr(const ir::logical::Argument& arg,
 
   PX_ASSIGN_OR_RETURN(ArgInfo arg_info, GetArgInfo(args_map_, components.front()));
 
+  std::string base_var;
   switch (language_) {
     case ir::shared::GOLANG:
-      return ProcessVarExpr(arg.id(), arg_info, kSPVarName, components, output_probe);
+      switch (arg_info.location.loc_type) {
+        case LocationType::kStack:
+          base_var = kSPVarName;
+          break;
+        case LocationType::kRegister:
+          base_var = kParmPtrVarName;
+          break;
+        default:
+          return error::Internal("Unsupported argument LocationType $0",
+                                 magic_enum::enum_name(arg_info.location.loc_type));
+      }
+      return ProcessVarExpr(arg.id(), arg_info, base_var, components, output_probe);
     case ir::shared::CPP:
     case ir::shared::C: {
-      std::string base_var;
-
       switch (arg_info.location.loc_type) {
         case LocationType::kStack:
           base_var = kSPVarName;
@@ -999,7 +1026,7 @@ Status Dwarvifier::ProcessRetValExpr(const ir::logical::ReturnValue& ret_val,
 
   switch (language_) {
     case ir::shared::GOLANG: {
-      // This represents the actualy return value being returned,
+      // This represents the actual return value being returned,
       // without sub-field accesses.
       std::string ret_val_name(components.front());
 
@@ -1029,8 +1056,15 @@ Status Dwarvifier::ProcessRetValExpr(const ir::logical::ReturnValue& ret_val,
 
       // Golang return values are really arguments located on the stack, so get the arg info.
       PX_ASSIGN_OR_RETURN(ArgInfo arg_info, GetArgInfo(args_map_, ret_val_name));
-
-      return ProcessVarExpr(ret_val.id(), arg_info, kSPVarName, components, output_probe);
+      switch (arg_info.location.loc_type) {
+        case LocationType::kStack:
+          return ProcessVarExpr(ret_val.id(), arg_info, kSPVarName, components, output_probe);
+        case LocationType::kRegister:
+          return ProcessVarExpr(ret_val.id(), arg_info, kParmPtrVarName, components, output_probe);
+        default:
+          return error::Internal("Unsupported return value LocationType $0",
+                                 magic_enum::enum_name(arg_info.location.loc_type));
+      }
     }
     case ir::shared::CPP:
     case ir::shared::C: {