InitListExpr with void type

Today I was running the latest build of the static analyzer over the Wine sources and noticed a crash in its handling of InitListExprs. I won't go into the gory details; essentially there are cases where an InitListExpr can have type 'void' and this is a case the analyzer does not (yet) handle.

My question is whether or not it is valid for InitListExprs to have a 'void' type, and if so, how should they be interpreted?

Here is an example (reduced test case from wine):

struct _D3DMATRIX { union { float m[4][4]; }; };
typedef struct _D3DMATRIX D3DXMATRIX;
int compare_matrix(const D3DXMATRIX *m1, const D3DXMATRIX *m2) {
   const D3DXMATRIX mat1 = {
     { { 1.0f, 2.0f, 3.0f, 4.0f,
         5.0f, 6.0f, 7.0f, 8.0f,
         9.0f, 10.0f, 11.0f, 12.0f,
         13.0f, 14.0f, 15.0f, 16.0f } }
   };
}

And the ast dump:

(CompoundStmt 0x1d03ef0 </Users/kremenek/Desktop/t.c:3:64, line:10:1>
   (DeclStmt 0x1d03dc0 <line:4:3>
     0x1d04880 "D3DXMATRIX const mat1 =
       (InitListExpr 0x1d04c60 <col:27, line:9:3> 'D3DXMATRIX const':'struct _D3DMATRIX const'
         (InitListExpr 0x1d04c30 <line:5:5, line:8:38> 'void'
           (InitListExpr 0x1d04bc0 <line:5:7, line:8:36> 'void'
             (FloatingLiteral 0x1d03d60 <line:5:9> 'float' 1.000000)
             (FloatingLiteral 0x1d03d90 <col:15> 'float' 2.000000)
             ... <SNIP>

Notice that the two nested InitListExprs have a 'void' type. How are clients suppose to interpret this?

Incidentally, clang generates a warning for the above (and the original) code:

$ clang t.c
t.c:5:5: warning: excess elements in array initializer
     { { 1.0f, 2.0f, 3.0f, 4.0f,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
1 diagnostic generated.

Is this correct behavior, or is there a bug in the ASTs?

Ted Kremenek wrote:

Today I was running the latest build of the static analyzer over the
Wine sources and noticed a crash in its handling of InitListExprs. I
won't go into the gory details; essentially there are cases where an
InitListExpr can have type 'void' and this is a case the analyzer does
not (yet) handle.

My question is whether or not it is valid for InitListExprs to have a
'void' type, and if so, how should they be interpreted?
  

I'm pretty sure InitListExprs pretty much *always* have void type, due
to these lines in Sema::ActOnInitList:

  InitListExpr *E = new (Context) InitListExpr(LBraceLoc, InitList,
NumInit,
                                    RBraceLoc,
Designators.hasAnyDesignators());
  E->setType(Context.VoidTy); // FIXME: just a place holder for now.

Sebastian

Today I was running the latest build of the static analyzer over the
Wine sources and noticed a crash in its handling of InitListExprs. I
won't go into the gory details; essentially there are cases where an
InitListExpr can have type 'void' and this is a case the analyzer does
not (yet) handle.

Blech.

My question is whether or not it is valid for InitListExprs to have a
'void' type, and if so, how should they be interpreted?

IMO, it isn't valid. After semantic analysis of the initializer list, all InitListExprs should have the type of the object they initialize. If the InitListExpr still has void type, then either (1) it's an invalid AST node and we shouldn't be looking at it, or (2) there's a bug in semantic analysis of initializer lists.

Here is an example (reduced test case from wine):

struct _D3DMATRIX { union { float m[4][4]; }; };
typedef struct _D3DMATRIX D3DXMATRIX;
int compare_matrix(const D3DXMATRIX *m1, const D3DXMATRIX *m2) {
  const D3DXMATRIX mat1 = {
    { { 1.0f, 2.0f, 3.0f, 4.0f,
        5.0f, 6.0f, 7.0f, 8.0f,
        9.0f, 10.0f, 11.0f, 12.0f,
        13.0f, 14.0f, 15.0f, 16.0f } }
  };
}

Oh, fun. The initialization code doesn't handle anonymous unions properly (it skips them).

And the ast dump:

(CompoundStmt 0x1d03ef0 </Users/kremenek/Desktop/t.c:3:64, line:10:1>
  (DeclStmt 0x1d03dc0 <line:4:3>
    0x1d04880 "D3DXMATRIX const mat1 =
      (InitListExpr 0x1d04c60 <col:27, line:9:3> 'D3DXMATRIX
const':'struct _D3DMATRIX const'
        (InitListExpr 0x1d04c30 <line:5:5, line:8:38> 'void'
          (InitListExpr 0x1d04bc0 <line:5:7, line:8:36> 'void'
            (FloatingLiteral 0x1d03d60 <line:5:9> 'float' 1.000000)
            (FloatingLiteral 0x1d03d90 <col:15> 'float' 2.000000)
            ... <SNIP>

Notice that the two nested InitListExprs have a 'void' type. How are
clients suppose to interpret this?

They shouldn't have to.

Incidentally, clang generates a warning for the above (and the
original) code:

$ clang t.c
t.c:5:5: warning: excess elements in array initializer
    { { 1.0f, 2.0f, 3.0f, 4.0f,
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
1 diagnostic generated.

That warning is bogus, and it shows that we're handling this initializer list improperly.

I'm tackling the initializer-list stuff as part of CodeGen for designated initializers, and this example is (now) working in my local tree. If you've filed a bug, feel free to kick it over to me to get the InitListExpr types straightened out as part of this work.

  - Doug

Thanks Doug. I haven't filed a bug yet. I would be more than happy to if that helps anyone.

That's what I thought. If it an invalid AST node, should we just reject the file? If we don't build the InitListExpr correctly because of [insert some feature not implemented yet] it seems that we should just abort with a fail-stop error in Sema rather than having unpredictable behavior in clients that assume the ASTs are kosher.

Today I was running the latest build of the static analyzer over the
Wine sources and noticed a crash in its handling of InitListExprs. I
won't go into the gory details; essentially there are cases where an
InitListExpr can have type 'void' and this is a case the analyzer does
not (yet) handle.

Blech.

My question is whether or not it is valid for InitListExprs to have a
'void' type, and if so, how should they be interpreted?

IMO, it isn't valid. After semantic analysis of the initializer list, all InitListExprs should have the type of the object they initialize. If the InitListExpr still has void type, then either (1) it's an invalid AST node and we shouldn't be looking at it, or (2) there's a bug in semantic analysis of initializer lists.

That's what I thought. If it an invalid AST node, should we just reject the file?

Yes. We should error and then not link the invalid AST node into the AST produced.

If we don't build the InitListExpr correctly because of [insert some feature not implemented yet] it seems that we should just abort with a fail-stop error in Sema rather than having unpredictable behavior in clients that assume the ASTs are kosher.

In this case, our misinterpretation of the initializer resulted in a warning, not a hard error, so it thought the AST was valid.

The real issue is that we need an assert that says: after we've processed the initializer list, we either had an error or our InitListExpr type is not void. No path should sneak through the semantic analysis without setting the type or reporting an error.

  - Doug