Skip to content

Commit

Permalink
fix CRAN ASAN-UBSAN warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
pachadotdev committed Nov 12, 2024
1 parent b9f7326 commit d895519
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 7 deletions.
6 changes: 6 additions & 0 deletions rpkg/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# redatam 2.0.3

* Fixes memory management issues suggested by Ivan Krylov regarding the C++ to R
list casting.
* Uses 2 threads during R CMD check

# redatam 2.0.1

* Fixes memory leaks warned by CRAN on clang-ASAN and gcc-UBSAN.
Expand Down
2 changes: 1 addition & 1 deletion rpkg/configure
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ LDFLAGS="-stdlib=libc++"

# False positive error in ASAN
# see https://github.com/microsoft/DirectXShaderCompiler/issues/created_by/5971)
export ASAN_OPTIONS=detect_container_overflow=0
# export ASAN_OPTIONS=alloc_dealloc_mismatch=0

# Write to Makevars using sed to replace placeholders
sed -e "s|@cflags@|$PKG_CFLAGS|" \
Expand Down
25 changes: 23 additions & 2 deletions rpkg/cran-comments.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
## R CMD check results

0 errors | 0 warnings | 1 note
0 errors | 0 warnings | 2 note

* This was tested with all available R-Hub images.
❯ checking CRAN incoming feasibility ... [5s/15s] NOTE
Maintainer: ‘Mauricio Vargas Sepulveda <m.sepulveda@mail.utoronto.ca>’

New submission

Package was archived on CRAN

CRAN repository db overrides:
X-CRAN-Comment: Archived on 2024-11-06 for repeated policy violation.

Repeatedy spamming a team member's personal email address in HTML.

❯ checking compilation flags used ... NOTE
Compilation used the following non-portable flag(s):
‘-Wp,-D_FORTIFY_SOURCE=3’

Fixes:

* This was tested with all available R-Hub images including the Clang ASAN image.
* Fixes UBSan issues (https://win-builder.r-project.org/incoming_pretest/redatam_2.0.3_20241107_154750/specialChecks/clang-san/summary.txt)
* Fixes the empty string issue (https://www.stats.ox.ac.uk/pub/bdr/memtests/clang-ASAN/redatam/00check.log)
* Uses 2 threads during R CMD check
5 changes: 3 additions & 2 deletions rpkg/src/redatamlib/exporters/RListExporter.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#include "RListExporter.hpp"

#include <algorithm> // For std::replace
#include <sstream>

#include "Entity.hpp"
#include "ParentIDCalculator.hpp"
#include "RListExporter.hpp"

namespace RedatamLib {
using std::endl;
Expand All @@ -12,7 +13,7 @@ using std::ostringstream;
// just to mimic the original CSVExporter
ListExporter::ListExporter(const std::string &outputDirectory)
: m_path(outputDirectory) {
if ('/' != m_path.back()) {
if (!m_path.empty() && '/' != m_path.back()) {
m_path.append("/");
}
}
Expand Down
17 changes: 15 additions & 2 deletions rpkg/src/redatamlib/readers/FuzzyVariableParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,17 @@ FuzzyVariableParser::FuzzyVariableParser(ByteArrayReader reader,
void FuzzyVariableParser::ParseAllVariables(vector<Entity> &entities) {
vector<pair<size_t, size_t>> searchBounds = GetSearchBounds(entities);

size_t numThreads = std::thread::hardware_concurrency();
numThreads = std::min(entities.size(), numThreads);
// R-devel suggestion: Default to using all available hardware concurrency
size_t maxThreads = std::thread::hardware_concurrency();

// Then check for _R_CHECK_LIMIT_CORES_ environment variable
const char *checkLimitCoresEnv = std::getenv("_R_CHECK_LIMIT_CORES_");
if (checkLimitCoresEnv != nullptr &&
std::string(checkLimitCoresEnv) == "TRUE") {
maxThreads = 2;
}

size_t numThreads = std::min(entities.size(), maxThreads);

size_t chunkSize = entities.size() / numThreads;

Expand All @@ -42,6 +51,10 @@ vector<pair<size_t, size_t>>
FuzzyVariableParser::GetSearchBounds(vector<Entity> entities) {
vector<pair<size_t, size_t>> ret;

if (entities.empty()) {
return ret;
}

for (size_t i = 0; i < entities.size() - 1; ++i) {
ret.push_back(
{entities[i].GetBounds().second, entities[i + 1].GetBounds().first});
Expand Down

0 comments on commit d895519

Please sign in to comment.