Skip to content

Commit

Permalink
Fix additional integer overflow issues
Browse files Browse the repository at this point in the history
- correct the condition for using 32bit int indices
  - previous condition: `n_points < UINT_MAX`
  - however the data array in the C code has size `n_points * n_dim` and is indexed using a 32bit int type (see e.g. the PA macro)
  - this means results were incorrect for the case where `n_points < UINT_MAX` and `n_points * n_dim >= UINT_MAX`
  - new condition: `n_points * n_dim < UINT_MAX`
  - resolves storpipfugl#138
- render mako template
  - the generated _kdtree_core.c did not contain latest changes from _kdtree_core.c.mako (sorry my mistake)
  - this meant results were incorrect for the case `n_points < UINT_MAX` and `n_query * k >= UINT_MAX`
  - resolves storpipfugl#137
- use signed 64bit ints for the OpenMP loop on all platforms
  - to support OpenMP versions < 3 which only allow signed integers as loop counters
  • Loading branch information
lkeegan committed Jan 29, 2025
1 parent 5755b5e commit 587e2f3
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 35 deletions.
40 changes: 12 additions & 28 deletions pykdtree/_kdtree_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -836,14 +836,10 @@ void search_tree_float_int32_t(Tree_float_int32_t *tree, float *pa, float *point
int8_t no_dims = tree->no_dims;
float *bbox = tree->bbox;
uint32_t *pidx = tree->pidx;
uint32_t j = 0;
#if defined(_MSC_VER) && defined(_OPENMP)
int32_t i = 0;
int32_t local_num_points = (int32_t) num_points;
#else
uint32_t i;
uint32_t local_num_points = num_points;
#endif
/* use 64-bit ints for indexing to avoid overflow, use signed ints to support all Openmp implementations */
int64_t i = 0;
int64_t j = 0;
int64_t local_num_points = (int64_t) num_points;
Node_float_int32_t *root = (Node_float_int32_t *)tree->root;

/* Queries are OpenMP enabled */
Expand Down Expand Up @@ -1412,14 +1408,10 @@ void search_tree_float_int64_t(Tree_float_int64_t *tree, float *pa, float *point
int8_t no_dims = tree->no_dims;
float *bbox = tree->bbox;
uint64_t *pidx = tree->pidx;
uint64_t j = 0;
#if defined(_MSC_VER) && defined(_OPENMP)
/* use 64-bit ints for indexing to avoid overflow, use signed ints to support all Openmp implementations */
int64_t i = 0;
int64_t j = 0;
int64_t local_num_points = (int64_t) num_points;
#else
uint64_t i;
uint64_t local_num_points = num_points;
#endif
Node_float_int64_t *root = (Node_float_int64_t *)tree->root;

/* Queries are OpenMP enabled */
Expand Down Expand Up @@ -2057,14 +2049,10 @@ void search_tree_double_int32_t(Tree_double_int32_t *tree, double *pa, double *p
int8_t no_dims = tree->no_dims;
double *bbox = tree->bbox;
uint32_t *pidx = tree->pidx;
uint32_t j = 0;
#if defined(_MSC_VER) && defined(_OPENMP)
int32_t i = 0;
int32_t local_num_points = (int32_t) num_points;
#else
uint32_t i;
uint32_t local_num_points = num_points;
#endif
/* use 64-bit ints for indexing to avoid overflow, use signed ints to support all Openmp implementations */
int64_t i = 0;
int64_t j = 0;
int64_t local_num_points = (int64_t) num_points;
Node_double_int32_t *root = (Node_double_int32_t *)tree->root;

/* Queries are OpenMP enabled */
Expand Down Expand Up @@ -2633,14 +2621,10 @@ void search_tree_double_int64_t(Tree_double_int64_t *tree, double *pa, double *p
int8_t no_dims = tree->no_dims;
double *bbox = tree->bbox;
uint64_t *pidx = tree->pidx;
uint64_t j = 0;
#if defined(_MSC_VER) && defined(_OPENMP)
/* use 64-bit ints for indexing to avoid overflow, use signed ints to support all Openmp implementations */
int64_t i = 0;
int64_t j = 0;
int64_t local_num_points = (int64_t) num_points;
#else
uint64_t i;
uint64_t local_num_points = num_points;
#endif
Node_double_int64_t *root = (Node_double_int64_t *)tree->root;

/* Queries are OpenMP enabled */
Expand Down
7 changes: 1 addition & 6 deletions pykdtree/_kdtree_core.c.mako
Original file line number Diff line number Diff line change
Expand Up @@ -716,15 +716,10 @@ void search_tree_${DTYPE}_${ITYPE}(Tree_${DTYPE}_${ITYPE} *tree, ${DTYPE} *pa, $
int8_t no_dims = tree->no_dims;
${DTYPE} *bbox = tree->bbox;
u${ITYPE} *pidx = tree->pidx;
#if defined(_MSC_VER) && defined(_OPENMP)
/* use 64-bit ints for indexing to avoid overflow, use signed ints to support all Openmp implementations */
int64_t i = 0;
int64_t j = 0;
int64_t local_num_points = (int64_t) num_points;
#else
uint64_t i;
uint64_t j;
uint64_t local_num_points = (uint64_t) num_points;
#endif
Node_${DTYPE}_${ITYPE} *root = (Node_${DTYPE}_${ITYPE} *)tree->root;

/* Queries are OpenMP enabled */
Expand Down
2 changes: 1 addition & 1 deletion pykdtree/kdtree.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ cdef class KDTree:

# Get tree info
self.n = <uint64_t>data_pts.shape[0]
self._use_int32_t = self.n < UINT32_MAX
self._use_int32_t = self.n * <uint64_t>data_pts.shape[1] < UINT32_MAX
self.leafsize = <uint32_t>leafsize
if data_pts.ndim == 1:
self.ndim = 1
Expand Down

0 comments on commit 587e2f3

Please sign in to comment.