[libcxx-commits] [flang] [compiler-rt] [libc] [clang] [llvm] [libcxx] [Clang] Use correct base expression for counted_by field (#73168) (PR #73465)

Nick Desaulniers via libcxx-commits libcxx-commits at lists.llvm.org
Mon Nov 27 11:42:57 PST 2023


================
@@ -916,7 +916,7 @@ CodeGenFunction::emitFlexibleArrayMemberSize(const Expr *E, unsigned Type,
 
   // Build a load of the counted_by field.
   bool IsSigned = CountedByFD->getType()->isSignedIntegerType();
-  const Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD);
+  Expr *CountedByExpr = BuildCountedByFieldExpr(Base, CountedByFD);
----------------
nickdesaulniers wrote:

> for starters, we shouldn't be generating new AST nodes at codegen time

Yeah, I should have caught that before approving the initial implementation.  I think it would have been better to generate the corresponding GEP manually here.  Instead, Bill is synthesizing an expression via Expr which is why he's running into this const confusion.

i.e. if we have:
```c
struct annotated {
  int count;
  int array[] __counted_by(count);
};
int foo (struct annotated *a, size_t i) { return a->array[i]; }
```
the current implementation is turning that into:
```c
struct annotated {
  int count;
  int array[] __counted_by(count);
};
int foo (struct annotated *a, size_t i) {
  if (i < a->count)
    __builtin_ubsan_trap();
  return a->array[i];
}
```
because it's perhaps simpler/easier to generate `a->count` (for arbitrarily complex counted by definitions) and then rely on existing machinery to codegen the correct GEP.

But, I now believe we should have generated the GEP, and not synthesized the Expr.

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


More information about the libcxx-commits mailing list