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

More fuzzer-discovered crash fixes #707

Merged
merged 7 commits into from
May 23, 2024
Merged

More fuzzer-discovered crash fixes #707

merged 7 commits into from
May 23, 2024

Conversation

ehaas
Copy link
Collaborator

@ehaas ehaas commented May 10, 2024

No description provided.

@Vexu
Copy link
Owner

Vexu commented May 19, 2024

Parser: add TODO for complex fp16/_Float16 types

Can you give an example of the crash this prevents?

@ehaas
Copy link
Collaborator Author

ehaas commented May 19, 2024

_Complex float x = 1.0f16 + 2i;

@ehaas
Copy link
Collaborator Author

ehaas commented May 20, 2024

Should I add this as a test with a TESTS_SKIPPED?

@Vexu
Copy link
Owner

Vexu commented May 20, 2024

The TODO could be avoided by reversing #454 for _Float16 since Clang does seem to support it and casting __fp16 to float:

diff --git a/src/aro/Parser.zig b/src/aro/Parser.zig
index 5bb1784..77eea6c 100644
--- a/src/aro/Parser.zig
+++ b/src/aro/Parser.zig
@@ -5758,8 +5758,8 @@ pub const Result = struct {
                 // No `_Complex _Float16`
                 .{ .invalid, .float16 },
             };
-            const a_spec = a.ty.canonicalize(.standard).specifier;
-            const b_spec = b.ty.canonicalize(.standard).specifier;
+            var a_spec = a.ty.canonicalize(.standard).specifier;
+            var b_spec = b.ty.canonicalize(.standard).specifier;
             if (p.comp.target.c_type_bit_size(.longdouble) == 128) {
                 if (try a.floatConversion(b, a_spec, b_spec, p, float_types[0])) return;
             }
@@ -5771,9 +5771,18 @@ pub const Result = struct {
             if (p.comp.target.c_type_bit_size(.longdouble) == 64) {
                 if (try a.floatConversion(b, a_spec, b_spec, p, float_types[0])) return;
             }
+            if (a_spec == .fp16 and b.ty.isComplex()) {
+                try a.floatCast(p, .{ .specifier =  .float });
+                a_spec = .float;
+            }
+            if (b_spec == .fp16 and a.ty.isComplex()) {
+                try b.floatCast(p, .{ .specifier =  .float });
+                b_spec = .float;
+            }
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[3])) return;
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[4])) return;
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[5])) return;
+            unreachable;
         }
 
         if (a.ty.eql(b.ty, p.comp, true)) {

@ehaas
Copy link
Collaborator Author

ehaas commented May 22, 2024

I must have tested on an ancient version of clang - looks like _Complex _Float16 support was added in clang 15, so I just went ahead and added that.

@Vexu Vexu merged commit d662d5f into Vexu:master May 23, 2024
3 checks passed
@ehaas ehaas deleted the crash-fixes branch May 23, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants