In testing pqR on Solaris SPARC systems, I have found two bugs that are also present in recent R Core versions. You can see the bugs and fixes at the following URLs: https://github.com/radfordneal/pqR/commit/739a4960a4d8f3a3b20cfc311518369576689f37 https://github.com/radfordneal/pqR/commit/339b7286c7b43dcc6b00e51515772f1d7dce7858 The first bug, in nls, is most likely to occur on a 64-bit big-endian system, but will occur with low probability on most platforms. The second bug, in readBin, may occur on systems in which unaligned access to data more than one byte in size is an error, depending on details of the compiler. It showed up with gcc 4.9.2 on a SPARC system. The fix slightly changes the error behaviuor, signaling an error on inappropriate reads before reading any data, rather than after reading one (but not all) items as before. Radford Neal
On 14/07/2015 6:08 PM, Radford Neal wrote:> In testing pqR on Solaris SPARC systems, I have found two bugs that > are also present in recent R Core versions. You can see the bugs and > fixes at the following URLs: > > https://github.com/radfordneal/pqR/commit/739a4960a4d8f3a3b20cfc311518369576689f37Thanks for the report. Just one followup on this one: There are two sections of code that are really similar. Your patch applies to the code in port_nlsb(), but there's a very similar test in port_nlminb(), which is called from nlminb() in R. Do you think it would be a good idea to apply the same patch there as well? It doesn't look like it would hurt, but I don't know this code at all, so it might be unnecessary. Duncan Murdoch> > https://github.com/radfordneal/pqR/commit/339b7286c7b43dcc6b00e51515772f1d7dce7858 > > The first bug, in nls, is most likely to occur on a 64-bit big-endian > system, but will occur with low probability on most platforms. > > The second bug, in readBin, may occur on systems in which unaligned > access to data more than one byte in size is an error, depending on > details of the compiler. It showed up with gcc 4.9.2 on a SPARC > system. The fix slightly changes the error behaviuor, signaling an > error on inappropriate reads before reading any data, rather than > after reading one (but not all) items as before. > > Radford Neal > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >
On Tue, Jul 14, 2015 at 07:52:56PM -0400, Duncan Murdoch wrote:> On 14/07/2015 6:08 PM, Radford Neal wrote: > > In testing pqR on Solaris SPARC systems, I have found two bugs that > > are also present in recent R Core versions. You can see the bugs and > > fixes at the following URLs: > > > > https://github.com/radfordneal/pqR/commit/739a4960a4d8f3a3b20cfc311518369576689f37 > > Thanks for the report. Just one followup on this one: > > There are two sections of code that are really similar. Your patch > applies to the code in port_nlsb(), but there's a very similar test in > port_nlminb(), which is called from nlminb() in R. Do you think it > would be a good idea to apply the same patch there as well? It doesn't > look like it would hurt, but I don't know this code at all, so it might > be unnecessary.Looking at nlminb, it seems that this bug doesn't exist there. The R code sets low <- upp <- NULL, as in nls, but later there is an "else low <- upp <- numeric()" that ensures that low and upp are never actually NULL. This may have been a fix for the bug showing up in nlminb that was not applied to nls as well (and of course, the fix didn't delete the now pointless low <- upp <- NULL). The nlminb code might be a better fix, stylistically, after removing "low <- upp <- NULL", though it seems that both it and my fix for nls should work. Of course, both assume that the call of the C function is done only from this R code, so no other types for low and upp are possible. And really the whole thing ought to be rewritten, since the .Call functions modify variables without any regard for whether or not their values are shared. Radford Neal
On 14/07/2015 6:08 PM, Radford Neal wrote:> In testing pqR on Solaris SPARC systems, I have found two bugs that > are also present in recent R Core versions. You can see the bugs and > fixes at the following URLs: > > https://github.com/radfordneal/pqR/commit/739a4960a4d8f3a3b20cfc311518369576689f37 > > https://github.com/radfordneal/pqR/commit/339b7286c7b43dcc6b00e51515772f1d7dce7858 > > The first bug, in nls, is most likely to occur on a 64-bit big-endian > system, but will occur with low probability on most platforms. > > The second bug, in readBin, may occur on systems in which unaligned > access to data more than one byte in size is an error, depending on > details of the compiler. It showed up with gcc 4.9.2 on a SPARC > system. The fix slightly changes the error behaviuor, signaling an > error on inappropriate reads before reading any data, rather than > after reading one (but not all) items as before.I've now taken a look at the second bug. It's a little harder to apply your patch, since your code is based on an old version of R; current R has more error checking and allows long vector results from this function. But it's basically just a matter of being careful not to make changes to the newer code. So this one should go into R-patched more or less as-is. Duncan Murdoch