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

cranelift-fuzzgen: use a different namespace #4795

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

jameysharp
Copy link
Contributor

Otherwise I get a panic with "Duplicate function with name u0:1 found!"
at fuzz/fuzz_targets/cranelift-fuzzgen.rs:76:10.

Otherwise I get a panic with "Duplicate function with name u0:1 found!"
at fuzz/fuzz_targets/cranelift-fuzzgen.rs:76:10.
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Aug 26, 2022
@jameysharp jameysharp enabled auto-merge (squash) August 26, 2022 22:59
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the randomly generated input contains other functions in the u0:x namespace presumably? A little more reasoning in the comment would be helpful, for the curious reader; but moving to a different toplevel function name (u1:0 rather than u0:1) shouldn't hurt anything if consistent. So this LGTM.

Avoiding setting r+ bit because of above request and auto-merge but happy to approve with a slightly more detailed comment.

@jameysharp
Copy link
Contributor Author

It's actually because the trampolines and renamed functions are in namespace 0. References to other (non-existent) functions should be fine since they get renamed. I think?

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense; happy to see this merge as-is then, just to get the fuzzer fix in. Thanks!

@jameysharp jameysharp merged commit 573ae0c into bytecodealliance:main Aug 26, 2022
@jameysharp jameysharp deleted the unique-namespace branch August 26, 2022 23:58
@afonso360
Copy link
Contributor

Thanks! And sorry for the mess this created.

The hard coded function names should go away when we start generating random function names.

@jameysharp
Copy link
Contributor Author

No worries! I think the only "mess" was my flailing around in different issues trying to figure out what happened. 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants