[clang] [CodeGen] Revamp counted_by calculations (PR #70606)

via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 31 16:00:39 PDT 2023


================
@@ -859,53 +859,60 @@ CodeGenFunction::emitBuiltinObjectSize(const Expr *E, unsigned Type,
   }
 
   if (IsDynamic) {
-    LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
-        getLangOpts().getStrictFlexArraysLevel();
-    const Expr *Base = E->IgnoreParenImpCasts();
-
-    if (FieldDecl *FD = FindCountedByField(Base, StrictFlexArraysLevel)) {
-      const auto *ME = dyn_cast<MemberExpr>(Base);
-      llvm::Value *ObjectSize = nullptr;
-
-      if (!ME) {
-        const auto *DRE = dyn_cast<DeclRefExpr>(Base);
-        ValueDecl *VD = nullptr;
-
-        ObjectSize = ConstantInt::get(
-            ResType,
-            getContext().getTypeSize(DRE->getType()->getPointeeType()) / 8,
-            true);
-
-        if (auto *RD = DRE->getType()->getPointeeType()->getAsRecordDecl())
-          VD = RD->getLastField();
-
-        Expr *ICE = ImplicitCastExpr::Create(
-            getContext(), DRE->getType(), CK_LValueToRValue,
-            const_cast<Expr *>(cast<Expr>(DRE)), nullptr, VK_PRValue,
-            FPOptionsOverride());
-        ME = MemberExpr::CreateImplicit(getContext(), ICE, true, VD,
-                                        VD->getType(), VK_LValue, OK_Ordinary);
-      }
-
-      // At this point, we know that \p ME is a flexible array member.
-      const auto *ArrayTy = getContext().getAsArrayType(ME->getType());
+    // The code generated here calculates the size of a struct with a flexible
+    // array member that uses the counted_by attribute. There are two instances
+    // we handle:
+    //
+    //       struct s {
+    //         unsigned long flags;
+    //         int count;
+    //         int array[] __attribute__((counted_by(count)));
+    //       }
+    //
+    //   1) bdos of the flexible array itself:
+    //
+    //     __builtin_dynamic_object_size(p->array, 1) ==
+    //         p->count * sizeof(*p->array)
+    //
+    //   2) bdos of the whole struct, including the flexible array:
+    //
+    //     __builtin_dynamic_object_size(p, 1) ==
+    //        sizeof(*p) + p->count * sizeof(*p->array)
----------------
apple-fcloutier wrote:

Sorry, I don’t understand what we don’t mutually understand. In the same situation:

```
#include <string.h>

struct flex {
    double dummy;
    char c;
    char fam[__counted_by(c)];
};

struct flex f = { 123.0, 2, { 1, 2 } };

int main() {
    memset(&f, 0, sizeof(f) + f.c * sizeof(*f.fam));
}
```

We have an ASAN trap. Here are all the ways I can think of to reconcile this fact with the model that you propose:

* 2 is the wrong count value for a 2-element array;
* Clang is under-allocating for `f`;
* this is in-bounds and ASAN has a false positive;
* it's actually OK to write out of bounds when relying on `__builtin_dynamic_object_size(p, 1)`;
* the definition chosen for `__builtin_dynamic_object_size` is wrong.

The only one that makes sense to me is that the definition chosen for `__builtin_dynamic_object_size`. Which one is it to you? Do you see another way out?

https://github.com/llvm/llvm-project/pull/70606


More information about the cfe-commits mailing list