Re: recent coverity and rand
On Tue, 2014-10-07 at 10:18 +0300, Tor Lillqvist wrote: > The problem was that it called > comphelper::rng::uniform_real_distribution() with two equal arguments, > which is invalid use of that API, and causes the current > implementation to get stuck in a loop in the boost code. (I didn't > bother checking whether it is possible that the loop eventually will > exit.) Fixed with 4bb67e666b2ca36a5caa5c3f22d0f1e802db527e and > 1b226b02471f2fab5e5e6f79b6593636274d2320 . Excellent. Thanks for the analysis and fix. Kohei ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: recent coverity and rand
> FYI, this change
>
> double random(double min, double max)
> {
> -return floor(((double)rand() / ((unsigned int)RAND_MAX + 1)) * (max -
> min + 1) + min);
> +return comphelper::rng::uniform_real_distribution(min, max);
> }
>
> in sc/source/core/opencl/opencl_device.cxx has made the opencl cppunit
> test run extremely slow, if not hanging.
The problem was that it called comphelper::rng::uniform_real_distribution()
with two equal arguments, which is invalid use of that API, and causes the
current implementation to get stuck in a loop in the boost code. (I didn't
bother checking whether it is possible that the loop eventually will exit.)
Fixed with 4bb67e666b2ca36a5caa5c3f22d0f1e802db527e
and 1b226b02471f2fab5e5e6f79b6593636274d2320 .
--tml
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: recent coverity and rand
On Thu, 2014-10-02 at 16:44 +0100, Caolán McNamara wrote:
> The latest coverity has taken a dislike to "rand" and we've a big block
> of cids, cid#1242372 to cid#1242410 now marked with
> static_checker_DC.WEAK_CRYPTO "Don't call".
>
> We have our own random pool stuff in sal, is there a drop in replacement
> for rand in there somewhere or a common pattern we could follow in
> replacing those ?
FYI, this change
double random(double min, double max)
{
-return floor(((double)rand() / ((unsigned int)RAND_MAX + 1)) * (max - min
+ 1) + min);
+return comphelper::rng::uniform_real_distribution(min, max);
}
in sc/source/core/opencl/opencl_device.cxx has made the opencl cppunit
test run extremely slow, if not hanging. I think I waited about 5
minutes before deciding to Ctrl-C. Switching it back to the rand()
based solution makes the test run normally.
Kohei
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: recent coverity and rand
On Sat, Oct 4, 2014 at 3:23 PM, Caolán McNamara wrote: > On Thu, 2014-10-02 at 23:35 +0200, Michael Stahl wrote: >> * direct usage of boost::random for fancy distributions in Calc, >> sc/source/ui/StatisticsDialogs/RandomNumberGeneratorDialog.cxx >> >> * include/comphelper/random.hxx: >> double uniform() function with [0,1) range >> implemented with boost::random / MersenneTwister for speed > > >> ... so i'd guess that the comphelper/random.hxx approach is most >> promising for general-purpose random numbers (i.e. not crypto); it even >> nicely encapsulates the boost template madness behind a small ABI. >> >> oh, there is also a header in C++11, likely inspired by >> boost::random; i wonder if our new baseline toolchains have support for >> this... actually GCC 4.5 release notes list it as a new feature, and >> MSVC 2012 has it too: > > Hmm, little trap here, uniform_int_distribution takes [a,b] range while > uniform_real_distribution takes [a,b). so: (int)(floor(uniform_real_distributtion(a, b+1)) ? Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: recent coverity and rand
On Thu, 2014-10-02 at 23:35 +0200, Michael Stahl wrote: > * direct usage of boost::random for fancy distributions in Calc, > sc/source/ui/StatisticsDialogs/RandomNumberGeneratorDialog.cxx > > * include/comphelper/random.hxx: > double uniform() function with [0,1) range > implemented with boost::random / MersenneTwister for speed > ... so i'd guess that the comphelper/random.hxx approach is most > promising for general-purpose random numbers (i.e. not crypto); it even > nicely encapsulates the boost template madness behind a small ABI. > > oh, there is also a header in C++11, likely inspired by > boost::random; i wonder if our new baseline toolchains have support for > this... actually GCC 4.5 release notes list it as a new feature, and > MSVC 2012 has it too: Hmm, little trap here, uniform_int_distribution takes [a,b] range while uniform_real_distribution takes [a,b). C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: recent coverity and rand
On 02/10/14 17:44, Caolán McNamara wrote: > The latest coverity has taken a dislike to "rand" and we've a big block > of cids, cid#1242372 to cid#1242410 now marked with > static_checker_DC.WEAK_CRYPTO "Don't call". there were definitely bad implementations of standard C library random functions; no idea if that is the case on current desktop platforms... iirc couple years ago users were actually complaining about bad random numbers in Calc generated from rand() on Windows. > We have our own random pool stuff in sal, is there a drop in replacement > for rand in there somewhere or a common pattern we could follow in > replacing those ? let's see what we've got: * direct usage of boost::random for fancy distributions in Calc, sc/source/ui/StatisticsDialogs/RandomNumberGeneratorDialog.cxx * include/comphelper/random.hxx: double uniform() function with [0,1) range implemented with boost::random / MersenneTwister for speed * include/rtl/random.h rtlRandomPool MD5-based PRNG, probably designed for cryptographic purposes, although mostly untouched since the 90s and perhaps insufficient for crypto today; it doesn't even appear to be seeded with *real* entropy... (if there are actual cryptographic uses of this they probably should be replaced with something from NSS) ... so i'd guess that the comphelper/random.hxx approach is most promising for general-purpose random numbers (i.e. not crypto); it even nicely encapsulates the boost template madness behind a small ABI. oh, there is also a header in C++11, likely inspired by boost::random; i wonder if our new baseline toolchains have support for this... actually GCC 4.5 release notes list it as a new feature, and MSVC 2012 has it too: http://msdn.microsoft.com/en-us/library/bb982398%28v=vs.110%29.aspx ... so perhaps is usable for us already. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
recent coverity and rand
The latest coverity has taken a dislike to "rand" and we've a big block of cids, cid#1242372 to cid#1242410 now marked with static_checker_DC.WEAK_CRYPTO "Don't call". We have our own random pool stuff in sal, is there a drop in replacement for rand in there somewhere or a common pattern we could follow in replacing those ? C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
