[BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[yonghong-song]

Summary

In current linux BPF uapi

https://github.com/torvalds/linux/blob/master/include/uapi/linux/bpf.h

struct __sk_buff/xdp_md have fields

__u32 data;
__u32 data_end;
__u32 data_meta;

which actually represent pointers. Typically in bpf program, users
write the following:

void *data = (void *)(long)__sk_buff->data;

The kernel verifier magically copied the address to the target
64bit register for __sk_buff->data and hope nothing is messed up
and it can survive to the variable "data".

In the past, we have seen a few issues with this. For example,
for the above C code, the IR looks like:

i32_v = load u32
i64_v = zext i32_v
...

The BPF backend has tried, through InstrInfo.td pattern matching or
optimization in MachineInstr SSA analysis and transformation,
to recognize the above pattern to remove zext so the really "addr"
is preserved. But this is still fragile and in the past, we have
to fix multiple bugs due to other changes in BPF backend. The
optimization may not cover all possible cases. Some users may even
use inline assembly to work around potentially missed compiler
zext elimination.

The patch introduced the following builtin function for bpf target:

void *ptr = __builtin_bpf_load_u32_to_ptr(void *base, int offset);

The builtin will perform a 32bit load with address "base + offset"
and the result, with zext, will be returned. This way, user is
guaranteed a correct address.

[yonghong-song]

The corresponding llvm change: https://reviews.llvm.org/D81480

[ast]

clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c

5

can it be expressed as:
__builtin_load_u32_to_ptr(&arg->b) ?

[yonghong-song]

clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c

5

Good question. If this, we will have code like

tmp = ctx + 76
void *ptr = *(u32 *)(tmp + 0)

IIRC, the verifier does not like this. The verifier prefers ctx + offset.

[anakryiko]

IntrinsicsBPF.td uses __builtin_bpf_load_u32_to_ptr, while everywhere it is just __builtin_load_u32_to_ptr. I think having bpf prefix there is good, given this is bpf-specific. But either way, probably should be consistent everywhere?

[yonghong-song]

In D81479#2083510, @anakryiko wrote:

IntrinsicsBPF.td uses __builtin_bpf_load_u32_to_ptr, while everywhere it is just __builtin_load_u32_to_ptr. I think having bpf prefix there is good, given this is bpf-specific. But either way, probably should be consistent everywhere?

Sounds good. This is indeed specific to bpf, adding bpf prefix to the public builtin name sounds reasonable. Will make the change.

[yonghong-song]

change builtin name from builtin_load_u32_to_ptr to builtin_bpf_load_u32_to_ptr to reflect it is bpf specific.

[ast]

It feels that the same thing can be represented as inline asm.
What advantage builtin has?

[yonghong-song]

In D81479#2083933, @ast wrote:

It feels that the same thing can be represented as inline asm.
What advantage builtin has?

Yes, this can be represented as an inline asm. I have a tendency not liking inline assembly codes in bpf programs.
But maybe for this one, we could hide inline asm in the header and provide the same signature like
__builtin_bpf_load_u32_to_ptr() to users.

[yonghong-song]

I guess I will go with inline asm in kernel for now as llvm seems already doing a pretty good job parsing/understanding inline asm to integrated into optimization passes. A few passes like SimplifyCFG, GVN, etc. may have some impact but probably does not really matter for the use case here.

[yonghong-song]

inline asm can do the work, so abandon this patch.

clang/include/clang/Basic/BuiltinsBPF.def

 #endif
 
 // Get record field information.
 TARGET_BUILTIN(__builtin_preserve_field_info, "Ui.", "t", "")
 
 // Get BTF type id.
 TARGET_BUILTIN(__builtin_btf_type_id, "Ui.", "t", "")
 
+// Load a unsigned value and convert it to a pointer.
+TARGET_BUILTIN(__builtin_load_u32_to_ptr, "v*v*Ui", "n", "")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_builtin_matrix_arg: Error<
   "%select{first|second}0 argument must be a matrix">;
 
 def err_preserve_field_info_not_field : Error<
   "__builtin_preserve_field_info argument %0 not a field access">;
 def err_preserve_field_info_not_const: Error<
   "__builtin_preserve_field_info argument %0 not a constant">;
 def err_btf_type_id_not_const: Error<
   "__builtin_btf_type_id argument %0 not a constant">;
+def err_load_u32_to_ptr_not_const: Error<
+  "__builtin_load_u32_to_ptr argument %0 not a constant">;
 
 def err_bit_cast_non_trivially_copyable : Error<
   "__builtin_bit_cast %select{source|destination}0 type must be trivially copyable">;
 def err_bit_cast_type_size_mismatch : Error<
   "__builtin_bit_cast source size does not equal destination size (%0 vs %1)">;
 
 // SYCL-specific diagnostics
 def warn_sycl_kernel_num_of_template_params : Warning<

clang/lib/CodeGen/CGBuiltin.cpp

case NEON::BI__builtin_neon_vuqaddq_v: {
     return EmitNeonCall(CGM.getIntrinsic(Int, Ty), Ops, "vuqadd");
   }
   }
 }
 
 Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID,
                                            const CallExpr *E) {
   assert((BuiltinID == BPF::BI__builtin_preserve_field_info ||
-          BuiltinID == BPF::BI__builtin_btf_type_id) &&
+          BuiltinID == BPF::BI__builtin_btf_type_id ||
+          BuiltinID == BPF::BI__builtin_load_u32_to_ptr) &&
          "unexpected BPF builtin");
 
   switch (BuiltinID) {
   default:
     llvm_unreachable("Unexpected BPF builtin");
   case BPF::BI__builtin_preserve_field_info: {
     const Expr *Arg = E->getArg(0);
     bool IsBitField = Arg->IgnoreParens()->getObjectKind() == OK_BitField;
case BPF::BI__builtin_btf_type_id: {
     Constant *CV = ConstantInt::get(IntTy, IsLValue);
     llvm::Function *FnBtfTypeId = llvm::Intrinsic::getDeclaration(
         &CGM.getModule(), llvm::Intrinsic::bpf_btf_type_id,
         {FieldVal->getType(), CV->getType()});
     CallInst *Fn = Builder.CreateCall(FnBtfTypeId, {FieldVal, CV, FlagValue});
     Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo);
     return Fn;
   }
+  case BPF::BI__builtin_load_u32_to_ptr: {
+    Value *BaseV = EmitScalarExpr(E->getArg(0));
+    ConstantInt *C = cast<ConstantInt>(EmitScalarExpr(E->getArg(1)));
+    Value *OffsetV = ConstantInt::get(Int64Ty, C->getSExtValue());
+
+    // Built the IR for the bpf_load_u32_to_ptr intrinsic.
+    llvm::Function *FnLoadU32ToPtr = llvm::Intrinsic::getDeclaration(
+        &CGM.getModule(), llvm::Intrinsic::bpf_load_u32_to_ptr, {});
+    return Builder.CreateCall(FnLoadU32ToPtr, {BaseV, OffsetV});
+  }
   }
 }
 
 llvm::Value *CodeGenFunction::
 BuildVector(ArrayRef<llvm::Value*> Ops) {
   assert((Ops.size() & (Ops.size() - 1)) == 0 &&
          "Not a power-of-two sized vector!");
   bool AllConstants = true;

clang/lib/Sema/SemaChecking.cpp

bool Sema::CheckAArch64BuiltinFunctionCall(const TargetInfo &TI,
   }
 
   return SemaBuiltinConstantArgRange(TheCall, i, l, u + l);
 }
 
 bool Sema::CheckBPFBuiltinFunctionCall(unsigned BuiltinID,
                                        CallExpr *TheCall) {
   assert((BuiltinID == BPF::BI__builtin_preserve_field_info ||
-          BuiltinID == BPF::BI__builtin_btf_type_id) &&
-         "unexpected ARM builtin");
+          BuiltinID == BPF::BI__builtin_btf_type_id ||
+          BuiltinID == BPF::BI__builtin_load_u32_to_ptr) &&
+         "unexpected BPF builtin");
+
+  // Generic checking has done basic checking against the
+  // signature, here only to ensure the second argument
+  // be a constant.
+  Expr *Arg;
+  if (BuiltinID == BPF::BI__builtin_load_u32_to_ptr) {
+    llvm::APSInt Value;
+    Arg = TheCall->getArg(1);
+    if (!Arg->isIntegerConstantExpr(Value, Context)) {
+      Diag(Arg->getBeginLoc(), diag::err_load_u32_to_ptr_not_const)
+          << 2 << Arg->getSourceRange();
+      return true;
+    }
+    return false;
+  }
 
   if (checkArgCount(*this, TheCall, 2))
     return true;
 
-  Expr *Arg;
   if (BuiltinID == BPF::BI__builtin_btf_type_id) {
     // The second argument needs to be a constant int
     llvm::APSInt Value;
     Arg = TheCall->getArg(1);
     if (!Arg->isIntegerConstantExpr(Value, Context)) {
       Diag(Arg->getBeginLoc(), diag::err_btf_type_id_not_const)
           << 2 << Arg->getSourceRange();
       return true;

clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c

+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S %s -o - | FileCheck %s
+
+struct t { int a; int b; };
+void *test(struct t *arg) { return __builtin_load_u32_to_ptr(arg, 4); }
+
+// CHECK: define dso_local i8* @test
+// CHECK: call i8* @llvm.bpf.load.u32.to.ptr(i8* %{{[0-9a-z.]+}}, i64 4)

clang/test/Sema/builtin-bpf-load-u32-to-ptr.c

+// RUN: %clang_cc1 -x c -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+struct t { int a; int b; };
+
+void *invalid1(struct t *arg) { return __builtin_load_u32_to_ptr(arg, arg->a); } // expected-error {{__builtin_load_u32_to_ptr argument 2 not a constant}}
+void *invalid2(struct t *arg) { return __builtin_load_u32_to_ptr(arg + 4); } // expected-error {{too few arguments to function call, expected 2, have 1}}
+void *invalid3(struct t *arg) { return __builtin_load_u32_to_ptr(arg, 4, 0); } // expected-error {{too many arguments to function call, expected 2, have 3}}
+unsigned invalid4(struct t *arg) { return __builtin_load_u32_to_ptr(arg, 4); } // expected-warning {{incompatible pointer to integer conversion returning 'void *' from a function with result type 'unsigned int'}}
+
+void *valid1(struct t *arg) { return __builtin_load_u32_to_ptr(arg, 4); }

llvm/include/llvm/IR/IntrinsicsBPF.td

let TargetPrefix = "bpf" in {  // All intrinsics start with "llvm.bpf."
   def int_bpf_pseudo : GCCBuiltin<"__builtin_bpf_pseudo">,
               Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty]>;
   def int_bpf_preserve_field_info : GCCBuiltin<"__builtin_bpf_preserve_field_info">,
               Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty, llvm_i64_ty],
               [IntrNoMem, ImmArg<ArgIndex<1>>]>;
   def int_bpf_btf_type_id : GCCBuiltin<"__builtin_bpf_btf_type_id">,
               Intrinsic<[llvm_i32_ty], [llvm_any_ty, llvm_any_ty, llvm_i64_ty],
               [IntrNoMem]>;
+  def int_bpf_load_u32_to_ptr : GCCBuiltin<"__builtin_bpf_load_u32_to_ptr">,
+              Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i64_ty], [IntrReadMem]>;
 }