[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: vsapsai, rsmith, george.burgess.iv.

The commit r316245 introduced a regression that causes an assertion failure when Clang tries to cast an `IncompleteArrayType` to a `PointerType` when evaluating __builtin_object_size in this sample:

  typedef struct 	 {
    char string[512];
  } NestedArrayStruct;
  
  typedef struct 	 {
      int x;
      NestedArrayStruct	session[];
  } IncompleteArrayStruct;
  
  void func(IncompleteArrayStruct* p) {
     __builtin___strlcpy_chk (p->session[0].string, "ab", 2, __builtin_object_size(p->session[0].string, 1));
  }

Interestingly enough gcc seems to produce a different output for the above code (when `1` is the last parameter to `__builtin_object_size`). It evaluates `__builtin_object_size` to 512 instead of -1 like Clang:

https://godbolt.org/g/vD9F9T

I'm still not sure what's the right behavior after reading GCC's description of __builtin_object_size (https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html). Maybe someone who's more familiar with this builtin could point to the cause of this discrepancy.

rdar://36094951


Repository:
  rC Clang

https://reviews.llvm.org/D41405

Files:
  lib/AST/ExprConstant.cpp
  test/Sema/builtin-object-size.c


Index: test/Sema/builtin-object-size.c
===================================================================
--- test/Sema/builtin-object-size.c
+++ test/Sema/builtin-object-size.c
@@ -91,3 +91,16 @@
 
   return n;
 }
+
+typedef struct 	 {
+  char string[512];
+} NestedArrayStruct;
+
+typedef struct 	 {
+    int x;
+    NestedArrayStruct	session[];
+} IncompleteArrayStruct;
+
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct* p) {
+   __builtin___strlcpy_chk (p->session[0].string, "ab", 2, __builtin_object_size(p->session[0].string, 1));
+}
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -7419,7 +7419,10 @@
     // If we don't know the array bound, conservatively assume we're looking at
     // the final array element.
     ++I;
-    BaseType = BaseType->castAs<PointerType>()->getPointeeType();
+    if (BaseType->isIncompleteArrayType())
+      BaseType = Ctx.getAsArrayType(BaseType)->getElementType();
+    else
+      BaseType = BaseType->castAs<PointerType>()->getPointeeType();
   }
 
   for (unsigned E = LVal.Designator.Entries.size(); I != E; ++I) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41405.127438.patch
Type: text/x-patch
Size: 1204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171219/ebca7409/attachment.bin>