~sircmpwn/hare-dev

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

[PATCH harec] Fix segfault on not enough params for variadic function call

Details
Message ID
<20220604032243.2773284-1-mail@smlavine.com>
DKIM signature
pass
Download raw message
Patch: +1 -1
As shown by this minimal test case:

fn foo(a: int, v: int...) void = void;

export fn main() void = {
	//foo(0); // << okay
	foo(); // harec segfaults
};

Signed-off-by: Sebastian LaVine <mail@smlavine.com>
---
 src/check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/check.c b/src/check.c
index 558187f..f5980c3 100644
--- a/src/check.c
+++ b/src/check.c
@@ -1211,7 +1211,7 @@ check_expr_call(struct context *ctx,
		}
	}

	if (param && fntype->func.variadism == VARIADISM_HARE) {
	if (param && param->type->array.members && fntype->func.variadism == VARIADISM_HARE) {
		// No variadic arguments, lower to empty slice
		arg = *next = xcalloc(1, sizeof(struct call_argument));
		arg->value = xcalloc(1, sizeof(struct expression));
-- 
2.36.1

[harec/patches] build failed

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CKH111XHJV55.1W44PWQZ0O9MU@cirno>
In-Reply-To
<20220604032243.2773284-1-mail@smlavine.com> (view parent)
DKIM signature
missing
Download raw message
harec/patches: FAILED in 56s

[Fix segfault on not enough params for variadic function call][0] from [Sebastian LaVine][1]

[0]: https://lists.sr.ht/~sircmpwn/hare-dev/patches/32748
[1]: mail@smlavine.com

✓ #773869 SUCCESS harec/patches/alpine.yml  https://builds.sr.ht/~sircmpwn/job/773869
✗ #773870 FAILED  harec/patches/freebsd.yml https://builds.sr.ht/~sircmpwn/job/773870
Details
Message ID
<CKHAQ4L9KCUR.1534HDCODD4JC@attila>
In-Reply-To
<20220604032243.2773284-1-mail@smlavine.com> (view parent)
DKIM signature
pass
Download raw message
> @@ -1211,7 +1211,7 @@ check_expr_call(struct context *ctx,
> 	}
> }
>  
> - if (param && fntype->func.variadism == VARIADISM_HARE) {
> + if (param && param->type->array.members && fntype->func.variadism ==
> VARIADISM_HARE) {
> 	// No variadic arguments, lower to empty slice

You've found the right place for a fix, but the fix is not correct.
param->type may not be an array, and reading param->type->array.members isn't
valid in that case. I think what we really want to check here is that the param
we're looking at is the last one, so perhaps condition like
`if (param && !param->next && fntype->func.variadism == VARIADISM_HARE)` would
be worth trying out.
Details
Message ID
<CKHI1EMKZ5EP.SELXUYJED0P@archlinux-x220>
In-Reply-To
<CKHAQ4L9KCUR.1534HDCODD4JC@attila> (view parent)
DKIM signature
pass
Download raw message
On Sat Jun 4, 2022 at 6:59 AM EDT, Bor Grošelj Simić wrote:
> > @@ -1211,7 +1211,7 @@ check_expr_call(struct context *ctx,
> > 	}
> > }
> >  
> > - if (param && fntype->func.variadism == VARIADISM_HARE) {
> > + if (param && param->type->array.members && fntype->func.variadism ==
> > VARIADISM_HARE) {
> > 	// No variadic arguments, lower to empty slice
>
> You've found the right place for a fix, but the fix is not correct.
> param->type may not be an array, and reading param->type->array.members isn't
> valid in that case. I think what we really want to check here is that the param
> we're looking at is the last one, so perhaps condition like
> `if (param && !param->next && fntype->func.variadism == VARIADISM_HARE)` would
> be worth trying out.

Thanks, that seems to work. And it matches the condition used in the
loop above this section. Sending a v2 shortly.
Reply to thread Export thread (mbox)