-
Notifications
You must be signed in to change notification settings - Fork 833
[EH] Add validation for new instructions #6185
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2446,7 +2446,75 @@ void FunctionValidator::visitTry(Try* curr) { | |
| } | ||
|
|
||
| void FunctionValidator::visitTryTable(TryTable* curr) { | ||
| // TODO | ||
| shouldBeTrue( | ||
| getModule()->features.hasExceptionHandling(), | ||
| curr, | ||
| "try_table requires exception-handling [--enable-exception-handling]"); | ||
| if (curr->type != Type::unreachable) { | ||
| shouldBeSubType(curr->body->type, | ||
| curr->type, | ||
| curr->body, | ||
| "try_table's type does not match try_table body's type"); | ||
| } | ||
|
|
||
| shouldBeEqual(curr->catchTags.size(), | ||
| curr->catchDests.size(), | ||
| curr, | ||
| "the number of catch tags and catch destinations do not match"); | ||
| shouldBeEqual(curr->catchTags.size(), | ||
| curr->catchRefs.size(), | ||
| curr, | ||
| "the number of catch tags and catch refs do not match"); | ||
| shouldBeEqual(curr->catchTags.size(), | ||
| curr->sentTypes.size(), | ||
| curr, | ||
| "the number of catch tags and sent types do not match"); | ||
|
|
||
| const char* invalidSentTypeMsg = "invalid catch sent type information"; | ||
| Type exnref = Type(HeapType::exn, Nullable); | ||
| for (Index i = 0; i < curr->catchTags.size(); i++) { | ||
| auto sentType = curr->sentTypes[i]; | ||
| size_t tagTypeSize; | ||
|
|
||
| Name tagName = curr->catchTags[i]; | ||
| if (!tagName) { // catch_all or catch_all_ref | ||
| tagTypeSize = 0; | ||
| } else { // catch or catch_ref | ||
| // Check tag validity | ||
| auto* tag = getModule()->getTagOrNull(tagName); | ||
| if (!shouldBeTrue(tag != nullptr, curr, "")) { | ||
| getStream() << "catch's tag name is invalid: " << tagName << "\n"; | ||
| } else if (!shouldBeEqual(tag->sig.results, Type(Type::none), curr, "")) { | ||
| getStream() | ||
| << "catch's tag (" << tagName | ||
| << ") has result values, which is not allowed for exception handling"; | ||
| } | ||
|
|
||
| // tagType and sentType should be the same (except for the possible exnref | ||
| // at the end of sentType) | ||
| auto tagType = tag->sig.params; | ||
| tagTypeSize = tagType.size(); | ||
| for (Index j = 0; j < tagType.size(); j++) { | ||
| shouldBeEqual(tagType[j], sentType[j], curr, invalidSentTypeMsg); | ||
| } | ||
| } | ||
|
|
||
| // If this is catch_ref or catch_all_ref, sentType.size() should be | ||
| // tagType.size() + 1 because there is an exrnef tacked at the end. If | ||
| // this is catch/catch_all, the two sizes should be the same. | ||
| if (curr->catchRefs[i]) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to check that the sizes match when this is false as well. Alternatively, when there is no extra exnref, you can just check
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was actually incorrect because even if there is no values to send,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nevermind the comment. Didn't know
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ended up rewriting much of the loop, but I think I an now checking both cases. PTAL |
||
| if (shouldBeTrue( | ||
| sentType.size() == tagTypeSize + 1, curr, invalidSentTypeMsg)) { | ||
| shouldBeEqual( | ||
| sentType[sentType.size() - 1], exnref, curr, invalidSentTypeMsg); | ||
| } | ||
| } else { | ||
| shouldBeTrue(sentType.size() == tagTypeSize, curr, invalidSentTypeMsg); | ||
| } | ||
|
|
||
| // Note catch destinations with sent types | ||
| noteBreak(curr->catchDests[i], curr->sentTypes[i], curr); | ||
| } | ||
| } | ||
|
|
||
| void FunctionValidator::visitThrow(Throw* curr) { | ||
|
|
@@ -2470,9 +2538,10 @@ void FunctionValidator::visitThrow(Throw* curr) { | |
| Type(Type::none), | ||
| curr, | ||
| "tags with result types must not be used for exception handling"); | ||
| if (!shouldBeTrue(curr->operands.size() == tag->sig.params.size(), | ||
| curr, | ||
| "tag's param numbers must match")) { | ||
| if (!shouldBeEqual(curr->operands.size(), | ||
| tag->sig.params.size(), | ||
| curr, | ||
| "tag's param numbers must match")) { | ||
| return; | ||
| } | ||
| size_t i = 0; | ||
|
|
@@ -2524,7 +2593,11 @@ void FunctionValidator::visitTupleMake(TupleMake* curr) { | |
| } | ||
|
|
||
| void FunctionValidator::visitThrowRef(ThrowRef* curr) { | ||
| // TODO | ||
| Type exnref = Type(HeapType::exn, Nullable); | ||
| shouldBeSubType(curr->exnref->type, | ||
| exnref, | ||
| curr, | ||
| "throw_ref's argument should be a subtype of exnref"); | ||
| } | ||
|
|
||
| void FunctionValidator::visitTupleExtract(TupleExtract* curr) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| (assert_invalid | ||
| (module | ||
| (tag $e-i32 (param i32)) | ||
| (func $f0 | ||
| (try_table | ||
| (i32.const 0) | ||
| ) | ||
| ) | ||
| ) | ||
| "try_table's type does not match try_table body's type" | ||
| ) | ||
|
|
||
| (assert_invalid | ||
| (module | ||
| (tag $e-i32 (param i32)) | ||
| (func $f0 | ||
| (throw $e-i32 (f32.const 0)) | ||
| ) | ||
| ) | ||
| "tag param types must match" | ||
| ) | ||
|
|
||
| (assert_invalid | ||
| (module | ||
| (tag $e-i32 (param i32 f32)) | ||
| (func $f0 | ||
| (throw $e-i32 (f32.const 0)) | ||
| ) | ||
| ) | ||
| "tag's param numbers must match" | ||
| ) | ||
|
|
||
| (assert_invalid | ||
| (module | ||
| (func $f0 | ||
| (block $l | ||
| (try_table (catch $e $l)) | ||
| ) | ||
| ) | ||
| ) | ||
| "catch's tag name is invalid: e" | ||
| ) | ||
|
|
||
| (assert_invalid | ||
| (module | ||
| (tag $e (param i32) (result i32)) | ||
| (func $f0 | ||
| (throw $e (i32.const 5)) | ||
| ) | ||
| ) | ||
| "tags with result types must not be used for exception handling" | ||
| ) | ||
|
|
||
| (assert_invalid | ||
| (module | ||
| (tag $e (param i32) (result i32)) | ||
| (func $f0 | ||
| (block $l | ||
| (try_table (catch $e $l)) | ||
| ) | ||
| ) | ||
| ) | ||
| "catch's tag (e) has result values, which is not allowed for exception handling" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be combined with the
elsecase into a singleshouldBeSubType, since the only subtype ofType::unreachableisType::unreachable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done