r281277 - [Sema] Fix PR30346: relax __builtin_object_size checks.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 12 16:50:36 PDT 2016
Author: gbiv
Date: Mon Sep 12 18:50:35 2016
New Revision: 281277
URL: http://llvm.org/viewvc/llvm-project?rev=281277&view=rev
Log:
[Sema] Fix PR30346: relax __builtin_object_size checks.
This patch makes us act more conservatively when trying to determine
the objectsize for an array at the end of an object. This is in
response to code like the following:
```
struct sockaddr {
/* snip */
char sa_data[14];
};
void foo(const char *s) {
size_t slen = strlen(s) + 1;
size_t added_len = slen <= 14 ? 0 : slen - 14;
struct sockaddr *sa = malloc(sizeof(struct sockaddr) + added_len);
strcpy(sa->sa_data, s);
// ...
}
```
`__builtin_object_size(sa->sa_data, 1)` would return 14, when there
could be more than 14 bytes at `sa->sa_data`.
Code like this is apparently not uncommon. FreeBSD's manual even
explicitly mentions this pattern:
https://www.freebsd.org/doc/en/books/developers-handbook/sockets-essential-functions.html
(section 7.5.1.1.2).
In light of this, we now just give up on any array at the end of an
object if we can't find the object's initial allocation.
I lack numbers for how much more conservative we actually become as a
result of this change, so I chose the fix that would make us as
compatible with GCC as possible. If we want to be more aggressive, I'm
happy to consider some kind of whitelist or something instead.
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/CodeGen/object-size.c
cfe/trunk/test/CodeGen/pass-object-size.c
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=281277&r1=281276&r2=281277&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 12 18:50:35 2016
@@ -6826,14 +6826,21 @@ static bool tryEvaluateBuiltinObjectSize
// struct Foo *F = (struct Foo *)malloc(sizeof(struct Foo) + strlen(Bar));
// strcpy(&F->c[0], Bar);
//
- // 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.
+ // 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.
+ //
+ // 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.
if (End.InvalidBase && SubobjectOnly && Type == 1 &&
End.Designator.Entries.size() == End.Designator.MostDerivedPathLength &&
End.Designator.MostDerivedIsArrayElement &&
- End.Designator.MostDerivedArraySize < 2 &&
isDesignatorAtObjectEnd(Info.Ctx, End))
return false;
Modified: cfe/trunk/test/CodeGen/object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/object-size.c?rev=281277&r1=281276&r2=281277&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/object-size.c (original)
+++ cfe/trunk/test/CodeGen/object-size.c Mon Sep 12 18:50:35 2016
@@ -276,7 +276,7 @@ void test23(struct Test23Ty *p) {
// CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
gi = __builtin_object_size(&p->t[5], 0);
- // CHECK: store i32 20
+ // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
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);
@@ -444,7 +444,7 @@ void test29(struct DynStructVar *dv, str
// CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
gi = __builtin_object_size(ss->snd, 0);
- // CHECK: store i32 2
+ // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
gi = __builtin_object_size(ss->snd, 1);
// CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
gi = __builtin_object_size(ss->snd, 2);
@@ -505,7 +505,7 @@ void test31() {
// CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
gi = __builtin_object_size(ds1[9].snd, 1);
- // CHECK: store i32 2
+ // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
gi = __builtin_object_size(&ss[9].snd[0], 1);
// CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
@@ -517,3 +517,22 @@ void test31() {
// 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)
+ 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);
+}
Modified: cfe/trunk/test/CodeGen/pass-object-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pass-object-size.c?rev=281277&r1=281276&r2=281277&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/pass-object-size.c (original)
+++ cfe/trunk/test/CodeGen/pass-object-size.c Mon Sep 12 18:50:35 2016
@@ -59,7 +59,8 @@ void test1() {
// CHECK-LABEL: define void @test2
void test2(struct Foo *t) {
- // CHECK: call i32 @ObjectSize1(i8* %{{.*}}, i64 36)
+ // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+ // CHECK: call i32 @ObjectSize1(i8* %{{.*}}, i64 [[VAR]])
gi = ObjectSize1(&t->t[1]);
// CHECK: call i32 @ObjectSize3(i8* %{{.*}}, i64 36)
gi = ObjectSize3(&t->t[1]);
@@ -168,7 +169,8 @@ void test4(struct Foo *t) {
// CHECK: call i32 @_Z27NoViableOverloadObjectSize0PvU17pass_object_size0(i8* %{{.*}}, i64 %{{.*}})
gi = NoViableOverloadObjectSize0(&t[1].t[1]);
- // CHECK: call i32 @_Z27NoViableOverloadObjectSize1PvU17pass_object_size1(i8* %{{.*}}, i64 36)
+ // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+ // CHECK: call i32 @_Z27NoViableOverloadObjectSize1PvU17pass_object_size1(i8* %{{.*}}, i64 [[VAR]])
gi = NoViableOverloadObjectSize1(&t[1].t[1]);
// CHECK: call i32 @_Z27NoViableOverloadObjectSize2PvU17pass_object_size2(i8* %{{.*}}, i64 %{{.*}})
gi = NoViableOverloadObjectSize2(&t[1].t[1]);
@@ -274,7 +276,8 @@ void test7() {
// CHECK-LABEL: define void @test8
void test8(struct Foo *t) {
- // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 36)
+ // CHECK: [[VAR:%[0-9]+]] = call i64 @llvm.objectsize
+ // CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 [[VAR]])
gi = AsmObjectSize1(&t[1].t[1]);
// CHECK: call i32 @"\01Identity"(i8* %{{.*}}, i64 36)
gi = AsmObjectSize3(&t[1].t[1]);
More information about the cfe-commits
mailing list