[PATCH] Fix to PR5683 - issue diagnostic for pointer subtraction with type of size zero.

[Richard Smith]

On Mon, Apr 8, 2013 at 8:55 PM, Serge Pavlov <sepavloff at gmail.com> wrote:

>   Patch updated
>
>   Now correctly handles incomplete types and implements faster check
> (thanks to John McCall)
>
> Hi rsmith,
>
> http://llvm-reviews.chandlerc.com/D637
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D637?vs=1547&id=1554#toc
>
> Files:
>   include/clang/Basic/DiagnosticSemaKinds.td
>   lib/AST/ExprConstant.cpp
>   lib/Sema/SemaExpr.cpp
>   test/Sema/empty1.c
>
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td
> +++ include/clang/Basic/DiagnosticSemaKinds.td
> @@ -3971,6 +3971,9 @@
>  def warn_offsetof_non_standardlayout_type : ExtWarn<
>    "offset of on non-standard-layout type %0">, InGroup<InvalidOffsetof>;
>  def err_offsetof_bitfield : Error<"cannot compute offset of bit-field
> %0">;
> +def warn_sub_ptr_zero_size_types : Warning<
> +  "subtraction of pointers to type %0 with zero size has undefined
> behavior">,
> +  InGroup<PointerArith>;
>
>  def warn_floatingpoint_eq : Warning<
>    "comparing floating point with == or != is unsafe">,
> Index: lib/AST/ExprConstant.cpp
> ===================================================================
> --- lib/AST/ExprConstant.cpp
> +++ lib/AST/ExprConstant.cpp
> @@ -5003,6 +5003,12 @@
>          if (!HandleSizeof(Info, E->getExprLoc(), ElementType,
> ElementSize))
>            return false;
>
> +        // Empty struct or union in C has size 0 (GCC extension). Meaning
> of
> +        // pointer difference in such case is unspecified, so set
> ElementSize
> +        // to 1 to avoid division by zero.
> +        if (ElementSize.isZero())
> +            ElementSize = CharUnits::One();
> +
>          // FIXME: LLVM and GCC both compute LHSOffset - RHSOffset at
> runtime,
>          // and produce incorrect results when it overflows. Such behavior
>          // appears to be non-conforming, but is common, so perhaps we
> should
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp
> +++ lib/Sema/SemaExpr.cpp
> @@ -6734,6 +6734,19 @@
>                                                 LHS.get(), RHS.get()))
>          return QualType();
>
> +      if (!getLangOpts().CPlusPlus) {
> +        // If pointee type is a structure or union of zero size (GCC
> extension),
> +        // the subtraction does not make sense.
> +        if (rpointee.getTypePtr()->isRecordType()) {
>

This check still looks wrong to me. CodeGen has special cases for VLAs,
void, and function types; any other zero-size type should get a diagnostic.
Right now, it looks like you're missing at least the case of an array with
a zero-sized element type (such as an array of empty records). I think
that's the only other case for GNU C, but Objective-C might add some more
possibilities?


> +          CharUnits ElementSize = Context.getTypeSizeInChars(rpointee);
> +          if (ElementSize.isZero()) {
> +            Diag(Loc,diag::warn_sub_ptr_zero_size_types)
> +              << rpointee.getUnqualifiedType()
> +              << LHS.get()->getSourceRange() <<
> RHS.get()->getSourceRange();
> +          }
> +        }
> +      }
> +
>        if (CompLHSTy) *CompLHSTy = LHS.get()->getType();
>        return Context.getPointerDiffType();
>      }
> Index: test/Sema/empty1.c
> ===================================================================
> --- /dev/null
> +++ test/Sema/empty1.c
> @@ -0,0 +1,45 @@
> +// RUN: %clang_cc1 %s -fsyntax-only -verify
> +
> +struct empty {};
> +struct empty_2 {};
> +union  empty_u {};
> +
> +struct empty* func_1p (struct empty* x) {
> +  return x - 5;
> +}
> +
> +int func_1 () {
> +  struct empty v[1];
> +  return v - v;  // expected-warning {{subtraction of pointers to type
> 'struct empty' with zero size has undefined behavior}}
> +}
> +
> +int func_2 (struct empty* x) {
> +  return 1 + x - x;  // expected-warning {{subtraction of pointers to
> type 'struct empty' with zero size has undefined behavior}}
> +}
> +
> +int func_3 (struct empty* x, struct empty* y) {
> +  return x - y;  // expected-warning {{subtraction of pointers to type
> 'struct empty' with zero size has undefined behavior}}
> +}
> +
> +int func_4 (struct empty* x, const struct empty* y) {
> +  return x - y;  // expected-warning {{subtraction of pointers to type
> 'struct empty' with zero size has undefined behavior}}
> +}
> +
> +int func_5 (volatile struct empty* x, const struct empty* y) {
> +  return x - y;  // expected-warning {{subtraction of pointers to type
> 'struct empty' with zero size has undefined behavior}}
> +}
> +
> +int func_6 (struct empty* x, struct empty_2* y) {
> +  return x - y;  // expected-error {{'struct empty *' and 'struct empty_2
> *' are not pointers to compatible types}}
> +}
> +
> +int func_7 () {
> +  union empty_u v[1];
> +  return v - v;  // expected-warning {{subtraction of pointers to type
> 'union empty_u' with zero size has undefined behavior}}
> +}
> +
> +struct A;  // expected-note {{forward declaration of 'struct A'}}
> +
> +int func_8 (struct A* x, struct A* y) {
> +  return x - y;  // expected-error {{arithmetic on a pointer to an
> incomplete type 'struct A'}}
> +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130408/c56ca0f0/attachment.html>