Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 47b-udt-values-length #643 +/- ##
=========================================================
+ Coverage 98.64% 98.65% +0.01%
=========================================================
Files 105 105
Lines 29443 29510 +67
Branches 3449 3452 +3
=========================================================
+ Hits 29043 29113 +70
Misses 261 261
+ Partials 139 136 -3 🚀 New features to boost your workflow:
|
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 19, 2026 07:55
d4703df to
f11c703
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 20, 2026 22:28
f11c703 to
2e69171
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 21, 2026 04:14
2e69171 to
1e9240e
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
2 times, most recently
from
October 28, 2026 04:29
0e0e155 to
f5d2961
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 28, 2026 13:21
f5d2961 to
842d327
Compare
SuiteSparse compares the byte size of an imported values buffer with the
number of entries, not with the bytes they need (GB_import.c sets
Ax_memsize_for_non_iso to nvals or vlen * vdim), so any buffer of at least
nvals bytes got through and was read past its end:
Vector.ss.import_full(values=np.ones(3), size=5)
built a Vector whose last two values were heap garbage, and so did values
too short for a sub-array dtype, as in
Vector.ss.import_sparse(size=3, indices=[0, 1], values=np.ones(2), dtype="FP64[2]")
Every ss import and pack (sparse, bitmap and full Vectors; csr, csc,
hypercsr, hypercsc, bitmapr, bitmapc, fullr and fullc Matrices, and so
from_dcsr and from_dcsc) now raises
ValueError: values must have 5 or more elements of dtype FP64, not 3
before handing GraphBLAS the buffer. An iso import needs one element.
…lues SuiteSparse:GraphBLAS 10.5.2 fixed the import size check that check_values_size works around: GB_import.c compared the byte size of the values with the number of entries. With check_values_size bypassed, the five short imports tried (import_full, import_sparse of FP64[2], import_bitmapr, and two import_csr) now raise InvalidValue with an empty message on 10.5.2, where 10.5.1 built them from bytes past the end. check_values_size still runs on every version: older ones need it, and its ValueError says what is wrong. Its docstring and the comments of the two test_ss_import_values_size tests now give the versions. Comments only.
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 29, 2026 02:01
842d327 to
cdfaec6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SuiteSparse checks the size of an imported values buffer in the wrong units.
GB_importsets
Ax_memsize_for_non_isoto nvals (sparse, hypersparse) orvlen * vdim(bitmap,full), a count of entries. It then compares that count with
Ax_memsize, which is in bytes,as documented and as python-graphblas passes it (
values.nbytes). So any buffer of at leastnvals bytes is accepted, and later reads run past its end:
returned a Vector whose last two values were heap garbage. The same happened with values too
short for a sub-array dtype:
The previous PR in the stack, which checks for whole elements in build, left these
imports for a follow-up.
Every ss import and pack now checks the values before handing them to GraphBLAS: sparse,
bitmap and full Vectors, and csr, csc, hypercsr, hypercsc, bitmapr, bitmapc, fullr and
fullc Matrices, which also covers
Matrix.from_dcsrandfrom_dcscon SuiteSparse. Theyraise
The check is one helper,
check_values_size, next to 47b'scheck_element_shape. It countswhat GraphBLAS will read: nvals,
indptr[n], the size, or nrows x ncols. An iso import needsone element, so iso imports with a single value still work. A larger buffer is still
accepted.
test_ss_import_exportimported a 14 x 7 bitmap with 7 x 7 values, so GraphBLAS read 49values past their end. It now passes values for the whole bitmap and checks the second
half. The new tests (
test_ss_import_values_sizein test_vector.py and test_matrix.py,SuiteSparse only) cover FP64 and
FP64[2], sparse, bitmap and full formats, an import and apack, and iso imports given one element. Both fail on the parent commit.
The SuiteSparse side is written up as a C MRE with a small fix that multiplies by
type->size(not filed yet). It reproduces on 10.5.0, 10.5.1 and the currentstable; thefaulty lines date back to at least 7.4.0.