vlang / v

Simple, fast, safe, compiled language for developing maintainable software. Compiles itself in <1s with zero library dependencies. Supports automatic C => V translation. https://vlang.io
MIT License
35.53k stars 2.14k forks source link

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

Open ttytm opened 1 month ago

ttytm commented 1 month ago

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.

felipensp commented 1 month ago

What clang version are you using?

ttytm commented 1 month ago

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 commented 1 month ago

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

ttytm commented 1 month ago

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.

https://github.com/vlang/v/blob/27833e0dbe6a81f4f0154cadaf55197abf897b46/vlib/rand/rand.v#L490

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.

spytheman commented 1 month ago

whether it's really is thread-safe

It is not. It is there for convenience.

People that want thread safe random number generating, can create their own thread specific random number generator instances, which rand also supports, and then call methods on those, instead of calling say rand.i64() etc, which uses the global instance.