[PATCH] Flexible array initializers may be strings

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Flexible array initializers may be strings

Pierre Habouzit
for example:

struct a {
  int  b;
  char v[];
};

struct a foo = { .b = 0, .v = "bar" };

is perfectly sensible and correct (and accepted by gcc).

Signed-off-by: Pierre Habouzit <[hidden email]>
---
 lib/Sema/SemaInit.cpp |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


    For the record, clang accepts the following code:

    -------8<-----------
    #include <stdio.h>

    struct a {
        int  i;
        char v[];
    };

    int main(void) {
        struct a foo = { .i = 0, .v = "foo" };
        struct a bar = { .i = 0, .v = { 'b', 'a', 'r', '\0' } };

        printf("%p %s\n", foo.v, foo.v);
        printf("%p %s\n", bar.v, bar.v);
        return 0;
    }
    -------8<-----------

    whereas gcc rejects it with the error:
        error: non-static initialization of a flexible array member

    Clang doesn't complain, but generates invalid code though when foo and bar
    aren't static:

    $ clang -o a a.c && ./a
    0x7fff1eccc32c
    0x7fff1eccc324 ÿ

    Reading the relevant parts of the bitcode indeed shows what is wrong:
    there is a correct internal constant for main.foo, but the variable
    foo storage is allocated using sizeof(struct a) instead of
    sizeof(main.foo), and the memcpy also uses sizeof(struct a) which is
    4 instead of sizeof(main.foo).

    @main.foo = internal constant %0 { i32 0, [4 x i8] c"foo\00" }, align 4
    %foo = alloca %struct.a, align 4
    call void @llvm.memcpy.p0i8.p0i8.i64(i8* %tmp, i8* bitcast (%0* @main.foo to i8*), i64 4, i32 4, i1 false)

    Sadly fixing that exceeds my current clang hacking skills. Do you
    want me to report a bug for that ?

declare i32 @printf(i8*, ...)

diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 1ffa702..916134e 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -1461,7 +1461,7 @@ InitListChecker::CheckDesignatedInitializer(const InitializedEntity &Entity,
         Invalid = true;
       }
 
-      if (!hadError && !isa<InitListExpr>(DIE->getInit())) {
+      if (!hadError && !isa<InitListExpr>(DIE->getInit()) && !isa<StringLiteral>(DIE->getInit())) {
         // The initializer is not an initializer list.
         SemaRef.Diag(DIE->getInit()->getSourceRange().getBegin(),
                       diag::err_flexible_array_init_needs_braces)
--
1.7.2.2.566.g36af9

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Flexible array initializers may be strings

Pierre Habouzit
> For the record, clang accepts the following code:
>
> -------8<-----------
> #include <stdio.h>
>
> struct a {
>   int  i;
>   char v[];
> };
>
> int main(void) {
>   struct a foo = { .i = 0, .v = "foo" };
>   struct a bar = { .i = 0, .v = { 'b', 'a', 'r', '\0' } };
>
>   printf("%p %s\n", foo.v, foo.v);
>   printf("%p %s\n", bar.v, bar.v);
>   return 0;
> }
> -------8<-----------
>
> whereas gcc rejects it with the error:
>   error: non-static initialization of a flexible array member
>
> Clang doesn't complain, but generates invalid code though when foo and bar
> aren't static:
>
> $ clang -o a a.c && ./a
> 0x7fff1eccc32c
> 0x7fff1eccc324 ÿ
>
> Reading the relevant parts of the bitcode indeed shows what is wrong:
> there is a correct internal constant for main.foo, but the variable
> foo storage is allocated using sizeof(struct a) instead of
> sizeof(main.foo), and the memcpy also uses sizeof(struct a) which is
> 4 instead of sizeof(main.foo).
>
> @main.foo = internal constant %0 { i32 0, [4 x i8] c"foo\00" }, align 4
> %foo = alloca %struct.a, align 4
> call void @llvm.memcpy.p0i8.p0i8.i64(i8* %tmp, i8* bitcast (%0* @main.foo to i8*), i64 4, i32 4, i1 false)
>
> Sadly fixing that exceeds my current clang hacking skills. Do you
> want me to report a bug for that ?

I made it http://llvm.org/bugs/show_bug.cgi?id=8217
--
·O·  Pierre Habouzit
··O                                                [hidden email]
OOO                                                http://www.madism.org

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Flexible array initializers may be strings

Pierre Habouzit
In reply to this post by Pierre Habouzit
> -      if (!hadError && !isa<InitListExpr>(DIE->getInit())) {
> +      if (!hadError && !isa<InitListExpr>(DIE->getInit()) && !isa<StringLiteral>(DIE->getInit())) {

I noticed after that that there is an IsStringInit function, so I tried
the patch with
    !IsStringInit(DIE->getInit(), DIE->getType, SemaRef.Context)
which handles more cases, but for some reason it doesn't work on my
use case anymore, and I've not been able to understand why.
--
·O·  Pierre Habouzit
··O                                                [hidden email]
OOO                                                http://www.madism.org
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Flexible array initializers may be strings

Chris Lattner
In reply to this post by Pierre Habouzit

On Sep 24, 2010, at 2:14 AM, Pierre Habouzit wrote:

> for example:
>
> struct a {
>  int  b;
>  char v[];
> };
>
> struct a foo = { .b = 0, .v = "bar" };
>
> is perfectly sensible and correct (and accepted by gcc).

Thanks! Applied in r116165, sorry for the delay.

-Chris
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Flexible array initializers may be strings

Chris Lattner
In reply to this post by Pierre Habouzit

On Sep 24, 2010, at 2:41 AM, Pierre Habouzit wrote:

>> @main.foo = internal constant %0 { i32 0, [4 x i8] c"foo\00" }, align 4
>> %foo = alloca %struct.a, align 4
>> call void @llvm.memcpy.p0i8.p0i8.i64(i8* %tmp, i8* bitcast (%0* @main.foo to i8*), i64 4, i32 4, i1 false)
>>
>> Sadly fixing that exceeds my current clang hacking skills. Do you
>> want me to report a bug for that ?
>
> I made it http://llvm.org/bugs/show_bug.cgi?id=8217

Thanks, fixed in r116166!

-Chris
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev