Skip to content

Use ceiling for history frequency calculation#81

Merged
petedodd merged 1 commit into
mainfrom
Robadob-patch-history-frequency
Apr 13, 2026
Merged

Use ceiling for history frequency calculation#81
petedodd merged 1 commit into
mainfrom
Robadob-patch-history-frequency

Conversation

@Robadob

@Robadob Robadob commented Apr 13, 2026

Copy link
Copy Markdown
Member

Change integer division to use ceiling function for history frequency. This should resolve a potential out of bounds array access when writing history where steps does not cleanly divide logging frequency.

I think this will resolve the memory-leak reported by CRAN, via some pen & paper maths, however I wasn't able to reproduce the valgrind error locally so I'm unable to confirm that.

Change integer division to use ceiling function for history frequency. This should resolve a potential out of bounds array access when writing history where steps does not cleanly divide logging frequency.
@Robadob Robadob self-assigned this Apr 13, 2026
@Robadob Robadob added the bug Something isn't working label Apr 13, 2026
@Robadob

Robadob commented Apr 13, 2026

Copy link
Copy Markdown
Member Author

This is based on the CRAN valgrind log

Flavor: r-devel-linux-x86_64-debian-special-valgrind
Check: Post-processing issues found for valgrind, Result: WARNING
  File: tests/testthat.Rout
  ==148154== Memcheck, a memory error detector
  ==148154== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
  ==148154== Using Valgrind-3.25.1 and LibVEX; rerun with -h for copyright info
  ==148154== Command: /home/hornik/tmp/R-d-gcc-valg/bin/exec/R -f testthat.R --restore --save --no-readline --vanilla
  ==148154==
  ==148154== Invalid write of size 8
  ==148154==    at 0x2168F417: Simulation::stepHistory() (/home/hornik/tmp/CRAN_special_valgrind/golden.Rcheck/00_pkg_src/golden/src/Simulation.cpp:298)
  ==148154==    by 0x2169A63F: Simulation::run(Rcpp::Vector<19, Rcpp::PreserveStorage>) (/home/hornik/tmp/CRAN_special_valgrind/golden.Rcheck/00_pkg_src/golden/src/Simulation.cpp:105)
  ==148154==    by 0x2169FE72: run_simulation(Rcpp::Vector<19, Rcpp::PreserveStorage>, Rcpp::Vector<19, Rcpp::PreserveStorage>) (/home/hornik/tmp/CRAN_special_valgrind/golden.Rcheck/00_pkg_src/golden/src/rcpp_interface.cpp:144)
  ==148154==    by 0x21683DFE: _golden_run_simulation (/home/hornik/tmp/CRAN_special_valgrind/golden.Rcheck/00_pkg_src/golden/src/RcppExports.cpp:33)
  ==148154==    by 0x4966289: R_doDotCall (/home/hornik/src/R/src/main/dotcode.c:757)
  ==148154==    by 0x49A3BA0: bcEval_loop (/home/hornik/src/R/src/main/eval.c:8682)
  ==148154==    by 0x49B3BCA: bcEval (/home/hornik/src/R/src/main/eval.c:7515)
  ==148154==    by 0x49B3BCA: bcEval (/home/hornik/src/R/src/main/eval.c:7500)
  ==148154==    by 0x49B3F7A: Rf_eval (/home/hornik/src/R/src/main/eval.c:1167)
  ==148154==    by 0x49B5EDD: R_execClosure (/home/hornik/src/R/src/main/eval.c:2389)
  ==148154==    by 0x49B6C98: applyClosure_core (/home/hornik/src/R/src/main/eval.c:2302)
  ==148154==    by 0x49B40B9: Rf_applyClosure (/home/hornik/src/R/src/main/eval.c:2324)
  ==148154==    by 0x49B40B9: Rf_eval (/home/hornik/src/R/src/main/eval.c:1280)
  ==148154==    by 0x49B910C: do_set (/home/hornik/src/R/src/main/eval.c:3581)
  ==148154==  Address 0x21ea98a0 is 3,840 bytes inside a block of size 7,960 alloc'd
  ==148154==    at 0x4849818: malloc (coregrind/m_replacemalloc/vg_replace_malloc.c:446)
  ==148154==    by 0x49F9A53: GetNewPage (/home/hornik/src/R/src/main/memory.c:998)
  ==148154==    by 0x49FBBDA: Rf_allocVector3 (/home/hornik/src/R/src/main/memory.c:2872)
  ==148154==    by 0x499F29B: Rf_allocVector (/home/hornik/src/R/src/include/Rinlinedfuns.h:609)
  ==148154==    by 0x499F29B: R_bcEncode (/home/hornik/src/R/src/main/eval.c:8745)
  ==148154==    by 0x4A60C17: ReadBC1 (/home/hornik/src/R/src/main/serialize.c:2213)
  ==148154==    by 0x4A60D6E: ReadBCConsts (/home/hornik/src/R/src/main/serialize.c:2183)
  ==148154==    by 0x4A60D6E: ReadBC1 (/home/hornik/src/R/src/main/serialize.c:2214)
  ==148154==    by 0x4A5FBF8: ReadBC (/home/hornik/src/R/src/main/serialize.c:2225)
  ==148154==    by 0x4A5FBF8: ReadItem_Recursive (/home/hornik/src/R/src/main/serialize.c:2056)
  ==148154==    by 0x4A5F1B0: ReadItem_Iterative (/home/hornik/src/R/src/main/serialize.c:1863)
  ==148154==    by 0x4A5F1B0: ReadItem_Recursive (/home/hornik/src/R/src/main/serialize.c:1963)
  ==148154==    by 0x4A610B7: ReadItem (/home/hornik/src/R/src/main/serialize.c:2120)
  ==148154==    by 0x4A610B7: R_Unserialize (/home/hornik/src/R/src/main/serialize.c:2277)
  ==148154==    by 0x4A61913: R_unserialize (/home/hornik/src/R/src/main/serialize.c:3006)
  ==148154==    by 0x4A629B4: do_lazyLoadDBfetch (/home/hornik/src/R/src/main/serialize.c:3318)
  ==148154==    by 0x49B44C9: Rf_eval (/home/hornik/src/R/src/main/eval.c:1264)
  ==148154==
  ==148154==
  ==148154== HEAP SUMMARY:
  ==148154==     in use at exit: 112,965,909 bytes in 21,417 blocks
  ==148154==   total heap usage: 453,765 allocs, 432,348 frees, 517,192,781 bytes allocated
  ==148154==
  ==148154== LEAK SUMMARY:
  ==148154==    definitely lost: 0 bytes in 0 blocks
  ==148154==    indirectly lost: 0 bytes in 0 blocks
  ==148154==      possibly lost: 15,139 bytes in 129 blocks
  ==148154==    still reachable: 112,950,770 bytes in 21,288 blocks
  ==148154==                       of which reachable via heuristic:
  ==148154==                         stdstring          : 1,041 bytes in 10 blocks
  ==148154==         suppressed: 0 bytes in 0 blocks
  ==148154== Rerun with --leak-check=full to see details of leaked memory
  ==148154==
  ==148154== For lists of detected and suppressed errors, rerun with: -s
  ==148154== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Which points to this line:

t[hist_i] = as<double>(result);

I'm personally surprised that this achieves an access violation, as opposed to throwing an out of bounds exception as I didn't think RCpp vectors were C style arrays.

@petedodd
petedodd merged commit 43529f1 into main Apr 13, 2026
9 checks passed
@petedodd
petedodd deleted the Robadob-patch-history-frequency branch April 13, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants