Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential crash with non-integer array size #585

Closed
ehaas opened this issue Nov 27, 2023 · 3 comments · Fixed by #601
Closed

Potential crash with non-integer array size #585

ehaas opened this issue Nov 27, 2023 · 3 comments · Fixed by #601
Labels
bug Something isn't working crash A bug that causes the compiler to crash

Comments

@ehaas
Copy link
Collaborator

ehaas commented Nov 27, 2023

Strange one found via fuzzing. If the array size specifier is .invalid:

[-w-f]" "f [f]" "f[-w-f]
panic: reached unreachable code
/Users/ehaas/source/arocc/src/aro/Type.zig:2345:21: 0x1080024ce in fromType (arocc)
            else => unreachable,
                    ^
/Users/ehaas/source/arocc/src/aro/Parser.zig:435:30: 0x108000b88 in typeStr (arocc)
    if (Type.Builder.fromType(ty).str(p.comp.langopts)) |str| return str;
                             ^
/Users/ehaas/source/arocc/src/aro/Parser.zig:2984:70: 0x10817ad8b in directDeclarator (arocc)
            try p.errStr(.array_size_non_int, size_tok, try p.typeStr(size.ty));
                                                                     ^
/Users/ehaas/source/arocc/src/aro/Parser.zig:2879:38: 0x1080e7d99 in declarator (arocc)
        d.ty = try p.directDeclarator(d.ty, &d, kind);
                                     ^
/Users/ehaas/source/arocc/src/aro/Parser.zig:1746:31: 0x1080e529c in initDeclarator (arocc)
        .d = (try p.declarator(decl_spec.ty, .normal)) orelse return null,
                              ^
@ehaas ehaas added bug Something isn't working crash A bug that causes the compiler to crash labels Nov 27, 2023
@Vexu
Copy link
Owner

Vexu commented Nov 27, 2023

Fixed by something like:

diff --git a/src/aro/Parser.zig b/src/aro/Parser.zig
index ebaa318..3b84321 100644
--- a/src/aro/Parser.zig
+++ b/src/aro/Parser.zig
@@ -2980,6 +2980,7 @@ fn directDeclarator(p: *Parser, base_type: Type, d: *Declarator, kind: Declarato
         if (max_bits > 61) max_bits = 61;
         const max_bytes = (@as(u64, 1) << @truncate(max_bits)) - 1;
 
+        if (size.ty.is(.invalid)) return Type.invalid;
         if (!size.ty.isInt()) {
             try p.errStr(.array_size_non_int, size_tok, try p.typeStr(size.ty));
             return error.ParsingFailed;

@ehaas
Copy link
Collaborator Author

ehaas commented Nov 28, 2023

Here's an alternate path to trigger it:

struct Foo {
    int a;
} a;

char *f(char *arg) {
    return arg + a?a;
}

Do you think we should check in the typeStr functions? I'm guessing any place we call that could potentially have an invalid type

@Vexu
Copy link
Owner

Vexu commented Nov 28, 2023

That would solve the issue but it'd be nicer to properly handle invalid where needed. Maybe we could add a release mode check for invalid in typeStr* so that release modes don't crash?

ehaas added a commit to ehaas/arocc that referenced this issue Dec 14, 2023
Closes Vexu#585

Co-authored-by: Veikka Tuominen <git@vexu.eu>
ehaas added a commit to ehaas/arocc that referenced this issue Dec 15, 2023
Closes Vexu#585

Co-authored-by: Veikka Tuominen <git@vexu.eu>
@Vexu Vexu linked a pull request Feb 7, 2024 that will close this issue
@Vexu Vexu closed this as completed in #601 Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash A bug that causes the compiler to crash
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants