Skip to content

Commit d15d4a0

Browse files
MichaelChiricoMichaelChirico
authored
Canonical handling of overflow to remove compiler warning / avoid UB (#362)
* Gemini: canonical handling of overflow to remove compiler warning / avoid UB * shared warning message to reduce noise * Gemini: more tests around NA_INTEGER64 sentinel value * touch-up esp. comments * Gemini: more tests for coverage * Gemini: include results in expect_warning test * delint * Gemini: nocov rather than a fake test * clarifying comment
1 parent f5105c0 commit d15d4a0

5 files changed

Lines changed: 275 additions & 103 deletions

File tree

‎src/integer64.c‎

Lines changed: 60 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# C-Code
33
# S3 atomic 64bit integers for R
44
# (c) 2011-2024 Jens Oehlschägel
5-
# (c) 2025 Michael Chirico
5+
# (c) 2025-2026 Michael Chirico
66
# Licence: GPL2
77
# Provided 'as is', use at your own risk
88
# Created: 2011-12-11
@@ -227,11 +227,13 @@ SEXP as_integer64_bitstring(SEXP x_, SEXP ret_){
227227
for(i=0; i<n; i++){
228228
str = CHAR(STRING_ELT(x_, i));
229229
l = strlen(str);
230-
if (l>BITS_INTEGER64){
230+
// # nocov start: only reachable with invalid bitstring input
231+
if (l > BITS_INTEGER64){
231232
ret[i] = NA_INTEGER64;
232233
naflag = TRUE;
233234
break;
234235
}
236+
// # nocov end
235237
mask = 1;
236238
v = 0;
237239
for (k=l-1; k>=0; k--){
@@ -243,13 +245,13 @@ SEXP as_integer64_bitstring(SEXP x_, SEXP ret_){
243245
ret[i] = v;
244246
R_CheckUserInterrupt();
245247
}
246-
if (naflag)warning(BITSTRING_OVERFLOW_WARNING);
248+
if (naflag)warning(BITSTRING_OVERFLOW_WARNING); // # nocov
247249
return ret_;
248250
}
249251

250252

251253

252-
__attribute__((no_sanitize("signed-integer-overflow"))) SEXP plus_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
254+
SEXP plus_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
253255
long long i, n = LENGTH(ret_);
254256
long long i1, n1 = LENGTH(e1_);
255257
long long i2, n2 = LENGTH(e2_);
@@ -264,7 +266,7 @@ __attribute__((no_sanitize("signed-integer-overflow"))) SEXP plus_integer64(SEXP
264266
return ret_;
265267
}
266268

267-
__attribute__((no_sanitize("signed-integer-overflow"))) SEXP minus_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
269+
SEXP minus_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
268270
long long i, n = LENGTH(ret_);
269271
long long i1, n1 = LENGTH(e1_);
270272
long long i2, n2 = LENGTH(e2_);
@@ -279,7 +281,7 @@ __attribute__((no_sanitize("signed-integer-overflow"))) SEXP minus_integer64(SEX
279281
return ret_;
280282
}
281283

282-
__attribute__((no_sanitize("signed-integer-overflow"))) SEXP diff_integer64(SEXP x_, SEXP lag_, SEXP n_, SEXP ret_){
284+
SEXP diff_integer64(SEXP x_, SEXP lag_, SEXP n_, SEXP ret_){
283285
long long i, n = *((long long *) REAL(n_));
284286
long long * x = (long long *) REAL(x_);
285287
long long * lag = (long long *) REAL(lag_);
@@ -326,7 +328,7 @@ SEXP mod_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
326328
}
327329

328330

329-
__attribute__((no_sanitize("signed-integer-overflow"))) SEXP times_integer64_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
331+
SEXP times_integer64_integer64(SEXP e1_, SEXP e2_, SEXP ret_){
330332
long long i, n = LENGTH(ret_);
331333
long long i1, n1 = LENGTH(e1_);
332334
long long i2, n2 = LENGTH(e2_);
@@ -612,36 +614,31 @@ SEXP sum_integer64(SEXP e1_, SEXP na_rm_, SEXP ret_){
612614
long long i, n = LENGTH(e1_);
613615
long long * e1 = (long long *) REAL(e1_);
614616
long long * ret = (long long *) REAL(ret_);
615-
long long cumsum, tempsum;
616-
cumsum = 0;
617-
if (asLogical(na_rm_)){
618-
for(i=0; i<n; i++){
619-
if (e1[i]!=NA_INTEGER64){
620-
tempsum = cumsum + e1[i];
621-
if (!GOODISUM64(cumsum, e1[i], tempsum)){
622-
warning(INTEGER64_OVERFLOW_WARNING);
623-
ret[0] = NA_INTEGER64;
624-
return ret_;
625-
}
626-
cumsum = tempsum;
627-
}
617+
long long cumsum = 0;
618+
if (asLogical(na_rm_)){
619+
for(i=0; i<n; i++){
620+
if (e1[i]!=NA_INTEGER64){
621+
if (add64_overflow(cumsum, e1[i], &cumsum)){
622+
warning(INTEGER64_OVERFLOW_WARNING);
623+
ret[0] = NA_INTEGER64;
624+
return ret_;
628625
}
629-
}else{
630-
for(i=0; i<n; i++){
631-
if (e1[i]==NA_INTEGER64){
632-
ret[0] = NA_INTEGER64;
633-
return ret_;
634-
}else{
635-
tempsum = cumsum + e1[i];
636-
if (!GOODISUM64(cumsum, e1[i], tempsum)){
637-
warning(INTEGER64_OVERFLOW_WARNING);
638-
ret[0] = NA_INTEGER64;
639-
return ret_;
640-
}
641-
cumsum = tempsum;
642-
}
626+
}
627+
}
628+
}else{
629+
for(i=0; i<n; i++){
630+
if (e1[i]==NA_INTEGER64){
631+
ret[0] = NA_INTEGER64;
632+
return ret_;
633+
}else{
634+
if (add64_overflow(cumsum, e1[i], &cumsum)){
635+
warning(INTEGER64_OVERFLOW_WARNING);
636+
ret[0] = NA_INTEGER64;
637+
return ret_;
643638
}
639+
}
644640
}
641+
}
645642
ret[0] = cumsum;
646643
return ret_;
647644
}
@@ -678,36 +675,31 @@ SEXP prod_integer64(SEXP e1_, SEXP na_rm_, SEXP ret_){
678675
long long i, n = LENGTH(e1_);
679676
long long * e1 = (long long *) REAL(e1_);
680677
long long * ret = (long long *) REAL(ret_);
681-
long long cumprod, tempprod;
682-
cumprod = 1;
683-
if (asLogical(na_rm_)){
684-
for(i=0; i<n; i++){
685-
if (e1[i]!=NA_INTEGER64){
686-
tempprod = cumprod * e1[i];
687-
if (!GOODIPROD64(cumprod, e1[i], tempprod)){
688-
warning(INTEGER64_OVERFLOW_WARNING);
689-
ret[0] = NA_INTEGER64;
690-
return ret_;
691-
}
692-
cumprod = tempprod;
693-
}
678+
long long cumprod = 1;
679+
if (asLogical(na_rm_)){
680+
for(i=0; i<n; i++){
681+
if (e1[i]!=NA_INTEGER64){
682+
if (mul64_overflow(cumprod, e1[i], &cumprod)){
683+
warning(INTEGER64_OVERFLOW_WARNING);
684+
ret[0] = NA_INTEGER64;
685+
return ret_;
694686
}
695-
}else{
696-
for(i=0; i<n; i++){
697-
if (e1[i]==NA_INTEGER64){
698-
ret[0] = NA_INTEGER64;
699-
return ret_;
700-
}else{
701-
tempprod = cumprod * e1[i];
702-
if (!GOODIPROD64(cumprod, e1[i], tempprod)){
703-
warning(INTEGER64_OVERFLOW_WARNING);
704-
ret[0] = NA_INTEGER64;
705-
return ret_;
706-
}
707-
cumprod = tempprod;
708-
}
687+
}
688+
}
689+
}else{
690+
for(i=0; i<n; i++){
691+
if (e1[i]==NA_INTEGER64){
692+
ret[0] = NA_INTEGER64;
693+
return ret_;
694+
}else{
695+
if (mul64_overflow(cumprod, e1[i], &cumprod)){
696+
warning(INTEGER64_OVERFLOW_WARNING);
697+
ret[0] = NA_INTEGER64;
698+
return ret_;
709699
}
700+
}
710701
}
702+
}
711703
ret[0] = cumprod;
712704
return ret_;
713705
}
@@ -1065,7 +1057,7 @@ SEXP as_list_integer64(SEXP x_){
10651057
return x_;
10661058
}
10671059

1068-
__attribute__((no_sanitize("signed-integer-overflow"))) SEXP matmult_integer64_integer64(SEXP x_, SEXP y_, SEXP ret_){
1060+
SEXP matmult_integer64_integer64(SEXP x_, SEXP y_, SEXP ret_){
10691061
long long i, j, k;
10701062
// get dimension of x
10711063
SEXP dim1 = getAttrib(x_, R_DimSymbol);
@@ -1080,7 +1072,7 @@ __attribute__((no_sanitize("signed-integer-overflow"))) SEXP matmult_integer64_i
10801072
long long * y = (long long *) REAL(y_);
10811073
long long * ret = (long long *) REAL(ret_);
10821074
Rboolean naflag = FALSE;
1083-
long long cumsum, tempsum, addValue;
1075+
long long cumsum, addValue;
10841076

10851077
for(i=0; i<nrow1; i++){
10861078
for(j=0; j<ncol2; j++){
@@ -1091,16 +1083,11 @@ __attribute__((no_sanitize("signed-integer-overflow"))) SEXP matmult_integer64_i
10911083
cumsum = NA_INTEGER64;
10921084
break;
10931085
}
1094-
tempsum = cumsum + addValue;
1095-
// for some reason GOODISUM64(cumsum, addValue, tempsum) does not work properly on macos-latest, compared to the others
1096-
// therefore a workaround is tried here by adding the GOODISUM64 logic with long double casting
1097-
if(!GOODISUM64(cumsum, addValue, tempsum) ||
1098-
!((cumsum > 0) ? (((long double) addValue) < ((long double) tempsum)) : ! (((long double) addValue) < ((long double) tempsum)))){
1086+
if(add64_overflow(cumsum, addValue, &cumsum)){
10991087
naflag = TRUE;
11001088
cumsum = NA_INTEGER64;
11011089
break;
11021090
}
1103-
cumsum = tempsum;
11041091
}
11051092
ret[i + j*nrow1] = cumsum;
11061093
}
@@ -1125,7 +1112,7 @@ SEXP matmult_double_integer64(SEXP x_, SEXP y_, SEXP ret_){
11251112
long long * y = (long long *) REAL(y_);
11261113
long long * ret = (long long *) REAL(ret_);
11271114
Rboolean naflag = FALSE;
1128-
long long cumsum, tempsum, addValue;
1115+
long long cumsum, addValue;
11291116
long double longret;
11301117

11311118
for(i=0; i<nrow1; i++){
@@ -1137,13 +1124,11 @@ SEXP matmult_double_integer64(SEXP x_, SEXP y_, SEXP ret_){
11371124
cumsum = NA_INTEGER64;
11381125
break;
11391126
}
1140-
tempsum = cumsum + addValue;
1141-
if(!GOODISUM64(cumsum, addValue, tempsum)){
1127+
if(add64_overflow(cumsum, addValue, &cumsum)){
11421128
naflag = TRUE;
11431129
cumsum = NA_INTEGER64;
11441130
break;
11451131
}
1146-
cumsum = tempsum;
11471132
}
11481133
ret[i + j*nrow1] = cumsum;
11491134
}
@@ -1167,7 +1152,7 @@ SEXP matmult_integer64_double(SEXP x_, SEXP y_, SEXP ret_){
11671152
double * y = REAL(y_);
11681153
long long * ret = (long long *) REAL(ret_);
11691154
Rboolean naflag = FALSE;
1170-
long long cumsum, tempsum, addValue;
1155+
long long cumsum, addValue;
11711156
long double longret;
11721157

11731158
for(i=0; i<nrow1; i++){
@@ -1179,13 +1164,11 @@ SEXP matmult_integer64_double(SEXP x_, SEXP y_, SEXP ret_){
11791164
cumsum = NA_INTEGER64;
11801165
break;
11811166
}
1182-
tempsum = cumsum + addValue;
1183-
if(!GOODISUM64(cumsum, addValue, tempsum)){
1167+
if(add64_overflow(cumsum, addValue, &cumsum)){
11841168
naflag = TRUE;
11851169
cumsum = NA_INTEGER64;
11861170
break;
11871171
}
1188-
cumsum = tempsum;
11891172
}
11901173
ret[i + j*nrow1] = cumsum;
11911174
}

0 commit comments

Comments
 (0)