~mcf/cproc

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH] Fix zero/variable length arrays handling

Details
Message ID
<D559A6MN4901.3SAPG8K2M50W4@posteo.net>
DKIM signature
pass
Download raw message
Patch: +2 -2
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.

---
 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
Details
Message ID
<2M97OEBFHTNP0.2YVV8LW7320D5@mforney.org>
In-Reply-To
<D559A6MN4901.3SAPG8K2M50W4@posteo.net> (view parent)
DKIM signature
pass
Download raw message
"Sertonix" <sertonix@posteo.net> wrote:
> 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.

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.

C23 6.5.3.4p2 says

> If the type of the operand is a variable length array type, the
> operand is evaluated; otherwise, the operand is not evaluated and
> the result is an integer constant.

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

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

> If the expression is a constant expression, it shall have a value
> greater than zero.

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}`.
Details
Message ID
<D55QRKZK3FEH.2HUSK1YDETBM8@posteo.net>
In-Reply-To
<2M97OEBFHTNP0.2YVV8LW7320D5@mforney.org> (view parent)
DKIM signature
pass
Download raw message
> > 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.
>
> 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.

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?

> C23 6.5.3.4p2 says
>
> > If the type of the operand is a variable length array type, the
> > operand is evaluated; otherwise, the operand is not evaluated and
> > the result is an integer constant.
>
> So we must not evaluate the expression `(puts("fail"), 1)`.
>
> Technically, arrays with size 0 are not allowed in C (6.7.6.2p1):

Yes, I am aware. I *thought* it would be easy to support that gcc
extension. Whenever or not cproc will support it in the end it would
probably be good to add it to doc/extensions.md.

> > If the expression is a constant expression, it shall have a value
> > greater than zero.
>
> Have you found code like this in the wild?

I envountered this in the apk-tools code (ADBI_NUM_ENTRIES is defined as 0):
> o = adb_r_deref(db, v, 0, sizeof(adb_val_t[ADBI_NUM_ENTRIES]));
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.)

> 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}`.

It is hard to tell which coded uses this cause cproc seems to accidentally
compile it without warning/error before 4f206ac1ea1.
Reply to thread Export thread (mbox)