[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:808
       unsigned NumElems = numStructUnionElements(ILE->getType());
-      if (RDecl->hasFlexibleArrayMember())
+      if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember())
         ++NumElems;
----------------
shafik wrote:
> Fznamznon wrote:
> > Just for some context, numStructUnionElements checks that there is a flexible array member and returns number_of_initializable_fields-1 for structs. For unions it just returns 1 or 0, so flexible array member caused adding one more element to initlistexpr that was never properly handled.
> > 
> > Instead of doing this change, we could probably never enter initialization since the record (union) declaration is not valid, but that is not the case even for other types of errors in code, for example, I've tried declaring field of struct with a typo:
> > 
> > ```
> > struct { cha x[]; } r = {1}; 
> > ```
> > Initialization is still performed by clang.
> > Also, it seems MSVC considers flexible array member inside union as valid, so the test code is probably not always invalid.
> I am not sure what to think here, looking at gcc documentation for this extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html 
> 
> and using the following code:
> 
> ```
> struct f1 {
>   int x; int y[];
> } f1 = { 1, { 2, 3, 4 } }; // #1
> 
> struct f2 {
>   struct f1 f1; int data[3];
> } f2 = { { 1 }, { 2, 3, 4 } }; // #2
> 
> struct { char x[]; } r = {1};  // #3
> ```
> 
> gcc rejects 2 and 3 even though 2 comes from their documentation. Clang warns on 2 and MSVC rejects 2
> 
> CC @aaron.ballman @rsmith 
Yes, I also had a feeling that we probably need to refine how these extensions are supported by clang, that is probably a bit out of scope of the fix though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147626/new/

https://reviews.llvm.org/D147626