Force udst ctx->#reg load to be volatile

This is related to issue #1133. Compiler sometimes
generates code patterns likes:
     r1 = ctx + 96
     goto next
   here:
     r1 = ctx + 48
   next:
     r3 = load (r1 + 0)
Verifier will fail for such cases as r1 is marked
as "unknown" at the time of load.

The previous workaround is to add volatile attribute
to the store like
   *(volatile u64 *)&dest = ctx->bx
The hope is to force ctx related load in-place since
its value is needed for store.

Unfortunately, this does not always work and compiler
still has freedom to merge different ctx loads at the
same time honoring the volatile &dest. In USDT generated
code, different branches of &dest are the same.

This patch directly make ctx->bx itself as a volatile load:
  *(volatile u64 *)&ctx->bx
This seems working as compiler stops playing around
the address pointing to a volatile data.

Signed-off-by: Yonghong Song <yhs@fb.com>

diff --git a/src/cc/usdt.cc b/src/cc/usdt.cc
index bd21c50625ea67865e09140814415208511d7900..8801b19331d682b05435eb50ee78b100636acbfd 100644 (file)
--- a/src/cc/usdt.cc
+++ b/src/cc/usdt.cc
@@ -152,7 +152,7 @@ bool Probe::usdt_getarg(std::ostream &stream) {
 
   for (size_t arg_n = 0; arg_n < arg_count; ++arg_n) {
     std::string ctype = largest_arg_type(arg_n);
-    std::string cptr = tfm::format("*((volatile %s *)dest)", ctype);
+    std::string cptr("dest");
 
     tfm::format(stream,
                 "static __always_inline int _bpf_readarg_%s_%d("

diff --git a/src/cc/usdt_args.cc b/src/cc/usdt_args.cc
index 5d76c04f3ffa8f9d8d4f65c429b07fd5a6a10255..ee1a3ee528416d1a1f95d4e1ccde4b36ee0e92d1 100644 (file)
--- a/src/cc/usdt_args.cc
+++ b/src/cc/usdt_args.cc
@@ -67,17 +67,17 @@ bool Argument::assign_to_local(std::ostream &stream,
   }
 
   if (!deref_offset_) {
-    tfm::format(stream, "%s = (%s)ctx->%s;", local_name, ctype(),
+    tfm::format(stream, "%s = *(volatile %s *)&ctx->%s;", local_name, ctype(),
                 *base_register_name_);
     return true;
   }
 
   if (deref_offset_ && !deref_ident_) {
-    tfm::format(stream, "{ u64 __addr = ctx->%s + (%d)",
+    tfm::format(stream, "{ u64 __addr = (*(volatile u64 *)&ctx->%s) + (%d)",
                 *base_register_name_, *deref_offset_);
     if (index_register_name_) {
       int scale = scale_.value_or(1);
-      tfm::format(stream, " + (ctx->%s * %d);", *index_register_name_, scale);
+      tfm::format(stream, " + ((*(volatile u64 *)&ctx->%s) * %d);", *index_register_name_, scale);
     } else {
       tfm::format(stream, ";");
     }