[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Tinycc-devel] tccgen.c: off by one in flexible array members
From: |
Michael Matz |
Subject: |
Re: [Tinycc-devel] tccgen.c: off by one in flexible array members |
Date: |
Fri, 11 Mar 2016 19:59:14 +0100 (CET) |
User-agent: |
Alpine 2.20 (LSU 67 2015-01-07) |
Hi,
On Wed, 9 Mar 2016, Henry Kroll wrote:
I created a + 1 patch that seems to work, but I need to run more tests
before committing.
=====================================================
diff --git a/tccgen.c b/tccgen.c
index 3cd28ed..270d11c 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -5847,7 +5847,7 @@ static void decl_initializer_alloc(CType *type,
AttributeDef *ad, int r,
tcc_error("unknown type size");
}
if (flexible_array)
- size += flexible_array->type.ref->c *
pointed_size(&flexible_array->type);
+ size += flexible_array->type.ref->c *
pointed_size(&flexible_array->type) + 1;
/* take into account specified alignment if bigger */
if (ad->a.aligned) {
if (ad->a.aligned > align)
=====================================================
Have a great day!
This still isn't quite right. The thing is, type->ref.c is -1 for an
unused flex array, so with your example ("char mem[]") your change
basically boils down to "size += -1 * 1 + 1" (i.e. +=0). But with a
different type (say int) it's now "size += -1 * 4 + 1" (i.e. +=3). So
this example still fails the same way:
-----------------
#include <stdio.h>
struct w {
char *data; int mem[];
};
int main (void) {
char s[9]="nonono"; struct w q = {"bugs"};
printf ("tcc has %s %s\n", s, q.data);
return !s[0];
}
-----------------
(only changed the mem[] member type). The +1 also looks conceptually
wrong, we try to adjust the size by some number of elements. The example
works of course if you'd do
size += (flexible_array->type.ref->c + 1)
* pointed_size(&flexible_array->type)
But then it'll break this example: (well, not break it, but it seems it'd
allocate more then necessary), which actually does use the flex member:
-----------------
#include <stdio.h>
struct w {
char *data; int mem[];
};
int main (void) {
char s[9]="nonono";
struct w q = {"bugs", { 1 }};
printf ("tcc has %s %s\n", s, q.data);
return !s[0];
}
-----------------
Here type->ref.c (after parsing the initializer) will be 1, the number of
elements used, and the original size calculation seems correct.
type->ref.c will be 0 if the flex member is there but empty (in the
initializer), so for that case the original is also correct.
So, I think it's more correct to special case the ref->c == -1 case only
(don't adjust size in that case), instead of playing +-1 tricks (as in,
it's not a off-by-one error). Will think a bit over dinner :)
diff --git a/tccgen.c b/tccgen.c
index 270d11c..896a520 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -5846,8 +5846,10 @@ static void decl_initializer_alloc(CType *type,
AttributeDef *ad, int r,
if (size < 0)
tcc_error("unknown type size");
}
- if (flexible_array)
- size += flexible_array->type.ref->c *
pointed_size(&flexible_array->type) + 1;
+ if (flexible_array
+ /* If the flex member was used in the initializer. */
+ && flexible_array->type.ref->c > 0)
+ size += flexible_array->type.ref->c *
pointed_size(&flexible_array->type);
/* take into account specified alignment if bigger */
if (ad->a.aligned) {
if (ad->a.aligned > align)
Ciao,
Michael.