Commit 4e2ce228 authored by Yonghong Song's avatar Yonghong Song
Browse files

[BPF] Add preserve_access_index attribute for record definition 

This is a resubmission for the previous reverted commit
94343604 with the same subject. This commit fixed the
segfault issue and addressed additional review comments.

This patch introduced a new bpf specific attribute which can
be added to struct or union definition. For example,
  struct s { ... } __attribute__((preserve_access_index));
  union u { ... } __attribute__((preserve_access_index));
The goal is to simplify user codes for cases
where preserve access index happens for certain struct/union,
so user does not need to use clang __builtin_preserve_access_index
for every members.

The attribute has no effect if -g is not specified.

When the attribute is specified and -g is specified, any member
access defined by that structure or union, including array subscript
access and inner records, will be preserved through
  __builtin_preserve_{array,struct,union}_access_index()
IR intrinsics, which will enable relocation generation
in bpf backend.

The following is an example to illustrate the usage:
  -bash-4.4$ cat t.c
  #define __reloc__ __attribute__((preserve_access_index))
  struct s1 {
    int c;
  } __reloc__;

  struct s2 {
    union {
      struct s1 b[3];
    };
  } __reloc__;

  struct s3 {
    struct s2 a;
  } __reloc__;

  int test(struct s3 *arg) {
    return arg->a.b[2].c;
  }
  -bash-4.4$ clang -target bpf -g -S -O2 t.c

A relocation with access string "0:0:0:0:2:0" will be generated
representing access offset of arg->a.b[2].c.

forward declaration with attribute is also handled properly such
that the attribute is copied and populated in real record definition.

Differential Revision: https://reviews.llvm.org/D69759
parent e5f3760e

clang/include/clang/Basic/Attr.td

+8 −0
Original line number Diff line number Diff line
@@ -332,6 +332,7 @@ class TargetArch<list<string> arches> : TargetSpec { 
}

def TargetARM : TargetArch<["arm", "thumb", "armeb", "thumbeb"]>;

def TargetAVR : TargetArch<["avr"]>;

def TargetBPF : TargetArch<["bpfel", "bpfeb"]>;

def TargetMips32 : TargetArch<["mips", "mipsel"]>;

def TargetAnyMips : TargetArch<["mips", "mipsel", "mips64", "mips64el"]>;

def TargetMSP430 : TargetArch<["msp430"]>;
@@ -1578,6 +1579,13 @@ def AMDGPUNumVGPR : InheritableAttr { 
  let Subjects = SubjectList<[Function], ErrorDiag, "kernel functions">;

}



def BPFPreserveAccessIndex : InheritableAttr,

                             TargetSpecificAttr<TargetBPF>  {

  let Spellings = [Clang<"preserve_access_index">];

  let Subjects = SubjectList<[Record], ErrorDiag>;

  let Documentation = [BPFPreserveAccessIndexDocs];

}



def WebAssemblyImportModule : InheritableAttr,

                              TargetSpecificAttr<TargetWebAssembly> {

  let Spellings = [Clang<"import_module">];

clang/include/clang/Basic/AttrDocs.td

+11 −0
Original line number Diff line number Diff line
@@ -1634,6 +1634,17 @@ The semantics are as follows: 
  }];

}



def BPFPreserveAccessIndexDocs : Documentation {

  let Category = DocCatFunction;

  let Content = [{

Clang supports the ``__attribute__((preserve_access_index))``

attribute for the BPF target. This attribute may be attached to a

struct or union declaration, where if -g is specified, it enables

preserving struct or union member access debuginfo indicies of this

struct or union, similar to clang ``__builtin_preserve_acceess_index()``.

  }];

}



def MipsInterruptDocs : Documentation {

  let Category = DocCatFunction;

  let Heading = "interrupt (MIPS)";

clang/lib/CodeGen/CGExpr.cpp

+68 −7
Original line number Diff line number Diff line
@@ -3402,11 +3402,67 @@ static QualType getFixedSizeElementType(const ASTContext &ctx, 
  return eltType;

}



/// Given an array base, check whether its member access belongs to a record

/// with preserve_access_index attribute or not.

static bool IsPreserveAIArrayBase(CodeGenFunction &CGF, const Expr *ArrayBase) {

  if (!ArrayBase || !CGF.getDebugInfo())

    return false;



  const auto *ImplicitCast = dyn_cast<ImplicitCastExpr>(ArrayBase);

  if (!ImplicitCast)

    return false;



  // Only support base as either a MemberExpr or DeclRefExpr.

  // DeclRefExpr to cover cases like:

  //    struct s { int a; int b[10]; };

  //    struct s *p;

  //    p[1].a

  // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.

  // p->b[5] is a MemberExpr example.

  const Expr *E = ImplicitCast->getSubExpr();

  const auto *MemberCast = dyn_cast<MemberExpr>(E);

  if (MemberCast)

    return MemberCast->getMemberDecl()->hasAttr<BPFPreserveAccessIndexAttr>();



  const auto *DeclRefCast = dyn_cast<DeclRefExpr>(E);

  if (DeclRefCast) {

    const VarDecl *VarDef = dyn_cast<VarDecl>(DeclRefCast->getDecl());

    if (!VarDef)

      return false;



    const auto *PtrT = dyn_cast<PointerType>(VarDef->getType().getTypePtr());

    if (!PtrT)

      return false;

    const auto *PointeeT = PtrT->getPointeeType().getTypePtr();



    // Peel off typedef's

    const auto *TypedefT = dyn_cast<TypedefType>(PointeeT);

    while (TypedefT) {

      PointeeT = TypedefT->desugar().getTypePtr();

      TypedefT = dyn_cast<TypedefType>(PointeeT);

    }



    // Not a typedef any more, it should be an elaborated type.

    const auto ElaborateT = dyn_cast<ElaboratedType>(PointeeT);

    if (!ElaborateT)

      return false;



    const auto *RecT = dyn_cast<RecordType>(ElaborateT->desugar().getTypePtr());

    if (!RecT)

      return false;



    return RecT->getDecl()->hasAttr<BPFPreserveAccessIndexAttr>();

  }



  return false;

}



static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,

                                     ArrayRef<llvm::Value *> indices,

                                     QualType eltType, bool inbounds,

                                     bool signedIndices, SourceLocation loc,

                                     QualType *arrayType = nullptr,

                                     const Expr *Base = nullptr,

                                     const llvm::Twine &name = "arrayidx") {

  // All the indices except that last must be zero.

#ifndef NDEBUG
@@ -3428,7 +3484,8 @@ static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr, 


  llvm::Value *eltPtr;

  auto LastIndex = dyn_cast<llvm::ConstantInt>(indices.back());

  if (!CGF.IsInPreservedAIRegion || !LastIndex) {

  if (!LastIndex ||

      (!CGF.IsInPreservedAIRegion && !IsPreserveAIArrayBase(CGF, Base))) {

    eltPtr = emitArraySubscriptGEP(

        CGF, addr.getPointer(), indices, inbounds, signedIndices,

        loc, name);
@@ -3582,7 +3639,7 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E, 
    Addr = emitArraySubscriptGEP(

        *this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},

        E->getType(), !getLangOpts().isSignedOverflowDefined(), SignedIndices,

        E->getExprLoc(), &arrayType);

        E->getExprLoc(), &arrayType, E->getBase());

    EltBaseInfo = ArrayLV.getBaseInfo();

    EltTBAAInfo = CGM.getTBAAInfoForSubobject(ArrayLV, E->getType());

  } else {
@@ -3592,7 +3649,8 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E, 
    QualType ptrType = E->getBase()->getType();

    Addr = emitArraySubscriptGEP(*this, Addr, Idx, E->getType(),

                                 !getLangOpts().isSignedOverflowDefined(),

                                 SignedIndices, E->getExprLoc(), &ptrType);

                                 SignedIndices, E->getExprLoc(), &ptrType,

                                 E->getBase());

  }



  LValue LV = MakeAddrLValue(Addr, E->getType(), EltBaseInfo, EltTBAAInfo);
@@ -3993,12 +4051,13 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, 
    const CGBitFieldInfo &Info = RL.getBitFieldInfo(field);

    Address Addr = base.getAddress();

    unsigned Idx = RL.getLLVMFieldNo(field);

    if (!IsInPreservedAIRegion) {

    const RecordDecl *rec = field->getParent();

    if (!IsInPreservedAIRegion &&

        (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>())) {

      if (Idx != 0)

        // For structs, we GEP to the field that the record layout suggests.

        Addr = Builder.CreateStructGEP(Addr, Idx, field->getName());

    } else {

      const RecordDecl *rec = field->getParent();

      llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(

          getContext().getRecordType(rec), rec->getLocation());

      Addr = Builder.CreatePreserveStructAccessIndex(Addr, Idx,
@@ -4081,7 +4140,8 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, 
      addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),

                     addr.getAlignment());



    if (IsInPreservedAIRegion) {

    if (IsInPreservedAIRegion ||

        (getDebugInfo() && rec->hasAttr<BPFPreserveAccessIndexAttr>())) {

      // Remember the original union field index

      llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(

          getContext().getRecordType(rec), rec->getLocation());
@@ -4095,7 +4155,8 @@ LValue CodeGenFunction::EmitLValueForField(LValue base, 
      addr = Builder.CreateElementBitCast(

          addr, CGM.getTypes().ConvertTypeForMem(FieldType), field->getName());

  } else {

    if (!IsInPreservedAIRegion)

    if (!IsInPreservedAIRegion &&

        (!getDebugInfo() || !rec->hasAttr<BPFPreserveAccessIndexAttr>()))

      // For structs, we GEP to the field that the record layout suggests.

      addr = emitAddrOfFieldStorage(*this, addr, field);

    else

clang/lib/Sema/SemaDeclAttr.cpp

+29 −1
Original line number Diff line number Diff line
@@ -5700,6 +5700,25 @@ static void handleAVRSignalAttr(Sema &S, Decl *D, const ParsedAttr &AL) { 
  handleSimpleAttribute<AVRSignalAttr>(S, D, AL);

}



static void handleBPFPreserveAIRecord(Sema &S, RecordDecl *RD) {

  // Add preserve_access_index attribute to all fields and inner records.

  for (auto D : RD->decls()) {

    if (D->hasAttr<BPFPreserveAccessIndexAttr>())

      continue;



    D->addAttr(BPFPreserveAccessIndexAttr::CreateImplicit(S.Context));

    if (auto *Rec = dyn_cast<RecordDecl>(D))

      handleBPFPreserveAIRecord(S, Rec);

  }

}



static void handleBPFPreserveAccessIndexAttr(Sema &S, Decl *D,

    const ParsedAttr &AL) {

  auto *Rec = cast<RecordDecl>(D);

  handleBPFPreserveAIRecord(S, Rec);

  Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));

}



static void handleWebAssemblyImportModuleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

  if (!isFunctionOrMethod(D)) {

    S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
@@ -6576,6 +6595,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, 
  case ParsedAttr::AT_AVRSignal:

    handleAVRSignalAttr(S, D, AL);

    break;

  case ParsedAttr::AT_BPFPreserveAccessIndex:

    handleBPFPreserveAccessIndexAttr(S, D, AL);

    break;

  case ParsedAttr::AT_WebAssemblyImportModule:

    handleWebAssemblyImportModuleAttr(S, D, AL);

    break;
@@ -7325,7 +7347,8 @@ void Sema::ProcessDeclAttributeList(Scope *S, Decl *D, 
  }

}



// Helper for delayed processing TransparentUnion attribute.

// Helper for delayed processing TransparentUnion or BPFPreserveAccessIndexAttr

// attribute.

void Sema::ProcessDeclAttributeDelayed(Decl *D,

                                       const ParsedAttributesView &AttrList) {

  for (const ParsedAttr &AL : AttrList)
@@ -7333,6 +7356,11 @@ void Sema::ProcessDeclAttributeDelayed(Decl *D, 
      handleTransparentUnionAttr(*this, D, AL);

      break;

    }



  // For BPFPreserveAccessIndexAttr, we want to populate the attributes

  // to fields and inner records as well.

  if (D && D->hasAttr<BPFPreserveAccessIndexAttr>())

    handleBPFPreserveAIRecord(*this, cast<RecordDecl>(D));

}



// Annotation attributes are the only attributes allowed after an access

clang/test/CodeGen/bpf-attr-preserve-access-index-1.c

0 → 100644
+23 −0
Original line number Diff line number Diff line 
// REQUIRES: bpf-registered-target

// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s



#define __reloc__ __attribute__((preserve_access_index))



// test simple member access and initial struct with non-zero stride access

struct s1 {

  int a;

  union {

   int b;

   int c;

  };

} __reloc__;

typedef struct s1 __s1;



int test(__s1 *arg) {

  return arg->a + arg[1].b;

}



// CHECK: call i32* @llvm.preserve.struct.access.index.p0i32.p0s_struct.s1s(%struct.s1* %{{[0-9a-z]+}}, i32 0, i32 0)

// CHECK: call %struct.s1* @llvm.preserve.array.access.index.p0s_struct.s1s.p0s_struct.s1s(%struct.s1* %{{[0-9a-z]+}}, i32 0, i32 1)

// CHECK: call %union.anon* @llvm.preserve.struct.access.index.p0s_union.anons.p0s_struct.s1s(%struct.s1* %{{[0-9a-z]+}}, i32 1, i32 1)

// CHECK: call %union.anon* @llvm.preserve.union.access.index.p0s_union.anons.p0s_union.anons(%union.anon* %{{[0-9a-z]+}}, i32 0)