[Sema] Only relax array-at-end-of-object checks in __builtin_object_size when -fno-strict-aliasing is given.

[george.burgess.iv]

Summary

Mostly asking for a review to verify that you guys are happy with this approach.

Given that Hal said struct-path-tbaa doesn't really deal with arrays (yet), I decided to ignore -fno-struct-path-tbaa. If that changes in the future, we can relax this when that flag is given, too.

[ahatanak]

Just want to give you a heads up that I was going to abandon the patch that adds support for attribute "flexible_array" (https://reviews.llvm.org/D21453) after r281277, but I'll probably need the attribute after all if we are going to make the changes in this patch.

[ahatanak]

Depends on D21453.

Adding a dependency as a reminder to look into D21453 before committing this patch.

include/clang/Basic/LangOptions.def

 COMPATIBLE_VALUE_LANGOPT(PICLevel    , 2, 0, "__PIC__ level")
 COMPATIBLE_VALUE_LANGOPT(PIE         , 1, 0, "is pie")
 COMPATIBLE_LANGOPT(GNUInline         , 1, 0, "GNU inline semantics")
 COMPATIBLE_LANGOPT(NoInlineDefine    , 1, 0, "__NO_INLINE__ predefined macro")
 COMPATIBLE_LANGOPT(Deprecated        , 1, 0, "__DEPRECATED predefined macro")
 COMPATIBLE_LANGOPT(FastMath          , 1, 0, "fast FP math optimizations, and __FAST_MATH__ predefined macro")
 COMPATIBLE_LANGOPT(FiniteMathOnly    , 1, 0, "__FINITE_MATH_ONLY__ predefined macro")
 COMPATIBLE_LANGOPT(UnsafeFPMath      , 1, 0, "Unsafe Floating Point Math")
+COMPATIBLE_LANGOPT(StrictAliasing    , 1, 0, "Using strict aliasing rules")
 
 BENIGN_LANGOPT(ObjCGCBitmapPrint , 1, 0, "printing of GC's bitmap layout for __weak/__strong ivars")
 
 BENIGN_LANGOPT(AccessControl     , 1, 1, "C++ access control")
 LANGOPT(CharIsSigned      , 1, 1, "signed char")
 LANGOPT(ShortWChar        , 1, 0, "unsigned short wchar_t")
 ENUM_LANGOPT(MSPointerToMemberRepresentationMethod, PragmaMSPointersToMembersKind, 2, PPTMK_BestCase, "member-pointer representation method")
 ENUM_LANGOPT(DefaultCallingConv, DefaultCallingConvention, 3, DCC_None, "default calling convention")

lib/AST/ExprConstant.cpp

static bool tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type,
   auto EndOffset = End.getLValueOffset();
 
   // The following is a moderately common idiom in C:
   //
   // struct Foo { int a; char c[1]; };
   // struct Foo *F = (struct Foo *)malloc(sizeof(struct Foo) + strlen(Bar));
   // strcpy(&F->c[0], Bar);
   //
-  // So, if we see that we're examining an array at the end of a struct with an
-  // unknown base, we give up instead of breaking code that behaves this way.
-  // Note that we only do this when Type=1, because Type=3 is a lower bound, so
-  // answering conservatively is fine.
+  // So, if we see that we're examining a 1-length (or 0-length) array at the
+  // end of a struct with an unknown base, we give up instead of breaking code
+  // that behaves this way. Note that we only do this when Type=1, because
+  // Type=3 is a lower bound, so answering conservatively is fine.
   //
-  // We used to be a bit more aggressive here; we'd only be conservative if the
-  // array at the end was flexible, or if it had 0 or 1 elements. This broke
-  // some common standard library extensions (PR30346), but was otherwise
-  // seemingly fine. It may be useful to reintroduce this behavior with some
-  // sort of whitelist. OTOH, it seems that GCC is always conservative with the
-  // last element in structs (if it's an array), so our current behavior is more
-  // compatible than a whitelisting approach would be.
+  // This check is relaxed a bit if strict aliasing is off, since there's a
+  // nontrivial amount of code that treats the array size at the end of a struct
+  // as a suggestion, rather than a constraint (PR30346).
+  bool RelaxedEndOfObjectChecks = !Info.Ctx.getLangOpts().StrictAliasing;
   if (End.InvalidBase && SubobjectOnly && Type == 1 &&
       End.Designator.Entries.size() == End.Designator.MostDerivedPathLength &&
       End.Designator.MostDerivedIsArrayElement &&
+      (RelaxedEndOfObjectChecks || End.Designator.MostDerivedArraySize < 2) &&
       isDesignatorAtObjectEnd(Info.Ctx, End))
     return false;
 
   if (BaseOffset > EndOffset)
     return Success(0, E);
 
   return Success((EndOffset - BaseOffset).getQuantity(), E);
 }

lib/Frontend/CompilerInvocation.cpp

Opts.FastMath = Args.hasArg(OPT_ffast_math) ||
       Args.hasArg(OPT_cl_fast_relaxed_math);
   Opts.FiniteMathOnly = Args.hasArg(OPT_ffinite_math_only) ||
       Args.hasArg(OPT_cl_finite_math_only) ||
       Args.hasArg(OPT_cl_fast_relaxed_math);
   Opts.UnsafeFPMath = Args.hasArg(OPT_menable_unsafe_fp_math) ||
                       Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
                       Args.hasArg(OPT_cl_fast_relaxed_math);
 
+  Opts.StrictAliasing = !Args.hasArg(OPT_relaxed_aliasing);
+
   Opts.RetainCommentsFromSystemHeaders =
       Args.hasArg(OPT_fretain_comments_from_system_headers);
 
   unsigned SSP = getLastArgIntValue(Args, OPT_stack_protector, 0, Diags);
   switch (SSP) {
   default:
     Diags.Report(diag::err_drv_invalid_value)
       << Args.getLastArg(OPT_stack_protector)->getAsString(Args) << SSP;

test/CodeGen/object-size.c

-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefixes=STRICT,CHECK
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 -relaxed-aliasing | FileCheck %s --check-prefixes=NOSTRICT,CHECK
 
 #define strcpy(dest, src) \
   ((__builtin_object_size(dest, 0) != -1ULL) \
    ? __builtin___strcpy_chk (dest, src, __builtin_object_size(dest, 1)) \
    : __inline_strcpy_chk(dest, src))
 
 static char *__inline_strcpy_chk (char *dest, const char *src) {
   return __builtin___strcpy_chk(dest, src, __builtin_object_size(dest, 1));
void test23(struct Test23Ty *p) {
   gi = __builtin_object_size(&p->a, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(&p->a, 2);
   // CHECK: store i32 4
   gi = __builtin_object_size(&p->a, 3);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(&p->t[5], 0);
-  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // NOSTRICT: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // STRICT: store i32 20
   gi = __builtin_object_size(&p->t[5], 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(&p->t[5], 2);
   // CHECK: store i32 20
   gi = __builtin_object_size(&p->t[5], 3);
 }
 
 // PR24493 -- ICE if __builtin_object_size called with NULL and (Type & 1) != 0
void test29(struct DynStructVar *dv, struct DynStruct0 *d0,
   gi = __builtin_object_size(d1->snd, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(d1->snd, 2);
   // CHECK: store i32 1
   gi = __builtin_object_size(d1->snd, 3);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(ss->snd, 0);
-  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // NOSTRICT: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // STRICT: store i32 2
   gi = __builtin_object_size(ss->snd, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(ss->snd, 2);
   // CHECK: store i32 2
   gi = __builtin_object_size(ss->snd, 3);
 }
 
 // CHECK-LABEL: @test30
void test31() {
   struct DynStructVar *dsv;
   struct DynStruct0 *ds0;
   struct DynStruct1 *ds1;
   struct StaticStruct *ss;
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(ds1[9].snd, 1);
 
-  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // NOSTRICT: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // STRICT: store i32 2
   gi = __builtin_object_size(&ss[9].snd[0], 1);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(&ds1[9].snd[0], 1);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(&ds0[9].snd[0], 1);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(&dsv[9].snd[0], 1);
 }
 
 // CHECK-LABEL: @PR30346
 void PR30346() {
   struct sa_family_t {};
   struct sockaddr {
     struct sa_family_t sa_family;
     char sa_data[14];
   };
 
   struct sockaddr *sa;
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(sa->sa_data, 0);
-  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // NOSTRICT: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  // STRICT: store i32 14
   gi = __builtin_object_size(sa->sa_data, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(sa->sa_data, 2);
   // CHECK: store i32 14
   gi = __builtin_object_size(sa->sa_data, 3);
 }

test/CodeGen/pass-object-size.c

-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -O0 %s -o - 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -O0 %s -o - 2>&1 | FileCheck %s --check-prefixes STRICT,CHECK
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -O0 %s -o - -relaxed-aliasing 2>&1 | FileCheck %s --check-prefixes NOSTRICT,CHECK
 
 typedef unsigned long size_t;
 
 struct Foo {
   int t[10];
 };
 
 #define PS(N) __attribute__((pass_object_size(N)))
void test1() {
   // CHECK: call i32 @ObjectSize2(i8* %{{.*}}, i64 356)
   gi = ObjectSize2(&t[1].t[1]);
   // CHECK: call i32 @ObjectSize3(i8* %{{.*}}, i64 36)
   gi = ObjectSize3(&t[1].t[1]);
 }
 
 // CHECK-LABEL: define void @test2
 void test2(struct Foo *t) {
-  // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
-  // CHECK: call i32 @ObjectSize1(i8* %{{.*}}, i64 [[VAR]])
+  // NOSTRICT: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+  // NOSTRICT: call i32 @ObjectSize1(i8* %{{.*}}, i64 [[VAR]])
+  //
+  // STRICT: call i32 @ObjectSize1(i8* %{{.*}}, i64 36)
   gi = ObjectSize1(&t->t[1]);
   // CHECK: call i32 @ObjectSize3(i8* %{{.*}}, i64 36)
   gi = ObjectSize3(&t->t[1]);
 }
 
 // CHECK-LABEL: define i32 @_Z27NoViableOverloadObjectSize0Pv
 int NoViableOverloadObjectSize0(void *const p) __attribute__((overloadable)) {
   // CHECK: @llvm.objectsize
void test4(struct Foo *t) {
   gi = NoViableOverloadObjectSize1(&t[1]);
   // CHECK: call i32 @_Z27NoViableOverloadObjectSize2PvU17pass_object_size2(i8* %{{.*}}, i64 %{{.*}})
   gi = NoViableOverloadObjectSize2(&t[1]);
   // CHECK: call i32 @_Z27NoViableOverloadObjectSize3PvU17pass_object_size3(i8* %{{.*}}, i64 0)
   gi = NoViableOverloadObjectSize3(&t[1]);
 
   // CHECK: call i32 @_Z27NoViableOverloadObjectSize0PvU17pass_object_size0(i8* %{{.*}}, i64 %{{.*}})
   gi = NoViableOverloadObjectSize0(&t[1].t[1]);
-  // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
-  // CHECK: call i32 @_Z27NoViableOverloadObjectSize1PvU17pass_object_size1(i8* %{{.*}}, i64 [[VAR]])
+  // NOSTRICT: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+  // NOSTRICT: call i32 @_Z27NoViableOverloadObjectSize1PvU17pass_object_size1(i8* %{{.*}}, i64 [[VAR]])
+  //
+  // STRICT: call i32 @_Z27NoViableOverloadObjectSize1PvU17pass_object_size1(i8* %{{.*}}, i64 36)
   gi = NoViableOverloadObjectSize1(&t[1].t[1]);
   // CHECK: call i32 @_Z27NoViableOverloadObjectSize2PvU17pass_object_size2(i8* %{{.*}}, i64 %{{.*}})
   gi = NoViableOverloadObjectSize2(&t[1].t[1]);
   // CHECK: call i32 @_Z27NoViableOverloadObjectSize3PvU17pass_object_size3(i8* %{{.*}}, i64 36)
   gi = NoViableOverloadObjectSize3(&t[1].t[1]);
 }
 
 void test5() {
void test7() {
   // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 356)
   gi = AsmObjectSize2(&t[1].t[1]);
   // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 36)
   gi = AsmObjectSize3(&t[1].t[1]);
 }
 
 // CHECK-LABEL: define void @test8
 void test8(struct Foo *t) {
-  // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
-  // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 [[VAR]])
+  // NOSTRICT: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+  // NOSTRICT: call i32 @"\01Identity"(i8* %{{.*}}, i64 [[VAR]])
+  //
+  // STRICT: call i32 @"\01Identity"(i8* %{{.*}}, i64 36)
   gi = AsmObjectSize1(&t[1].t[1]);
   // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 36)
   gi = AsmObjectSize3(&t[1].t[1]);
 }
 
 void DifferingObjectSize0(void *const p __attribute__((pass_object_size(0))));
 void DifferingObjectSize1(void *const p __attribute__((pass_object_size(1))));
 void DifferingObjectSize2(void *const p __attribute__((pass_object_size(2))));