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

Call to enable_result_type_inference is not safe #275

Closed
Danacus opened this issue Aug 16, 2023 · 3 comments · Fixed by #353
Closed

Call to enable_result_type_inference is not safe #275

Danacus opened this issue Aug 16, 2023 · 3 comments · Fixed by #353
Labels
bug Something isn't working

Comments

@Danacus
Copy link
Contributor

Danacus commented Aug 16, 2023

Calling enable_result_type_inference on OperationBuilder may trigger undefined behavior if types cannot be inferred, since mlirOperationCreate returns a nullptr in this case. OperationBuilder::build does not check for nullity, and as such an invalid Operation may be constructed, causing undefined behavior when it is accessed or dropped.

Code to reproduce:

{
    let _ = OperationBuilder::new("test", location)
        .enable_result_type_inference()
        .build();
}

Suggested fix: check for nullity before calling Operation::from_raw in OperationBuilder::build and return a Result<Operation, _> or simply panic.

@Danacus
Copy link
Contributor Author

Danacus commented Aug 16, 2023

After looking into it a bit more, I now think that this only really happens if you use an unregistered/unloaded dialect, so in that sense it's related to #23. I was writing tests for #274 and was using custom test dialects that weren't registered to MLIR, hence issues ocurred when trying to use operation interfaces such as InferTypeOpInterface.

@raviqqe
Copy link
Owner

raviqqe commented Aug 22, 2023

Thanks for the research!

I see. The unregistered dialects should be fixed asap...

@raviqqe raviqqe closed this as completed Aug 22, 2023
@raviqqe raviqqe reopened this Aug 22, 2023
@raviqqe
Copy link
Owner

raviqqe commented Aug 22, 2023

Sorry, I closed by mistake!

@raviqqe raviqqe added the bug Something isn't working label Aug 27, 2023
@raviqqe raviqqe mentioned this issue Sep 30, 2023
45 tasks
raviqqe added a commit that referenced this issue Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants