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

compiling with clang sanitizer signals undefined behavior in rand module #21525

Open
ttytm opened this issue May 18, 2024 · 4 comments
Open

compiling with clang sanitizer signals undefined behavior in rand module #21525

ttytm opened this issue May 18, 2024 · 4 comments
Labels
Bug This tag is applied to issues which reports bugs. Status: Confirmed This bug has been confirmed to be valid by a contributor.

Comments

@ttytm
Copy link
Member

ttytm commented May 18, 2024

Describe the bug

// file.v
import rand as _
❯ v -cc clang -cflags -fsanitize=undefined run file.v
runtime error: call to function rand__wyrand__WyRandRNG_free through pointer to incorrect function type 'void (*)(void *)'
# ...
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in... 

Reproduction Steps

above

Expected Behavior

no error

Current Behavior

runtime error about undefined behavior.

Possible Solution

No response

Additional Information/Context

Spreads to other vlib modules that import the rand module.

V version

v0.4.5

Environment details (OS name and version, etc.)

linux amd64

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@ttytm ttytm added the Bug This tag is applied to issues which reports bugs. label May 18, 2024
@spytheman spytheman added the Status: Confirmed This bug has been confirmed to be valid by a contributor. label May 19, 2024
@felipensp
Copy link
Member

What clang version are you using?

@ttytm
Copy link
Member Author

ttytm commented May 22, 2024

What clang version are you using?

Good question. 17.

I tried to do a quick repro with 14 and 15, but didn't run into the error.

@medvednikov
Copy link
Member

Good, they keep improving the sanitizer. Perhaps we can switch to a newer version of wyrand?

@ttytm
Copy link
Member Author

ttytm commented May 23, 2024

Good, they keep improving the sanitizer. Perhaps we can switch to a newer version of wyrand?

Yes, might be the way.

Regarding the update of the wyrand implementation, I checked out the rand module's code related to the error before reporting the issue. I was wondering about the approach of using a global for the seed and providing a public API to update the global, e.g. whether it's really is thread-safe.

__global default_rng &PRNG

With other wyrand implementations I've found, a seed is created on demand without using globals. Of course, without having looked deeply or used the module extensively, I may be missing the bigger picture and the advantages of the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs. Status: Confirmed This bug has been confirmed to be valid by a contributor.
Projects
None yet
Development

No branches or pull requests

4 participants