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

Type conversion (maybe corrupting data)? #409

Closed
sn248 opened this issue Feb 3, 2024 · 5 comments
Closed

Type conversion (maybe corrupting data)? #409

sn248 opened this issue Feb 3, 2024 · 5 comments

Comments

@sn248
Copy link

sn248 commented Feb 3, 2024

Hi

I am trying to update my R package sundialr which is an interface to some of the solvers in SUNDIALS. The CRAN checks for my package give -Woverflow warnings. I discussed this issue in r-package-devel forum where another user thought I should bring it to the attention of the library since it may be corrupting data.

See the entire issue here -

https://stat.ethz.ch/pipermail/r-package-devel/2024q1/010431.html

I am not able to submit to CRAN due to this error.

Specifically, these are the warnings I am getting

Flavor: r-devel-windows-x86_64
Check: whether package can be installed, Result: WARNING
Found the following significant warnings:
./sundials/sundials/sundials_hashmap.h:26:48: warning: conversion from
'long long unsigned int' to 'long unsigned int' changes value from
'14695981039346656037' to '2216829733' [-Woverflow]
./sundials/sundials/sundials_hashmap.h:27:48: warning: conversion from
'long long unsigned int' to 'long unsigned int' changes value from
'1099511628211' to '435' [-Woverflow]
sundials/sundials/sundials_hashmap.h:26:48: warning: conversion from
'long long unsigned int' to 'long unsigned int' changes value from
'14695981039346656037' to '2216829733' [-Woverflow]
sundials/sundials/sundials_hashmap.h:27:48: warning: conversion from
'long long unsigned int' to 'long unsigned int' changes value from
'1099511628211' to '435' [-Woverflow]
sundials/sundials/sundials_profiler.c:71:24: warning: function
declaration isn't a prototype [-Wstrict-prototypes]
See 'd:/RCompile/CRANincoming/R-devel/sundialr.Rcheck/00install.out' for
details.
Used C++ compiler: 'g++.exe (GCC) 12.3.0'

and

Flavor: r-devel-linux-x86_64-debian-gcc
Check: whether package can be installed, Result: WARNING
Found the following significant warnings:
sundials/sundials/sundials_profiler.c:71:41: warning: a function
declaration without a prototype is deprecated in all versions of C
[-Wstrict-prototypes]
See '/srv/hornik/tmp/CRAN/sundialr.Rcheck/00install.out' for details.
Used C++ compiler: 'Debian clang version 17.0.6 (5)'

Any update on this would be highly appreciated!

@s-u
Copy link

s-u commented Feb 4, 2024

To be more specific here (and not rely on external links) - in several places the sundials C code assumes that the long type is a 64-bit integer type, however, that assumption is wrong. The C standard defines the long type as at least 32-bit and on Windows it is exactly 32-bit. Above are some instances listed where 64-bit constants are cast to unsigned long and thus truncated to 32-bit, but generally, there are more places in the code where the long type is used incorrectly.

To fix, any uses of unsigned long where 64-bit integer is expected should be changed to uint64_t (and any signed use to int64_t) which is guaranteed to be 64-bit integer type.

@balos1
Copy link
Member

balos1 commented Feb 5, 2024

@s-u Yes, this is something we would like to fix now that we will not be tied to supporting C89 with SUNDIALS 7.0.0

@balos1
Copy link
Member

balos1 commented Feb 6, 2024

Related #249.

@s-u
Copy link

s-u commented Feb 6, 2024

@balos1 great!

Not sure I get your C89 comment, though, since you can't get away without using C99 since long long is only defined since C99 - but so are all the int64_t types so there is really no way to write any portable 64-bit code without it. Not that it matters IMHO as it is 25 years later and everyone supports it ...

@s-u
Copy link

s-u commented Mar 1, 2024

Thanks, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants