From dfc1f92653707c8d11bdb3be98e68f8297b9bc71 Mon Sep 17 00:00:00 2001

From: Viktor Malik <viktor.malik@gmail.com>

Date: Mon, 10 Oct 2022 14:26:38 +0200

Subject: [PATCH] IR builder: get rid of getPointerElementType calls

Usage of Value::getPointerElementType is deprecated and will be dropped

in LLVM 16 [1].

There are several places where we use this method:

- function (value) calls - the called function type is usually

  available, so just pass it to createCall, the only exception is

  CreateProbeReadStr which must have been refactored

- getting the type of alloca instruction - there is a dedicated

  AllocaInst::getAllocatedType method that can be used instead

- strncmp - pass sizes of the strings to CreateStrncmp to be able to get

  the correct string type (which is array of uint8)

[1] https://llvm.org/docs/OpaquePointers.html

---

 src/ast/irbuilderbpf.cpp        | 143 ++++++++++++--------------------

 src/ast/irbuilderbpf.h          |  23 +++--

 src/ast/passes/codegen_llvm.cpp |  30 +++++--

 3 files changed, 86 insertions(+), 110 deletions(-)

diff --git a/src/ast/irbuilderbpf.cpp b/src/ast/irbuilderbpf.cpp

index d49883f7..4036b2df 100644

--- a/src/ast/irbuilderbpf.cpp

+++ b/src/ast/irbuilderbpf.cpp

@@ -288,17 +288,16 @@ CallInst *IRBuilderBPF::CreateHelperCall(libbpf::bpf_func_id func_id,

   Constant *helper_func = ConstantExpr::getCast(Instruction::IntToPtr,

                                                 getInt64(func_id),

                                                 helper_ptr_type);

-  return createCall(helper_func, args, Name);

+  return createCall(helper_type, helper_func, args, Name);

 }

-CallInst *IRBuilderBPF::createCall(Value *callee,

+CallInst *IRBuilderBPF::createCall(FunctionType *callee_type,

+                                   Value *callee,

                                    ArrayRef<Value *> args,

                                    const Twine &Name)

 {

 #if LLVM_VERSION_MAJOR >= 11

-  auto *calleePtrType = cast<PointerType>(callee->getType());

-  auto *calleeType = cast<FunctionType>(calleePtrType->getPointerElementType());

-  return CreateCall(calleeType, callee, args, Name);

+  return CreateCall(callee_type, callee, args, Name);

 #else

   return CreateCall(callee, args, Name);

 #endif

@@ -307,7 +306,7 @@ CallInst *IRBuilderBPF::createCall(Value *callee,

 CallInst *IRBuilderBPF::CreateBpfPseudoCallId(int mapid)

 {

   Function *pseudo_func = module_.getFunction("llvm.bpf.pseudo");

-  return createCall(pseudo_func,

+  return CreateCall(pseudo_func,

                     { getInt64(BPF_PSEUDO_MAP_FD), getInt64(mapid) },

                     "pseudo");

 }

@@ -346,7 +345,8 @@ CallInst *IRBuilderBPF::createMapLookup(int mapid, Value *key)

       Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_map_lookup_elem),

       lookup_func_ptr_type);

-  return createCall(lookup_func, { map_ptr, key }, "lookup_elem");

+  return createCall(

+      lookup_func_type, lookup_func, { map_ptr, key }, "lookup_elem");

 }

 CallInst *IRBuilderBPF::CreateGetJoinMap(Value *ctx, const location &loc)

@@ -397,8 +397,7 @@ Value *IRBuilderBPF::CreateMapLookupElem(Value *ctx,

     CREATE_MEMCPY(value, call, type.GetSize(), 1);

   else

   {

-    assert(value->getType()->isPointerTy() &&

-           (value->getType()->getPointerElementType() == getInt64Ty()));

+    assert(value->getAllocatedType() == getInt64Ty());

     // createMapLookup  returns an u8*

     auto *cast = CreatePointerCast(call, value->getType(), "cast");

     CreateStore(CreateLoad(getInt64Ty(), cast), value);

@@ -448,7 +447,8 @@ void IRBuilderBPF::CreateMapUpdateElem(Value *ctx,

       Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_map_update_elem),

       update_func_ptr_type);

-  CallInst *call = createCall(update_func,

+  CallInst *call = createCall(update_func_type,

+                              update_func,

                               { map_ptr, key, val, flags },

                               "update_elem");

   CreateHelperErrorCond(ctx, call, libbpf::BPF_FUNC_map_update_elem, loc);

@@ -472,7 +472,8 @@ void IRBuilderBPF::CreateMapDeleteElem(Value *ctx,

       Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_map_delete_elem),

       delete_func_ptr_type);

-  CallInst *call = createCall(delete_func, { map_ptr, key }, "delete_elem");

+  CallInst *call = createCall(

+      delete_func_type, delete_func, { map_ptr, key }, "delete_elem");

   CreateHelperErrorCond(ctx, call, libbpf::BPF_FUNC_map_delete_elem, loc);

 }

@@ -508,72 +509,53 @@ void IRBuilderBPF::CreateProbeRead(Value *ctx,

   Constant *proberead_func = ConstantExpr::getCast(Instruction::IntToPtr,

                                                    getInt64(read_fn),

                                                    proberead_func_ptr_type);

-  CallInst *call = createCall(proberead_func,

+  CallInst *call = createCall(proberead_func_type,

+                              proberead_func,

                               { dst, size, src },

                               probeReadHelperName(read_fn));

   CreateHelperErrorCond(ctx, call, read_fn, loc);

 }

-Constant *IRBuilderBPF::createProbeReadStrFn(llvm::Type *dst,

-                                             llvm::Type *src,

-                                             AddrSpace as)

-{

-  assert(src && (src->isIntegerTy() || src->isPointerTy()));

-  // int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)

-  FunctionType *probereadstr_func_type = FunctionType::get(

-      getInt64Ty(), { dst, getInt32Ty(), src }, false);

-  PointerType *probereadstr_func_ptr_type = PointerType::get(

-      probereadstr_func_type, 0);

-  return ConstantExpr::getCast(Instruction::IntToPtr,

-                               getInt64(selectProbeReadHelper(as, true)),

-                               probereadstr_func_ptr_type);

-}

-

 CallInst *IRBuilderBPF::CreateProbeReadStr(Value *ctx,

-                                           AllocaInst *dst,

+                                           Value *dst,

                                            size_t size,

                                            Value *src,

                                            AddrSpace as,

                                            const location &loc)

 {

-  assert(ctx && ctx->getType() == getInt8PtrTy());

   return CreateProbeReadStr(ctx, dst, getInt32(size), src, as, loc);

 }

 CallInst *IRBuilderBPF::CreateProbeReadStr(Value *ctx,

                                            Value *dst,

-                                           size_t size,

-                                           Value *src,

-                                           AddrSpace as,

-                                           const location &loc)

-{

-  assert(ctx && ctx->getType() == getInt8PtrTy());

-  Constant *fn = createProbeReadStrFn(dst->getType(), src->getType(), as);

-  auto read_fn = selectProbeReadHelper(as, true);

-  CallInst *call = createCall(fn,

-                              { dst, getInt32(size), src },

-                              probeReadHelperName(read_fn));

-  CreateHelperErrorCond(ctx, call, read_fn, loc);

-  return call;

-}

-

-CallInst *IRBuilderBPF::CreateProbeReadStr(Value *ctx,

-                                           AllocaInst *dst,

                                            llvm::Value *size,

                                            Value *src,

                                            AddrSpace as,

                                            const location &loc)

 {

   assert(ctx && ctx->getType() == getInt8PtrTy());

-  assert(dst && dst->getAllocatedType()->isArrayTy() &&

-         dst->getAllocatedType()->getArrayElementType() == getInt8Ty());

   assert(size && size->getType()->isIntegerTy());

+  if (auto *dst_alloca = dyn_cast<AllocaInst>(dst))

+  {

+    assert(dst_alloca->getAllocatedType()->isArrayTy() &&

+           dst_alloca->getAllocatedType()->getArrayElementType() ==

+               getInt8Ty());

+  }

-  auto *size_i32 = CreateIntCast(size, getInt32Ty(), false);

+  auto *size_i32 = size;

+  if (size_i32->getType()->getScalarSizeInBits() != 32)

+    size_i32 = CreateIntCast(size_i32, getInt32Ty(), false);

-  Constant *fn = createProbeReadStrFn(dst->getType(), src->getType(), as);

   auto read_fn = selectProbeReadHelper(as, true);

-  CallInst *call = createCall(fn,

+  // int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr)

+  FunctionType *probereadstr_func_type = FunctionType::get(

+      getInt64Ty(), { dst->getType(), getInt32Ty(), src->getType() }, false);

+  PointerType *probereadstr_func_ptr_type = PointerType::get(

+      probereadstr_func_type, 0);

+  Constant *probereadstr_callee = ConstantExpr::getCast(

+      Instruction::IntToPtr, getInt64(read_fn), probereadstr_func_ptr_type);

+  CallInst *call = createCall(probereadstr_func_type,

+                              probereadstr_callee,

                               { dst, size_i32, src },

                               probeReadHelperName(read_fn));

   CreateHelperErrorCond(ctx, call, read_fn, loc);

@@ -732,8 +714,10 @@ Value *IRBuilderBPF::CreateUSDTReadArgument(Value *ctx,

   return result;

 }

-Value *IRBuilderBPF::CreateStrncmp(Value *val1,

-                                   Value *val2,

+Value *IRBuilderBPF::CreateStrncmp(Value *str1,

+                                   uint64_t str1_size,

+                                   Value *str2,

+                                   uint64_t str2_size,

                                    uint64_t n,

                                    bool inverse)

 {

@@ -762,40 +746,21 @@ Value *IRBuilderBPF::CreateStrncmp(Value *val1,

   // Check if the compared strings are literals.

   // If so, we can avoid storing the literal in memory.

   std::optional<std::string> literal1;

-  if (auto constString1 = dyn_cast<ConstantDataArray>(val1))

+  if (auto constString1 = dyn_cast<ConstantDataArray>(str1))

     literal1 = constString1->getAsString();

-  else if (isa<ConstantAggregateZero>(val1))

+  else if (isa<ConstantAggregateZero>(str1))

     literal1 = "";

   else

     literal1 = std::nullopt;

   std::optional<std::string> literal2;

-  if (auto constString2 = dyn_cast<ConstantDataArray>(val2))

+  if (auto constString2 = dyn_cast<ConstantDataArray>(str2))

     literal2 = constString2->getAsString();

-  else if (isa<ConstantAggregateZero>(val2))

+  else if (isa<ConstantAggregateZero>(str2))

     literal2 = "";

   else

     literal2 = std::nullopt;

-  auto *val1p = dyn_cast<PointerType>(val1->getType());

-  auto *val2p = dyn_cast<PointerType>(val2->getType());

-#ifndef NDEBUG

-  if (!literal1)

-  {

-    assert(val1p);

-    assert(val1p->getPointerElementType()->isArrayTy() &&

-           val1p->getPointerElementType()->getArrayElementType() ==

-               getInt8Ty());

-  }

-  if (!literal2)

-  {

-    assert(val2p);

-    assert(val2p->getPointerElementType()->isArrayTy() &&

-           val2p->getPointerElementType()->getArrayElementType() ==

-               getInt8Ty());

-  }

-#endif

-

   Function *parent = GetInsertBlock()->getParent();

   AllocaInst *store = CreateAllocaBPF(getInt1Ty(), "strcmp.result");

   BasicBlock *str_ne = BasicBlock::Create(module_.getContext(),

@@ -822,8 +787,8 @@ Value *IRBuilderBPF::CreateStrncmp(Value *val1,

       l = getInt8(literal1->c_str()[i]);

     else

     {

-      auto *ptr_l = CreateGEP(val1p->getPointerElementType(),

-                              val1,

+      auto *ptr_l = CreateGEP(ArrayType::get(getInt8Ty(), str1_size),

+                              str1,

                               { getInt32(0), getInt32(i) });

       l = CreateLoad(getInt8Ty(), ptr_l);

     }

@@ -833,8 +798,8 @@ Value *IRBuilderBPF::CreateStrncmp(Value *val1,

       r = getInt8(literal2->c_str()[i]);

     else

     {

-      auto *ptr_r = CreateGEP(val2p->getPointerElementType(),

-                              val2,

+      auto *ptr_r = CreateGEP(ArrayType::get(getInt8Ty(), str2_size),

+                              str2,

                               { getInt32(0), getInt32(i) });

       r = CreateLoad(getInt8Ty(), ptr_r);

     }

@@ -994,11 +959,9 @@ void IRBuilderBPF::CreateGetCurrentComm(Value *ctx,

                                         size_t size,

                                         const location &loc)

 {

-  assert(buf->getType()->getPointerElementType()->isArrayTy() &&

-         buf->getType()->getPointerElementType()->getArrayNumElements() >=

-             size &&

-         buf->getType()->getPointerElementType()->getArrayElementType() ==

-             getInt8Ty());

+  assert(buf->getAllocatedType()->isArrayTy() &&

+         buf->getAllocatedType()->getArrayNumElements() >= size &&

+         buf->getAllocatedType()->getArrayElementType() == getInt8Ty());

   // long bpf_get_current_comm(char *buf, int size_of_buf)

   // Return: 0 on success or negative error

@@ -1077,7 +1040,7 @@ void IRBuilderBPF::CreateSignal(Value *ctx, Value *sig, const location &loc)

       Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_send_signal),

       signal_func_ptr_type);

-  CallInst *call = createCall(signal_func, { sig }, "signal");

+  CallInst *call = createCall(signal_func_type, signal_func, { sig }, "signal");

   CreateHelperErrorCond(ctx, call, libbpf::BPF_FUNC_send_signal, loc);

 }

@@ -1091,7 +1054,7 @@ void IRBuilderBPF::CreateOverrideReturn(Value *ctx, Value *rc)

   Constant *override_func = ConstantExpr::getCast(Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_override_return),

       override_func_ptr_type);

-  createCall(override_func, { ctx, rc }, "override");

+  createCall(override_func_type, override_func, { ctx, rc }, "override");

 }

 CallInst *IRBuilderBPF::CreateSkbOutput(Value *skb,

@@ -1126,7 +1089,8 @@ CallInst *IRBuilderBPF::CreateSkbOutput(Value *skb,

       Instruction::IntToPtr,

       getInt64(libbpf::BPF_FUNC_skb_output),

       skb_output_func_ptr_type);

-  CallInst *call = createCall(skb_output_func,

+  CallInst *call = createCall(skb_output_func_type,

+                              skb_output_func,

                               { skb, map_ptr, flags, data, size_val },

                               "skb_output");

   return call;

@@ -1320,7 +1284,8 @@ void IRBuilderBPF::CreateSeqPrintf(Value *ctx,

                           CreateGEP(getInt64Ty(), meta, getInt64(0)),

                           "seq");

-  CallInst *call = createCall(seq_printf_func,

+  CallInst *call = createCall(seq_printf_func_type,

+                              seq_printf_func,

                               { seq, fmt, fmt_size, data, data_len },

                               "seq_printf");

   CreateHelperErrorCond(ctx, call, libbpf::BPF_FUNC_seq_printf, loc);

diff --git a/src/ast/irbuilderbpf.h b/src/ast/irbuilderbpf.h

index e124911b..c9ffb545 100644

--- a/src/ast/irbuilderbpf.h

+++ b/src/ast/irbuilderbpf.h

@@ -90,17 +90,11 @@ public:

                        AddrSpace as,

                        const location &loc;;

   CallInst *CreateProbeReadStr(Value *ctx,

-                               AllocaInst *dst,

+                               Value *dst,

                                llvm::Value *size,

                                Value *src,

                                AddrSpace as,

                                const location &loc;;

-  CallInst *CreateProbeReadStr(Value *ctx,

-                               AllocaInst *dst,

-                               size_t size,

-                               Value *src,

-                               AddrSpace as,

-                               const location &loc;;

   CallInst *CreateProbeReadStr(Value *ctx,

                                Value *dst,

                                size_t size,

@@ -115,7 +109,12 @@ public:

                                 pid_t pid,

                                 AddrSpace as,

                                 const location &loc;;

-  Value *CreateStrncmp(Value *val1, Value *val2, uint64_t n, bool inverse);

+  Value *CreateStrncmp(Value *str1,

+                       uint64_t str1_size,

+                       Value *str2,

+                       uint64_t str2_size,

+                       uint64_t n,

+                       bool inverse);

   CallInst *CreateGetNs(bool boot_time, const location &loc;;

   CallInst *CreateGetPidTgid(const location &loc;;

   CallInst *CreateGetCurrentCgroupId(const location &loc;;

@@ -131,7 +130,10 @@ public:

                              ArrayRef<Value *> args,

                              const Twine &Name,

                              const location *loc = nullptr);

-  CallInst   *createCall(Value *callee, ArrayRef<Value *> args, const Twine &Name);

+  CallInst *createCall(FunctionType *callee_type,

+                       Value *callee,

+                       ArrayRef<Value *> args,

+                       const Twine &Name);

   void        CreateGetCurrentComm(Value *ctx, AllocaInst *buf, size_t size, const location& loc);

   void CreatePerfEventOutput(Value *ctx,

                              Value *data,

@@ -185,9 +187,6 @@ private:

                                 AddrSpace as,

                                 const location &loc;;

   CallInst *createMapLookup(int mapid, Value *key);

-  Constant *createProbeReadStrFn(llvm::Type *dst,

-                                 llvm::Type *src,

-                                 AddrSpace as);

   libbpf::bpf_func_id selectProbeReadHelper(AddrSpace as, bool str);

   std::map<std::string, StructType *> structs_;

diff --git a/src/ast/passes/codegen_llvm.cpp b/src/ast/passes/codegen_llvm.cpp

index a818ca0b..2b888087 100644

--- a/src/ast/passes/codegen_llvm.cpp

+++ b/src/ast/passes/codegen_llvm.cpp

@@ -1133,8 +1133,12 @@ void CodegenLLVM::visit(Call &call)

     auto left_string = getString(left_arg);

     auto right_string = getString(right_arg);

-    expr_ = b_.CreateStrncmp(

-        left_string.first, right_string.first, size, false);

+    expr_ = b_.CreateStrncmp(left_string.first,

+                             left_string.second,

+                             right_string.first,

+                             right_string.second,

+                             size,

+                             false);

   }

   else if (call.func == "override")

   {

@@ -1269,8 +1273,7 @@ void CodegenLLVM::visit(Variable &var)

   else

   {

     auto *var_alloca = variables_[var.ident];

-    expr_ = b_.CreateLoad(var_alloca->getType()->getPointerElementType(),

-                          var_alloca);

+    expr_ = b_.CreateLoad(var_alloca->getAllocatedType(), var_alloca);

   }

 }

@@ -1310,7 +1313,12 @@ void CodegenLLVM::binop_string(Binop &binop)

   auto right_string = getString(binop.right);

   size_t len = std::min(left_string.second, right_string.second);

-  expr_ = b_.CreateStrncmp(left_string.first, right_string.first, len, inverse);

+  expr_ = b_.CreateStrncmp(left_string.first,

+                           left_string.second,

+                           right_string.first,

+                           right_string.second,

+                           len,

+                           inverse);

 }

 void CodegenLLVM::binop_buf(Binop &binop)

@@ -1334,7 +1342,12 @@ void CodegenLLVM::binop_buf(Binop &binop)

   size_t len = std::min(binop.left->type.GetSize(),

                         binop.right->type.GetSize());

-  expr_ = b_.CreateStrncmp(left_string, right_string, len, inverse);

+  expr_ = b_.CreateStrncmp(left_string,

+                           binop.left->type.GetSize(),

+                           right_string,

+                           binop.right->type.GetSize(),

+                           len,

+                           inverse);

 }

 void CodegenLLVM::binop_int(Binop &binop)

@@ -3528,9 +3541,8 @@ void CodegenLLVM::createIncDec(Unop &unop)

   else if (unop.expr->is_variable)

   {

     Variable &var = static_cast<Variable &>(*unop.expr);

-    Value *oldval = b_.CreateLoad(

-        variables_[var.ident]->getType()->getPointerElementType(),

-        variables_[var.ident]);

+    Value *oldval = b_.CreateLoad(variables_[var.ident]->getAllocatedType(),

+                                  variables_[var.ident]);

     Value *newval;

     if (is_increment)

       newval = b_.CreateAdd(oldval, b_.GetIntSameSize(step, oldval));

-- 

2.38.1