[BPF] Preserve debuginfo array/union/struct type/access index

[yonghong-song]

Summary

For background of BPF CO-RE project, please refer to

http://vger.kernel.org/bpfconf2019.html

In summary, BPF CO-RE intends to compile bpf programs
adjustable on struct/union layout change so the same
program can run on multiple kernels with adjustment
before loading based on native kernel structures.

In order to do this, we need keep track of GEP(getelementptr)
instruction base and result debuginfo types, so we
can adjust on the host based on kernel BTF info.
Capturing such information as an IR optimization is hard
as various optimization may have tweaked GEP and also
union is replaced by structure it is impossible to track
fieldindex for union member accesses.

Three intrinsic functions, preserve_{array,union,struct}_access_index,
are introducted.

addr = preserve_array_access_index(base, index, dimension)
addr = preserve_union_access_index(base, di_index)
addr = preserve_struct_access_index(base, gep_index, di_index)

here,

base: the base pointer for the array/union/struct access.
index: the last access index for array, the same for IR/DebugInfo layout.
dimension: the array dimension.
gep_index: the access index based on IR layout.
di_index: the access index based on user/debuginfo types.

If using these intrinsics blindly, i.e., transforming all GEPs
to these intrinsics and later on reducing them to GEPs, we have
seen up to 7% more instructions generated. To avoid such an overhead,
a clang builtin is proposed:

base = __builtin_preserve_access_index(base)

such that user wraps to-be-relocated GEPs in this builtin
and preserve_*_access_index intrinsics only apply to
those GEPs. Such a buyin will prevent performance degradation
if people do not use CO-RE, even for programs which use
bpf_probe_read().

For example, for the following example,

$ cat test.c
struct sk_buff {
   int i;
   int b1:1;
   int b2:2;
   union {
     struct {
       int o1;
       int o2;
     } o;
     struct {
       char flags;
       char dev_id;
     } dev;
     int netid;
   } u[10];
};

static int (*bpf_probe_read)(void *dst, int size, const void *unsafe_ptr)
    = (void *) 4;

#define _(x) (__builtin_preserve_access_index(x))

int bpf_prog(struct sk_buff *ctx) {
  char dev_id;
  bpf_probe_read(&dev_id, sizeof(char), _(&ctx->u[5].dev.dev_id));
  return dev_id;
}
$ clang -target bpf -O2 -g -emit-llvm -S -mllvm -print-before-all \
  test.c >& log

The generated IR looks like below:

...
define dso_local i32 @bpf_prog(%struct.sk_buff*) #0 !dbg !15 {
  %2 = alloca %struct.sk_buff*, align 8
  %3 = alloca i8, align 1
  store %struct.sk_buff* %0, %struct.sk_buff** %2, align 8, !tbaa !45
  call void @llvm.dbg.declare(metadata %struct.sk_buff** %2, metadata !43, metadata !DIExpression()), !dbg !49
  call void @llvm.lifetime.start.p0i8(i64 1, i8* %3) #4, !dbg !50
  call void @llvm.dbg.declare(metadata i8* %3, metadata !44, metadata !DIExpression()), !dbg !51
  %4 = load i32 (i8*, i32, i8*)*, i32 (i8*, i32, i8*)** @bpf_probe_read, align 8, !dbg !52, !tbaa !45
  %5 = load %struct.sk_buff*, %struct.sk_buff** %2, align 8, !dbg !53, !tbaa !45
  %6 = call [10 x %union.anon]* @llvm.preserve.struct.access.index.p0a10s_union.anons.p0s_struct.sk_buffs(
       %struct.sk_buff* %5, i32 2, i32 3), !dbg !53, !llvm.preserve.access.index !19
  %7 = call %union.anon* @llvm.preserve.array.access.index.p0s_union.anons.p0a10s_union.anons(
       [10 x %union.anon]* %6, i32 1, i32 5), !dbg !53
  %8 = call %union.anon* @llvm.preserve.union.access.index.p0s_union.anons.p0s_union.anons(
       %union.anon* %7, i32 1), !dbg !53, !llvm.preserve.access.index !26
  %9 = bitcast %union.anon* %8 to %struct.anon.0*, !dbg !53
  %10 = call i8* @llvm.preserve.struct.access.index.p0i8.p0s_struct.anon.0s(
       %struct.anon.0* %9, i32 1, i32 1), !dbg !53, !llvm.preserve.access.index !34
  %11 = call i32 %4(i8* %3, i32 1, i8* %10), !dbg !52
  %12 = load i8, i8* %3, align 1, !dbg !54, !tbaa !55
  %13 = sext i8 %12 to i32, !dbg !54
  call void @llvm.lifetime.end.p0i8(i64 1, i8* %3) #4, !dbg !56
  ret i32 %13, !dbg !57
}

!19 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "sk_buff", file: !3, line: 1, size: 704, elements: !20)
!26 = distinct !DICompositeType(tag: DW_TAG_union_type, scope: !19, file: !3, line: 5, size: 64, elements: !27)
!34 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !26, file: !3, line: 10, size: 16, elements: !35)

Note that @llvm.preserve.{struct,union}.access.index calls have metadata llvm.preserve.access.index
attached to instructions to provide struct/union debuginfo type information.

For &ctx->u[5].dev.dev_id,

. The "%6 = ..." represents struct member "u" with index 2 for IR layout and index 3 for DI layout.
. The "%7 = ..." represents array subscript "5".
. The "%8 = ..." represents union member "dev" with index 1 for DI layout.
. The "%10 = ..." represents struct member "dev_id" with index 1 for both IR and DI layout.

Basically, traversing the use-def chain recursively for the 3rd argument of bpf_probe_read() and
examining all preserve_*_access_index calls, the debuginfo struct/union/array access index
can be achieved.

The intrinsics also contain enough information to regenerate codes for IR layout.
For array and structure intrinsics, the proper GEP can be constructed.
For union intrinsics, replacing all uses of "addr" with "base" should be enough.

[lebedev.ri]

Tests seems to be missing?

[yonghong-song]

@lebedev.ri Thanks for the comment. This patch is not ready to land yet. Yes, tests are missing and I am going to add tests later.
More importantly, I want to get a sense whether what I am implementing here is the right direction or not.
The following two other patches are also related to describe our end goal:

https://reviews.llvm.org/D61810
https://reviews.llvm.org/D61524

[rsmith]

If I understand correctly, you want to be able to compile a program against some struct and union layouts, and then at load time "update" the program to cope with the actual layouts for those types being something else, but containing (at least) the set of members you know about. And your proposed approach is to add intrinsics into the IR that identify the GEPs that we emitted to model struct field accesses, but to otherwise not change the emitted IR. If so, I don't see how this approach to that problem can work. There seem to be a couple of problems:

%2 = alloca %struct.sk_buff*, align 8

The size you allocate here will presumably need to vary as the struct layout changes, and you have no way of knowing which allocas will need their sizes to be changed.

%6 = getelementptr inbounds %struct.sk_buff, %struct.sk_buff* %5, i32 0, i32 2, !dbg !54
%7 = call [10 x %union.anon]*
    @llvm.preserve.di.access.index.p0a10s_union.anons.p0a10s_union.anons.p0s_struct.sk_buffs(
    [10 x %union.anon]* %6, %struct.sk_buff* %5,
    i8* getelementptr inbounds ([8 x i8], [8 x i8]* @0, i32 0, i32 0), i32 3), !dbg !54

If this call is supposed to somehow represent that the %6 GEP is computed from %5 by way of a struct field access, this will not be robust against any kind of optimization: optimizations will happily rewrite uses of %6 to use %5 or some other form directly, and you will have lost track of the pointers you need to update.

It would seem better to me to represent a relocateable GEP directly: that is, instead of emitting a regular GEP and some fixup intrinsic, you could emit an intrinsic call instead of the GEP, and have the intrinsic compute the field offset. (And you'll need to do something about your allocas, such as using an intrinsic to get the struct size and emitting a dynamic alloca of that size.) For example, instead of the above, you could emit

%6 = call i8* @llvm.preserve.di.access.gep.2(i8* %5, i32 0, i32 2, !typeinfo)

(where !typeinfo is whatever information you need to identify struct sk_buff later on, when generating your relocations, and the indices you pass in are whichever ones you need to identify the subobject within that type information). Does that make sense?

[efriedma]

The size you allocate here will presumably need to vary as the struct layout changes, and you have no way of knowing which allocas will need their sizes to be changed.

Your example is just a pointer; the size of a pointer won't change.

That said, yes, address computation isn't the only place C cares about the layout of a struct. In particular variables (local or global) of struct type, sizeof(), and offsetof() need different handling. But I'm not sure how much any of those matter for BPF.

If this call is supposed to somehow represent that the %6 GEP is computed from %5 by way of a struct field access, this will not be robust against any kind of optimization: optimizations will happily rewrite uses of %6 to use %5 or some other form directly, and you will have lost track of the pointers you need to update.

I think actually, the idea is that the intrinsic represents the computation of some additional adjustment, and so the following instructions use %7, the result of the call, not %6. So I think the existing formulation is sound.

That said, it's awkward to have both an instruction and an intrinsic representing parts of the same offset computation; it probably makes sense to emit just the intrinsic, instead of both the intrinsic and the GEP, like you suggest.

[yonghong-song]

@rsmith @eli.friedman Thanks for your comments. I fully agree that it seems awkward that we have both GEP and intrinsic generation. I will try to do some experiments here to only have intrinsic generation. My only concern is possible performance degradation. I will post here once I got concrete implementation and numbers.

@rsmith regarding to your concerns about struct size change. Yes, the structure size may indeed change. Currently, we plan only to support field fields (non-struct/union type with 1/2/4/8 bytes, and strings). These are most common use cases. Let me explain a little bit more.

Here, we are targeting the bpf_probe_read (https://github.com/torvalds/linux/blob/master/include/uapi/linux/bpf.h#L532-L537)

int bpf_probe_read(void *dst, u32 size, const void *src)

which tries to copy some data from kernel (src pointer) to the stack of the bpf program.

Here, if "src" points to a structure, and user intends to copy the whole structure out of kernel, then "size" may need to be different for different kernels if different kernels indeed have different structure size.
But first, in all our use cases, users typically do not read the whole structure. Second, if "size" does change, we request users to use multiple versioning (using patchable global variables) with possibly different "size" values for different kernel versions.
Note that "size" must be constant for bpf verifier to succeed.
(https://github.com/torvalds/linux/blob/master/kernel/trace/bpf_trace.c#L138-L156)

Just gave another two examples here about reading kernel memories in iovisor/bcc. Note that
bcc will transform certain pointer access "b->c" to "bpf_probe_read(&dest, size, &b->c)" internally.

The bcc tool biosnoop.py contain several bpf probe read:

https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L110
      data.len = req->__data_len;
https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L117
      data.len = req->__data_len;
https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L118
      data.sector = req->__sector;
https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L120
      struct gendisk *rq_disk = req->rq_disk;
https://github.com/iovisor/bcc/blob/master/tools/biosnoop.py#L121-L122
     bpf_probe_read(&data.disk_name, sizeof(data.disk_name),
                     rq_disk->disk_name);

They are all primitive types and strings.

Another example for bcc tool tcpconnect.py

https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L128
  u16 dport = skp->__sk_common.skc_dport;
https://github.com/iovisor/bcc/blob/master/tools/tcpconnect.py#L136-L137
      data4.saddr = skp->__sk_common.skc_rcv_saddr;
      data4.daddr = skp->__sk_common.skc_daddr;

.....

In summary, right now, the to-be-read kernel memory size (through a specific bpf_probe_read() call)
won't change between different kernel versions. So we do not need to handle structure allocation size change here.

[yonghong-song]

do not generate gep any more, only intrinsics

[yonghong-song]

@rsmith @eli.friedman Hi, I just implemented a new version which generates intrinsics only. Three intrinsics are implemented for array/union/structure separately as their handling and parameters are different. Could you take a look? Thanks!

[efriedma]

lib/CodeGen/CGExpr.cpp

3406

I don't think there's any reason to expect the array subscript is a ConstantInt here?

[yonghong-song]

lib/CodeGen/CGExpr.cpp

3406

right.

[yonghong-song]

array index may not be constant and remove inst_name

[yonghong-song]

add clang builtin __builtin_preserve_access_index

[lebedev.ri]

Don't think i will be of any help here.

test/CodeGen/bpf-offsetreloc.c

2–4

This looks like a bad test.
Can't you use FileCheck?
I'd think there is more to test than a single tiny test..

[yonghong-song]

test/CodeGen/bpf-offsetreloc.c

2–4

Thanks. I will add more tests with FileCheck once we get an agreement for the general approach.

[ast]

from kernel, libbpf, tooling and user experience points of view this approach looks the best to me.

[yonghong-song]

@rsmith @eli.friedman Do you have any comments on the clang intrinsic interface in this patch and the llvm intrinsics interface at https://reviews.llvm.org/D61810?

[yonghong-song]

remove bpf offsetreloc option and use FileCheck in the test.

[ast]

lgtm

[yonghong-song]

@rsmith @eli.friedman Ping again. Do you have any comments on my proposed clang/IR intrinsics? Thanks!

[efriedma]

The general idea of restricting the intrinsics to specific contexts makes sense. I'm not sure it makes sense to mark expressions, as opposed to types, though; can we really expect the user to know which expressions to apply this to?

I'd like to see an actual specification for this in docs/LanguageExtensions.rst at some point.

Other than that, generally seems okay.

lib/CodeGen/CGExpr.cpp

663

I'm not sure you can use getParents like this safely... it's not really meant for use inside of clang semantic analysis/code generation, and I don't think we recompute it as the AST changes.

[yonghong-song]

@eli.friedman Sorry for replying late. I am outside US and currently in PTO. Will back to US soon to address your comments.

can we really expect the user to know which expressions to apply this to?

Yes, this is specifically targeting some bpf helper calls like bpf_probe_read. So users know which expressions to apply.

I'd like to see an actual specification for this in docs/LanguageExtensions.rst at some point.

I will find a place to put this into docs/LanguageExtensions.rst.

lib/CodeGen/CGExpr.cpp

663

Good point. Let me check whether I can traverse AST instead.

[yonghong-song]

do not use ctx.getParents() to check whether a GEP candidate inside a preserve_access_index. instead, mark the region upfront. Add the new clang intrinsic into the language doc.

[yonghong-song]

@eli.friedman I removed the usage of astcontext getParents() stuff. Instead, I mark the region upfront. I also added the intrinsic __builtin_preserve_access_index() into clang
lang extention doc. Could you take a look at new patch? Thanks!

[yonghong-song]

@eli.friedman Hi, Just ping. I have removed getParents() for ASTContext and added description of the new intrinsic in language doc. Could you take a look? Thanks!

[efriedma]

The new approach to tracking expressions inside of __builtin_preserve_access_index seems okay.

Please let @rsmith comment since he looked at this before.

docs/LanguageExtensions.rst

1958 ↗

(On Diff #206274)

"preserved with the IR intrinsics" isn't really useful; this is the user's manual, not a developer guide to LLVM internals. Probably better to say what it enables from the user's perspective: the CO-RE feature for BPF targets.

1960 ↗

(On Diff #206274)

I would rather not have __builtin_preserve_access_index fail to do anything when debug info is disabled. If it's hard to fix, making it a hard error is probably okay.

[yonghong-song]

@rsmith I have proposed one clang intrinsic and three IR intrinsics for CO-RE. Could you take a look and share your opinions as well? Thanks!

docs/LanguageExtensions.rst

1958 ↗

(On Diff #206274)

Will reword to be more towards users in the next revision.

1960 ↗

(On Diff #206274)

The IR intrinsics needs to have debuginfo as the metadata so that the user-level access info can be reconstructed. If no debug info, just IR intrinsics without debuginfo is less useful. So let me make a hard error.

We can relax it later if IR intrinsics without deebuginfo metadata becomes workable/useful.

[yonghong-song]

reword the lang doc for builtin_preserve_access_index() to be more user focused
treat using builtin_preserve_access_index() without -g or in nested way as compiler errors

[yonghong-song]

change void *__builtin_preserve_access_index(void *) to const void * _builtin_preserve_access_index(const void *).

[yonghong-song]

handle unnamed bitfield properly. these struct/union members are not encoded in debuginfo, so skip them during preserve_*_access_index IR intrinsics generation.

[yonghong-song]

@rsmith Just ping, could you take a look at the clang change? If possible, could you share your opinion? Thanks!

[yonghong-song]

@rsmith Ping again, do you have any comments on this patch? Thanks!

[jdoerfert]

The test coverage here is not acceptable:

1. The command line of the tests is far from what it should be (see other tests).
2. The check lines do little to prevent regressions in the future.
3. There are no test for wrong usage, errors, ...

Also, it seems these patches were reviewed without the respective commits list as a subscriber. Please add these for any future code review.

[yonghong-song]

@jdoerfert Thanks for comments. I will submit another patch to add adequate tests for clang frontend as you suggested.

[yonghong-song]

The followup patch for this change is at https://reviews.llvm.org/D64598.

[yonghong-song]

abandon this one. A revised version with most tests has been merged.

include/clang/Basic/Builtins.def

 BUILTIN(__builtin_smulll_overflow, "bSLLiCSLLiCSLLi*", "n")
 
 // Clang builtins (not available in GCC).
 BUILTIN(__builtin_addressof, "v*v&", "nct")
 BUILTIN(__builtin_operator_new, "v*z", "tc")
 BUILTIN(__builtin_operator_delete, "vv*", "tn")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 BUILTIN(__builtin_dump_struct, "ivC*v*", "tn")
+BUILTIN(__builtin_preserve_access_index, "v*v*", "nc")
 
 // Safestack builtins
 BUILTIN(__builtin___get_unsafe_stack_start, "v*", "Fn")
 BUILTIN(__builtin___get_unsafe_stack_bottom, "v*", "Fn")
 BUILTIN(__builtin___get_unsafe_stack_top, "v*", "Fn")
 BUILTIN(__builtin___get_unsafe_stack_ptr, "v*", "Fn")
 
 // Nontemporal loads/stores builtins

lib/Basic/Targets/BPF.h

BPFTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
     MaxAtomicInlineWidth = 64;
     TLSSupported = false;
   }
 
   void getTargetDefines(const LangOptions &Opts,
                         MacroBuilder &Builder) const override;
 
   bool hasFeature(StringRef Feature) const override {
-    return Feature == "bpf" || Feature == "alu32" || Feature == "dwarfris";
+    return Feature == "bpf" || Feature == "alu32" || Feature == "dwarfris" ||
+           Feature == "checkoffsetreloc";
   }
 
   void setFeatureEnabled(llvm::StringMap<bool> &Features, StringRef Name,
                          bool Enabled) const override {
     Features[Name] = Enabled;
   }
 
   ArrayRef<Builtin::Info> getTargetBuiltins() const override { return None; }

lib/CodeGen/CGBuilder.h

public:
   }
 
   using CGBuilderBaseTy::CreateMemSet;
   llvm::CallInst *CreateMemSet(Address Dest, llvm::Value *Value,
                                llvm::Value *Size, bool IsVolatile = false) {
     return CreateMemSet(Dest.getPointer(), Value, Size,
                         Dest.getAlignment().getQuantity(), IsVolatile);
   }
+
+  using CGBuilderBaseTy::CreatePreserveStructAccessIndex;
+  Address CreatePreserveStructAccessIndex(Address Addr,
+                                          unsigned Index,
+                                          unsigned FieldIndex,
+                                          llvm::MDNode *DbgInfo) {
+    llvm::StructType *ElTy = cast<llvm::StructType>(Addr.getElementType());
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    const llvm::StructLayout *Layout = DL.getStructLayout(ElTy);
+    auto Offset = CharUnits::fromQuantity(Layout->getElementOffset(Index));
+
+    return Address(CreatePreserveStructAccessIndex(Addr.getPointer(),
+                                                   Index, FieldIndex, DbgInfo),
+                   Addr.getAlignment().alignmentAtOffset(Offset));
+  }
 };
 
 }  // end namespace CodeGen
 }  // end namespace clang
 
 #endif

lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_dump_struct: {
     QualType Arg0Type = Arg0->getType()->getPointeeType();
 
     Value *RecordPtr = EmitScalarExpr(Arg0);
     Value *Res = dumpRecord(*this, Arg0Type, RecordPtr, Arg0Align,
                             {LLVMFuncType, Func}, 0);
     return RValue::get(Res);
   }
 
+  case Builtin::BI__builtin_preserve_access_index: {
+    Value *Res = EmitScalarExpr(E->getArg(0));
+    return RValue::get(Res);
+  }
+
   case Builtin::BI__builtin_cimag:
   case Builtin::BI__builtin_cimagf:
   case Builtin::BI__builtin_cimagl:
   case Builtin::BIcimag:
   case Builtin::BIcimagf:
   case Builtin::BIcimagl: {
     ComplexPairTy ComplexVal = EmitComplexExpr(E->getArg(0));
     return RValue::get(ComplexVal.second);

lib/CodeGen/CGExpr.cpp

 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/ConvertUTF.h"

 
 bool CodeGenFunction::sanitizePerformTypeCheck() const {
   return SanOpts.has(SanitizerKind::Null) |
          SanOpts.has(SanitizerKind::Alignment) |
          SanOpts.has(SanitizerKind::ObjectSize) |
          SanOpts.has(SanitizerKind::Vptr);
 }
 
+/// The expression E is a candidate for preserving debuginfo access index
+/// if it is inside an __builtin_preserve_access_index intrinsic call.
+bool CodeGenFunction::isPreserveDIAccessIndexNeeded(const Expr *E) {
+  if (!E)
+    return false;
+
+  if (!getDebugInfo())
+    return false;
+
+  while (true) {
+    const auto &Parents = getContext().getParents(*E);
+    if (Parents.size() != 1)
+      return false;
+
+    E = Parents[0].get<Expr>();
+    if (!E)
+      return false;
+
+    // Check whether E is a BI__builtin_preserve_access_index
+    // intrinsic call.
+    const auto *CE = dyn_cast<CallExpr>(E);
+    if (CE) {
+      // Callee must a builtin function.
+      const Expr *Callee = CE->getCallee()->IgnoreParens();
+      auto ICE = dyn_cast<ImplicitCastExpr>(Callee);
+      if (!ICE)
+        return false;
+      if (ICE->getCastKind() != CK_BuiltinFnToFnPtr)
+        return false;
+
+      auto DRE = dyn_cast<DeclRefExpr>(ICE->getSubExpr());
+      if (!DRE)
+        return false;
+
+      if (auto FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
+        if (FD->getBuiltinID() == Builtin::BI__builtin_preserve_access_index)
+          return true;
+      }
+
+      break;
+    }
+  }
+
+  return false;
+}
+
 void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
                                     llvm::Value *Ptr, QualType Ty,
                                     CharUnits Alignment,
                                     SanitizerSet SkippedChecks,
                                     llvm::Value *ArraySize) {
   if (!sanitizePerformTypeCheck())
     return;
 
do {
     eltType = vla->getElementType();
   } while ((vla = ctx.getAsVariableArrayType(eltType)));
   return eltType;
 }
 
 static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
                                      ArrayRef<llvm::Value *> indices,
                                      QualType eltType, bool inbounds,
-                                     bool signedIndices, SourceLocation loc,
+                                     bool signedIndices, const Expr *E,
                                      const llvm::Twine &name = "arrayidx") {
   // All the indices except that last must be zero.
 #ifndef NDEBUG
   for (auto idx : indices.drop_back())
     assert(isa<llvm::ConstantInt>(idx) &&
            cast<llvm::ConstantInt>(idx)->isZero());
 #endif
 
   // Determine the element size of the statically-sized base.  This is
   // the thing that the indices are expressed in terms of.
   if (auto vla = CGF.getContext().getAsVariableArrayType(eltType)) {
     eltType = getFixedSizeElementType(CGF.getContext(), vla);
   }
 
   // We can use that to compute the best alignment of the element.
   CharUnits eltSize = CGF.getContext().getTypeSizeInChars(eltType);
   CharUnits eltAlign =
     getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-      CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast<llvm::ConstantInt>(indices.back());
+  if (!CGF.isPreserveDIAccessIndexNeeded(E) || !LastIndex) {
+    eltPtr = emitArraySubscriptGEP(
+        CGF, addr.getPointer(), indices, inbounds, signedIndices,
+        E->getExprLoc(), name);
+  } else {
+    // Remember the original array subscript for bpf target
+    unsigned idx = LastIndex->getZExtValue();
+    eltPtr = CGF.Builder.CreatePreserveArrayAccessIndex(addr.getPointer(),
+                                                        indices.size() - 1,
+                                                        idx);
+  }
+
   return Address(eltPtr, eltAlign);
 }
 
 LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,
                                                bool Accessed) {
   // The index must always be an integer, which is not an aggregate.  Emit it
   // in lexical order (this complexity is, sadly, required by C++17).
   llvm::Value *IdxPre =
LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,
   // Handle the extvector case we ignored above.
   if (isa<ExtVectorElementExpr>(E->getBase())) {
     LValue LV = EmitLValue(E->getBase());
     auto *Idx = EmitIdxAfterBase(/*Promote*/true);
     Address Addr = EmitExtVectorElementLValue(LV);
 
     QualType EltType = LV.getType()->castAs<VectorType>()->getElementType();
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true,
-                                 SignedIndices, E->getExprLoc());
+                                 SignedIndices, E);
     return MakeAddrLValue(Addr, EltType, LV.getBaseInfo(),
                           CGM.getTBAAInfoForSubobject(LV, EltType));
   }
 
   LValueBaseInfo EltBaseInfo;
   TBAAAccessInfo EltTBAAInfo;
   Address Addr = Address::invalid();
   if (const VariableArrayType *vla =
if (const VariableArrayType *vla =
     if (getLangOpts().isSignedOverflowDefined()) {
       Idx = Builder.CreateMul(Idx, numElements);
     } else {
       Idx = Builder.CreateNSWMul(Idx, numElements);
     }
 
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, vla->getElementType(),
                                  !getLangOpts().isSignedOverflowDefined(),
-                                 SignedIndices, E->getExprLoc());
+                                 SignedIndices, E);
 
   } else if (const ObjCObjectType *OIT = E->getType()->getAs<ObjCObjectType>()){
     // Indexing over an interface, as in "NSString *P; P[4];"
 
     // Emit the base pointer.
     Addr = EmitPointerWithAlignment(E->getBase(), &EltBaseInfo, &EltTBAAInfo);
     auto *Idx = EmitIdxAfterBase(/*Promote*/true);
 
if (const VariableArrayType *vla =
     else
       ArrayLV = EmitLValue(Array);
     auto *Idx = EmitIdxAfterBase(/*Promote*/true);
 
     // Propagate the alignment from the array itself to the result.
     Addr = emitArraySubscriptGEP(
         *this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},
         E->getType(), !getLangOpts().isSignedOverflowDefined(), SignedIndices,
-        E->getExprLoc());
+        E);
     EltBaseInfo = ArrayLV.getBaseInfo();
     EltTBAAInfo = CGM.getTBAAInfoForSubobject(ArrayLV, E->getType());
   } else {
     // The base must be a pointer; emit it with an estimate of its alignment.
     Addr = EmitPointerWithAlignment(E->getBase(), &EltBaseInfo, &EltTBAAInfo);
     auto *Idx = EmitIdxAfterBase(/*Promote*/true);
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, E->getType(),
                                  !getLangOpts().isSignedOverflowDefined(),
-                                 SignedIndices, E->getExprLoc());
+                                 SignedIndices, E);
   }
 
   LValue LV = MakeAddrLValue(Addr, E->getType(), EltBaseInfo, EltTBAAInfo);
 
   if (getLangOpts().ObjC &&
       getLangOpts().getGC() != LangOptions::NonGC) {
     LV.setNonGC(!E->isOBJCGCCandidate(getContext()));
     setObjCGCLValueClass(getContext(), E, LV);
if (auto *VLA = getContext().getAsVariableArrayType(ResultExprTy)) {
     // signed-overflow, so we use the same semantics for our explicit
     // multiply.  We suppress this if overflow is not undefined behavior.
     if (getLangOpts().isSignedOverflowDefined())
       Idx = Builder.CreateMul(Idx, NumElements);
     else
       Idx = Builder.CreateNSWMul(Idx, NumElements);
     EltPtr = emitArraySubscriptGEP(*this, Base, Idx, VLA->getElementType(),
                                    !getLangOpts().isSignedOverflowDefined(),
-                                   /*SignedIndices=*/false, E->getExprLoc());
+                                   /*SignedIndices=*/false, E);
   } else if (const Expr *Array = isSimpleArrayDecayOperand(E->getBase())) {
     // If this is A[i] where A is an array, the frontend will have decayed the
     // base to be a ArrayToPointerDecay implicit cast.  While correct, it is
     // inefficient at -O0 to emit a "gep A, 0, 0" when codegen'ing it, then a
     // "gep x, i" here.  Emit one "gep A, 0, i".
     assert(Array->getType()->isArrayType() &&
            "Array to pointer decay must have array source type!");
     LValue ArrayLV;
     // For simple multidimensional array indexing, set the 'accessed' flag for
     // better bounds-checking of the base expression.
     if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Array))
       ArrayLV = EmitArraySubscriptExpr(ASE, /*Accessed*/ true);
     else
       ArrayLV = EmitLValue(Array);
 
     // Propagate the alignment from the array itself to the result.
     EltPtr = emitArraySubscriptGEP(
         *this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},
         ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
-        /*SignedIndices=*/false, E->getExprLoc());
+        /*SignedIndices=*/false, E);
     BaseInfo = ArrayLV.getBaseInfo();
     TBAAInfo = CGM.getTBAAInfoForSubobject(ArrayLV, ResultExprTy);
   } else {
     Address Base = emitOMPArraySectionBase(*this, E->getBase(), BaseInfo,
                                            TBAAInfo, BaseTy, ResultExprTy,
                                            IsLowerBound);
     EltPtr = emitArraySubscriptGEP(*this, Base, Idx, ResultExprTy,
                                    !getLangOpts().isSignedOverflowDefined(),
-                                   /*SignedIndices=*/false, E->getExprLoc());
+                                   /*SignedIndices=*/false, E);
   }
 
   return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo, TBAAInfo);
 }
 
 LValue CodeGenFunction::
 EmitExtVectorElementExpr(const ExtVectorElementExpr *E) {
   // Emit the base vector as an l-value.
if (E->isArrow()) {
     EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
                   /*Alignment=*/CharUnits::Zero(), SkippedChecks);
     BaseLV = MakeAddrLValue(Addr, PtrTy, BaseInfo, TBAAInfo);
   } else
     BaseLV = EmitCheckedLValue(BaseExpr, TCK_MemberAccess);
 
   NamedDecl *ND = E->getMemberDecl();
   if (auto *Field = dyn_cast<FieldDecl>(ND)) {
-    LValue LV = EmitLValueForField(BaseLV, Field);
+    LValue LV = EmitLValueForField(BaseLV, Field, E);
     setObjCGCLValueClass(getContext(), E, LV);
     return LV;
   }
 
   if (const auto *FD = dyn_cast<FunctionDecl>(ND))
     return EmitFunctionDeclLValue(*this, E, FD);
 
   llvm_unreachable("Unhandled member declaration!");
static Address emitAddrOfFieldStorage(CodeGenFunction &CGF, Address base,
   const RecordDecl *rec = field->getParent();
 
   unsigned idx =
     CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
 
   return CGF.Builder.CreateStructGEP(base, idx, field->getName());
 }
 
+static Address emitPreserveStructAccess(CodeGenFunction &CGF, Address base,
+                                        const FieldDecl *field) {
+  const RecordDecl *rec = field->getParent();
+  llvm::DIType *DbgInfo = CGF.getDebugInfo()->getOrCreateRecordType(
+      CGF.getContext().getRecordType(rec), rec->getLocation());
+
+  unsigned idx =
+      CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
+
+  return CGF.Builder.CreatePreserveStructAccessIndex(
+      base, idx, field->getFieldIndex(), DbgInfo);
+}
+
 static bool hasAnyVptr(const QualType Type, const ASTContext &Context) {
   const auto *RD = Type.getTypePtr()->getAsCXXRecordDecl();
   if (!RD)
     return false;
 
   if (RD->isDynamicClass())
     return true;
 
   for (const auto &Base : RD->bases())
     if (hasAnyVptr(Base.getType(), Context))
       return true;
 
   for (const FieldDecl *Field : RD->fields())
     if (hasAnyVptr(Field->getType(), Context))
       return true;
 
   return false;
 }
 
 LValue CodeGenFunction::EmitLValueForField(LValue base,
-                                           const FieldDecl *field) {
+                                           const FieldDecl *field,
+                                           const Expr *E) {
   LValueBaseInfo BaseInfo = base.getBaseInfo();
 
   if (field->isBitField()) {
     const CGRecordLayout &RL =
       CGM.getTypes().getCGRecordLayout(field->getParent());
     const CGBitFieldInfo &Info = RL.getBitFieldInfo(field);
     Address Addr = base.getAddress();
     unsigned Idx = RL.getLLVMFieldNo(field);
if (rec->isUnion()) {
     // For unions, there is no pointer adjustment.
     assert(!FieldType->isReferenceType() && "union has reference member");
     if (CGM.getCodeGenOpts().StrictVTablePointers &&
         hasAnyVptr(FieldType, getContext()))
       // Because unions can easily skip invariant.barriers, we need to add
       // a barrier every time CXXRecord field with vptr is referenced.
       addr = Address(Builder.CreateLaunderInvariantGroup(addr.getPointer()),
                      addr.getAlignment());
+
+    if (isPreserveDIAccessIndexNeeded(E)) {
+      // Remember the original union field index
+      llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateRecordType(
+          getContext().getRecordType(rec), rec->getLocation());
+      addr = Address(
+          Builder.CreatePreserveUnionAccessIndex(
+              addr.getPointer(), field->getFieldIndex(), DbgInfo),
+          addr.getAlignment());
+    }
   } else {
+
+    if (!isPreserveDIAccessIndexNeeded(E))
       // For structs, we GEP to the field that the record layout suggests.
       addr = emitAddrOfFieldStorage(*this, addr, field);
+    else
+      // Remember the original struct field index
+      addr = emitPreserveStructAccess(*this, addr, field);
 
     // If this is a reference field, load the reference right now.
     if (FieldType->isReferenceType()) {
       LValue RefLVal = MakeAddrLValue(addr, FieldType, FieldBaseInfo,
                                       FieldTBAAInfo);
       if (RecordCVR & Qualifiers::Volatile)
         RefLVal.getQuals().addVolatile();
       addr = EmitLoadOfReference(RefLVal, &FieldBaseInfo, &FieldTBAAInfo);

lib/CodeGen/CodeGenFunction.h

public:
 
   void setBeforeOutermostConditional(llvm::Value *value, Address addr) {
     assert(isInConditionalBranch());
     llvm::BasicBlock *block = OutermostConditional->getStartingBlock();
     auto store = new llvm::StoreInst(value, addr.getPointer(), &block->back());
     store->setAlignment(addr.getAlignment().getQuantity());
   }
 
+  /// isPreserveDIAccessIndexNeeded - Return true if it is needed to
+  /// preserve the Debuginfo access index.
+  bool isPreserveDIAccessIndexNeeded(const Expr *E);
+
   /// An RAII object to record that we're evaluating a statement
   /// expression.
   class StmtExprEvaluation {
     CodeGenFunction &CGF;
 
     /// We have to save the outermost conditional: cleanups in a
     /// statement expression aren't conditional just because the
     /// StmtExpr is.
public:
   llvm::Value *emitScalarConstant(const ConstantEmission &Constant, Expr *E);
 
   RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e,
                                 AggValueSlot slot = AggValueSlot::ignored());
   LValue EmitPseudoObjectLValue(const PseudoObjectExpr *e);
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
                               const ObjCIvarDecl *Ivar);
-  LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
+  LValue EmitLValueForField(LValue Base, const FieldDecl* Field,
+                            const Expr *E = nullptr);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
   /// EmitLValueForFieldInitialization - Like EmitLValueForField, except that
   /// if the Field is a reference, this will return the address of the reference
   /// and not the address of the value stored in the reference.
   LValue EmitLValueForFieldInitialization(LValue Base,
                                           const FieldDecl* Field);
 

lib/Sema/SemaChecking.cpp

static bool SemaBuiltinAddressof(Sema &S, CallExpr *TheCall) {
   if (ResultType.isNull())
     return true;
 
   TheCall->setArg(0, Arg.get());
   TheCall->setType(ResultType);
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+    return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
     return true;
 
   // First two arguments should be integers.
   for (unsigned I = 0; I < 2; ++I) {
     ExprResult Arg = TheCall->getArg(I);
     QualType Ty = Arg.get()->getType();
if (const auto *FT = dyn_cast<FunctionProtoType>(FuncType)) {
             << 2 << FnPtrArgType << "'int (*)(const char *, ...)'";
         return ExprError();
       }
     }
 
     TheCall->setType(Context.IntTy);
     break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+    if (SemaBuiltinPreserveAI(*this, TheCall))
+      return ExprError();
+    break;
   case Builtin::BI__builtin_call_with_static_chain:
     if (SemaBuiltinCallWithStaticChain(*this, TheCall))
       return ExprError();
     break;
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
     if (SemaBuiltinSEHScopeCheck(*this, TheCall, Scope::SEHExceptScope,
                                  diag::err_seh___except_block))

test/CodeGen/bpf-offsetreloc.c

+// RUN: %clang -target bpf -emit-llvm -S -g -O2 -o - %s > %t1
+// RUN: grep "llvm.preserve.struct.access.index" %t1
+// RUN: grep "llvm.preserve.array.access.index" %t1
+// RUN: grep "llvm.preserve.union.access.index" %t1
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+void *test(struct t *arg) {
+  return _(&arg->c[3].b);
+}