Fix zero/variable length arrays handling v1 PROPOSED

Sertonix: 1
 Fix zero/variable length arrays handling

 2 files changed, 2 insertions(+), 2 deletions(-)

https://git.alpinelinux.org/apk-tools/tree/src/adb.c?id=cef30b61c1a4c870f23f905423d76a287c22bf02#n478
(I need to figure out later if this code is actually correct.)

[PATCH] Fix zero/variable length arrays handling Export this patch

In 4f206ac1ea (Implement variable length arrays) arrays with the length 0
were considered to be variable length arrays. This may not be true and
caused cproc to segfault when parsing for example sizeof(int[0]). Using
t->prop & PROPVM as check for variable length arrays should always work
correctly.

Michael Forney <mforney@mforney.org>

6 months ago

Thanks for the patch!

However, the distinction between variably modified type and variably
length array is important here.

Consider this example:

	#include <stdio.h>
	int main(void) {
		return sizeof(int (*[1])[(puts("fail"), 1)]) != sizeof(int *);
	}

Here, the type is "array of length 1 of pointer to VLA of int".
This is an array type, and it is also a variably modified type since
there is a VLA in the derivation of the type, but it is not a VLA
itself.

Sertonix <sertonix@posteo.net>

6 months ago

Oh, well then maybe I don't understand cproc enought to fix this issue.

I have noticed some other issues too. Should I put them on todo.sr.ht or
the mailing list?

Next

C23 6.5.3.4p2 says

---
 expr.c | 2 +-
 init.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/expr.c b/expr.c
index aef6fe9..ee570b4 100644
--- a/expr.c
+++ b/expr.c
@@ -1106,7 +1106,7 @@ unaryexpr(struct scope *s)
 			error(&tok.loc, "%s operator applied to incomplete type", tokstr[op]);
 		if (t->kind == TYPEFUNC)
 			error(&tok.loc, "%s operator applied to function type", tokstr[op]);
-		if (t->kind == TYPEARRAY && t->size == 0 && op == TSIZEOF) {
+		if (t->kind == TYPEARRAY && (t->prop & PROPVM) && op == TSIZEOF) {
 			e = mkexpr(EXPRSIZEOF, &typeulong, e);
 			e->u.szof.type = e ? t : e->base->type;
 		} else {
diff --git a/init.c b/init.c
index c8f4128..3da917d 100644
--- a/init.c
+++ b/init.c
@@ -209,7 +209,7 @@ parseinit(struct scope *s, struct type *t)
 	if (t->incomplete) {
 		if (t->kind != TYPEARRAY)
 			error(&tok.loc, "initializer specified for incomplete type");
-	} else if (t->kind == TYPEARRAY && t->size == 0) {
+	} else if (t->kind == TYPEARRAY && (t->prop & PROPVM)) {
 		error(&tok.loc, "initializer specified for variable length array type");
 	}
 	for (;;) {
-- 
2.46.2

Michael Forney <mforney@mforney.org>

6 months ago

"Sertonix" <sertonix@posteo.net> wrote:

Michael Forney <mforney@mforney.org>

6 months ago

So we must not evaluate the expression `(puts("fail"), 1)`.

Technically, arrays with size 0 are not allowed in C (6.7.6.2p1):

Michael Forney <mforney@mforney.org>

6 months ago

Have you found code like this in the wild?

Ideally we'd just fail on this earlier in decl.c:declarator() since
it's a constraint violation, but I suspect this would break a lot
of legacy code (predating C99 flexible array members) using zero-length
arrays in structs.

One idea might be to treat zero-length arrays as incomplete types.
This would make use with sizeof error out and also prevent declaring
variables with zero-length array type. The only problem is with
initialization, since then we'd be unable to distinguish
`int x[] = {1, 2, 3}` from the invalid `int x[0] = {1, 2, 3}`.