Re: [PATCH] Perform calculation of array size in chars instead of bits to prevent an assertion.

[Richard Smith]

On Fri, May 10, 2013 at 7:35 PM, Richard Trieu <rtr...@google.com> wrote:

> For large arrays, the size calculation in bits may overflow a uint64_t.
>  If only the size in chars is important, perform the calculation in chars
> to prevent this overflow which triggers an assertion.  This fixes PR15216.
>
> http://llvm-reviews.chandlerc.com/D781
>
> Files:
>   test/Sema/offsetof.c
>   include/clang/AST/ASTContext.h
>   lib/AST/ASTContext.cpp
>
> Index: test/Sema/offsetof.c
> ===================================================================
> --- test/Sema/offsetof.c
> +++ test/Sema/offsetof.c
> @@ -69,3 +69,15 @@
>  int test5() {
>    return __builtin_offsetof(Array, array[*(int*)0]); //
> expected-warning{{indirection of non-volatile null pointer}}
> expected-note{{__builtin_trap}}
>  }
> +
> +// PR15216
> +// Don't crash on when taking computing the offset of structs with large
> arrays.
> +const unsigned long Size = (1l << 62);
> +
> +struct Chunk {
> +  char padding[Size];
> +  char data;
> +};
> +
> +int test6 = __builtin_offsetof(struct Chunk, data);
> +
> Index: include/clang/AST/ASTContext.h
> ===================================================================
> --- include/clang/AST/ASTContext.h
> +++ include/clang/AST/ASTContext.h
> @@ -1591,6 +1591,9 @@
>    std::pair<CharUnits, CharUnits> getTypeInfoInChars(const Type *T) const;
>    std::pair<CharUnits, CharUnits> getTypeInfoInChars(QualType T) const;
>
> +  std::pair<CharUnits, CharUnits> getConstantArrayInfoInChars(
> +      const ConstantArrayType* CAT) const;
> +
>    /// \brief Return the "preferred" alignment of the specified type \p T
> for
>    /// the current target, in bits.
>    ///
> Index: lib/AST/ASTContext.cpp
> ===================================================================
> --- lib/AST/ASTContext.cpp
> +++ lib/AST/ASTContext.cpp
> @@ -1345,11 +1345,28 @@
>
>  std::pair<CharUnits, CharUnits>
>  ASTContext::getTypeInfoInChars(const Type *T) const {
> +  if (const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(T))
> +    return getConstantArrayInfoInChars(CAT);
>    std::pair<uint64_t, unsigned> Info = getTypeInfo(T);
>    return std::make_pair(toCharUnitsFromBits(Info.first),
>                          toCharUnitsFromBits(Info.second));
>  }
>
> +/// getConstantArrayInfoInChars - Performing the computation in CharUnits
> +/// instead of in bits prevents overflowing the uint64_t for some large
> arrays.
> +std::pair<CharUnits, CharUnits>
> +ASTContext::getConstantArrayInfoInChars(const ConstantArrayType *CAT)
> const {
>

Can you make this a static non-member helper function? I don't think we
need it to be part of ASTContext's interface.


> +  std::pair<uint64_t, unsigned> EltInfo =
> getTypeInfo(CAT->getElementType());
> +  uint64_t Size = CAT->getSize().getZExtValue();
> +  assert((Size == 0 || EltInfo.first/getCharWidth() <=
> (uint64_t)(-1)/Size) &&
> +         "Overflow in array type char size evaluation");
> +  uint64_t Width = EltInfo.first / getCharWidth() * Size;
> +  unsigned Align = EltInfo.second / getCharWidth();
> +  Width = llvm::RoundUpToAlignment(Width, Align);
> +  return std::make_pair(CharUnits::fromQuantity(Width),
> +                        CharUnits::fromQuantity(Align));
> +}
> +
>  std::pair<CharUnits, CharUnits>
>  ASTContext::getTypeInfoInChars(QualType T) const {
>    return getTypeInfoInChars(T.getTypePtr());
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits