GNU Extension: Implement "new" of VLA types

[Quuxplusone]

Bugzilla Link	PR8209
Status	NEW
Importance	P enhancement
Reported by	M.E. O'Neill (oneill+llvmbugs@cs.hmc.edu)
Reported on	2010-09-23 02:17:45 -0700
Last modified on	2010-11-08 21:41:28 -0800
Version	trunk
Hardware	All All
CC	dgregor@apple.com, llvm-bugs@lists.llvm.org, schaub.johannes@googlemail.com
Fixed by commit(s)
Attachments	dyn-typedef.cpp (726 bytes, application/octet-stream)
Blocks
Blocked by
See also

Created attachment 5516
Sample code showing bug

Both g++ and clang++ allow you to write a typedef with dynamic size.  Both seem
to know what it means in principle, but only g++ currently generates correct
code.

If you run the attached simple test program (./dyntypedef 1 2 3) under g++, you
get

  Without typedef...
  sizeof(int[argc]) = 16
  operator new[](16)

  With typedef...
  sizeof(arr) = 16
  operator new[](16)

but if you run it with clang++, you get

  Without typedef...
  sizeof(int[argc]) = 16
  operator new[](16)

  With typedef...
  sizeof(arr) = 16
  operator new(0)

Results are indepenent of optimizaiton settings, etc.  Bug present in many
prior versions of clang++.

clang version 2.9 (trunk 114629)
Target: x86_64-unknown-linux-gnu
Thread model: posix

gcc version 4.4.3 (Ubuntu 4.4.3-4ubuntu5)

[Quuxplusone]

Attached dyn-typedef.cpp (726 bytes, application/octet-stream): Sample code showing bug

[Quuxplusone]

This is a GCC extension, which uses variable-length arrays in new expressions. We should either reject it outright or support it.

[Quuxplusone]

(In reply to comment #1)
> This is a GCC extension, which uses variable-length arrays in new expressions.
> We should either reject it outright or support it.

I think there is a GNU extension at work here, but I think it occurs before we
get to the new expression.

If you try the same code on Comeau C++ (using their free on-line tester), you
get:

Comeau C/C++ 4.3.10.1 (Oct  6 2008 11:28:09) for ONLINE_EVALUATION_BETA2
Copyright 1988-2008 Comeau Computing.  All rights reserved.
MODE:strict errors C++ C++0x_extensions

"ComeauTest.c", line 26: error: expression must have a constant value
    cout << "sizeof(int[argc]) = " << sizeof(int[argc]) << endl;
                                                 ^

"ComeauTest.c", line 31: error: expression must have a constant value
    typedef int arr[argc];
                    ^

2 errors detected in the compilation of "ComeauTest.c".

If we tell Comeau C++ to allow GNU extensions, we get:

Comeau C/C++ 4.3.10.1 (Oct  6 2008 11:28:09) for ONLINE_EVALUATION_BETA2
Copyright 1988-2008 Comeau Computing.  All rights reserved.
MODE:non-strict warnings g++_compat C++ C++0x_extensions

"ComeauTest.c", line 33: error: a variable-length array type is not allowed
    void* bp = new arr;
                   ^

1 error detected in the compilation of "ComeauTest.c".

What seems odd to me is that Clang accepts the typedef, seems to know what it's
got, since it gives the correct size in sizeof(), yet, it calls the wrong kind
of operator new and passes it the wrong size.

[Quuxplusone]

(In reply to comment #2)
> (In reply to comment #1)
> > This is a GCC extension, which uses variable-length arrays in new
expressions.
> > We should either reject it outright or support it.
>
> I think there is a GNU extension at work here, but I think it occurs before we
> get to the new expression.

The GNU extension is supporting variable-length arrays in C++.

> If we tell Comeau C++ to allow GNU extensions, we get:
>
> Comeau C/C++ 4.3.10.1 (Oct  6 2008 11:28:09) for ONLINE_EVALUATION_BETA2
> Copyright 1988-2008 Comeau Computing.  All rights reserved.
> MODE:non-strict warnings g++_compat C++ C++0x_extensions
>
> "ComeauTest.c", line 33: error: a variable-length array type is not allowed
>     void* bp = new arr;
>                    ^
>
> 1 error detected in the compilation of "ComeauTest.c".

... which is one perfectly-reasonable answer.

> What seems odd to me is that Clang accepts the typedef, seems to know what
it's
> got, since it gives the correct size in sizeof(), yet, it calls the wrong kind
> of operator new and passes it the wrong size.

Clang is just not treating it as an array, since nobody thought of this corner
case when VLA support for C++ was hacked into Clang.

What project did the original code come from?

[Quuxplusone]

(In reply to comment #3)
> What project did the original code come from?

It mostly comes from me, saying “Hmm, I wonder how clang handles this corner
case”.

I mostly don't mind what it does, but it shouldn't miscompile.

[Quuxplusone]

(In reply to comment #3)
> The GNU extension is supporting variable-length arrays in C++.

Right.  But variable-length arrays are a C99ism too, and so it isn't a wild-and-
crazy GNU extension, but a fairly straightforward one.

(If you remove everything related to operator new, the code I gave is valid
C99.)

> Clang is just not treating it as an array

Note that there are two parts to the miscompilation:
  - Calling operator new rather than operator new[]
  - Using zero as the size for the call

Both aspects are problematic.

[Quuxplusone]

As of r115790, Clang rejects allocation of variably-modified types. Holding this bug open until I decide whether to implement new'ing VLA types as an extension.

[Quuxplusone]

This should probably also consider the resolution of http://llvm.org/bugs/show_bug.cgi?id=4498 . GCC treats "new (int[n])" as creating a variable length array and complains accordingly in pedantic mode. My patch rejects this, while clang accepts it when written using a typedef.

[Quuxplusone]

(In reply to comment #7)
> This should probably also consider the resolution of
> http://llvm.org/bugs/show_bug.cgi?id=4498 . GCC treats "new (int[n])" as
> creating a variable length array and complains accordingly in pedantic mode.
My
> patch rejects this, while clang accepts it when written using a typedef.

Err, I misread doug's comment. Both ways are rejected now. Anyway, if we accept
one way later on as an extension, PR4498 should probably be accepted as an
extension too.

[Quuxplusone]

Marking as an extension.