Integer division by zero in FSE_normalizeM2 about finitestateentropy HOT 8 CLOSED

@Cyan4973 First off, thank you for the very prompt responses. Given what you've said, I am happy to include this very cool work in awesome-c, as it really seems that this issue is a corner case that people wouldn't encounter casually.

from finitestateentropy.

Thanks for the clear reproduction case @Makush .
I'll look into it.

from finitestateentropy.

Is this still an issue?

from finitestateentropy.

Yes it's still an issue.
Huge alphabet size is a fairly specific use case, I don't have any real-world situation where it applies.
But it's nonetheless something to fix.

from finitestateentropy.

@Cyan4973 The main reason I ask is because I maintain awesome-c, and I would like to include your work, but this issue has me concerned.

from finitestateentropy.

The issue still needs to be fixed.
So I need to find some time to do it.

But I wouldn't be that concerned by it.
I'm keeping the bug opened to not forget it, but really, you have to go through some uncommon scenario and invoke a rare interface labelled "experimental" which is not even present in fse.h, plus an additional set of conditions on distributions (which basically means FSE is not the right tool for the task) to have a chance to trigger it.

from finitestateentropy.

FYI, I believe that this might be fixed in the zstd code with the addition of:

if (ToDistribute == 0)
return 0
commit: facebook/zstd@9889bca#diff-27457e92342bf0494cfc4a4ad6ba6bc9

I was able to repro this with another use case and I've fixed it with the addition of that if statement. Unfortunately I don't have my repro code anymore.

from finitestateentropy.

@yohan1234 That code was pulled in here at ea7b345

Perhaps this is no longer an issue now.

from finitestateentropy.