422 clear setelement and usage of pinnedvector leads to segementation fault

include/graphblas/bsp1d/exec.hpp

@@ -37,9 +37,10 @@
 #include "init.hpp"
 
 #ifndef _GRB_NO_STDIO
-#include <iostream> //for std::cerr
+ #include <iostream> //for std::cerr
 #endif
 
+
 /** Global internal singleton to track whether MPI was initialized. */
 extern bool _grb_mpi_initialized;
 
@@ -51,8 +52,8 @@ void _grb_exec_spmd( lpf_t ctx, lpf_pid_t s, lpf_pid_t P, lpf_args_t args ) {
 
 #ifdef _DEBUG
 	if( s == 0 ) {
-		std::cout << "Info: launcher spawned or hooked " << P
-			<< " ALP/GraphBLAS user processes.\n";
+		std::cout << "Info: launcher spawned or hooked " << P << " ALP/GraphBLAS "
+			<< "user processes.\n";
 	}
 #endif
 

include/graphblas/bsp1d/io.hpp

@@ -189,6 +189,7 @@ namespace grb {
 	RC clear( Vector< DataType, BSP1D, Coords > &x ) noexcept {
 		const RC ret = clear( internal::getLocal( x ) );
 		if( ret == SUCCESS ) {
+			x._became_dense = false;
 			x._cleared = true;
 			internal::signalLocalChange( x );
 		}
@@ -431,7 +432,62 @@ namespace grb {
 		return ret;
 	}
 
-	/** \internal Requires no inter-process communication. */
+	namespace internal {
+
+		/** This is the variant that can handle use_index. */
+		template<
+			Descriptor descr,
+			typename DataType, typename Coords, typename T
+		>
+		RC set_handle_use_index(
+			Vector< DataType, BSP1D, Coords > &x,
+			const size_t old_nnz, const T &val,
+			const typename std::enable_if<
+				std::is_convertible< size_t, DataType >::value,
+			void >::type * const = nullptr
+		) {
+			if( descr & descriptors::use_index ) {
+				const internal::BSP1D_Data &data = internal::grb_BSP1D.cload();
+				const auto p = data.P;
+				const auto s = data.s;
+				const auto n = grb::size( x );
+				if( old_nnz < size( x ) ) {
+					internal::getCoordinates( internal::getLocal( x ) ).assignAll();
+				}
+				return eWiseLambda( [ &x, &n, &s, &p ]( const size_t i ) {
+					x[ i ] = internal::Distribution< BSP1D >::local_index_to_global(
+							i, n, s, p
+						);
+					}, x );
+			} else {
+				return set< descr >( internal::getLocal( x ), val );
+			}
+		}
+
+		/** This is the variant that cannot handle use_index. */
+		template<
+			Descriptor descr,
+			typename DataType, typename Coords, typename T
+		>
+		RC set_handle_use_index(
+			Vector< DataType, BSP1D, Coords > &x,
+			const size_t, const T &val,
+			const typename std::enable_if<
+				!std::is_convertible< size_t, DataType >::value,
+			void >::type * const = nullptr
+		) {
+			static_assert( !(descr & descriptors::use_index ),
+				"use_index requires casting from size_t to the vector value type" );
+			return set< descr >( internal::getLocal( x ), val );
+		}
+
+	} // end namespace internal
+
+	/**
+	 * \internal
+	 * Requires no inter-process communication.
+	 * \endinternal
+	 */
 	template<
 		Descriptor descr = descriptors::no_operation,
 		typename DataType, typename Coords,
@@ -442,8 +498,9 @@ namespace grb {
 		const T val,
 		const Phase &phase = EXECUTE,
 		const typename std::enable_if<
-			!grb::is_object< T >::value, void
-		>::type * const = nullptr
+			!grb::is_object< T >::value &&
+			std::is_convertible< T, DataType >::value,
+		void >::type * const = nullptr
 	) noexcept {
 		const size_t n = size( x );
 		const size_t old_nnz = nnz( x );
@@ -466,23 +523,7 @@ namespace grb {
 		}
 
 		assert( phase == EXECUTE );
-		RC ret = SUCCESS;
-		if( descr & descriptors::use_index ) {
-			const internal::BSP1D_Data &data = internal::grb_BSP1D.cload();
-			const auto p = data.P;
-			const auto s = data.s;
-			const auto n = grb::size( x );
-			if( old_nnz < size( x ) ) {
-				internal::getCoordinates( internal::getLocal( x ) ).assignAll();
-			}
-			ret = eWiseLambda( [ &x, &n, &s, &p ]( const size_t i ) {
-				x[ i ] = internal::Distribution< BSP1D >::local_index_to_global(
-						i, n, s, p
-					);
-				}, x );
-		} else {
-			ret = set< descr >( internal::getLocal( x ), val );
-		}
+		RC ret = internal::set_handle_use_index< descr >( x, old_nnz, val );
 		if( ret == SUCCESS ) {
 			internal::setDense( x );
 		}

include/graphblas/bsp1d/pinnedvector.hpp

@@ -55,10 +55,10 @@ namespace grb {
 			utils::AutoDeleter< IOType > _raw_deleter;
 
 			/**
-			 * Tell the system to delete \a _buffered_coordinates only when we had its
-			 * last reference.
+			 * Tell the system to delete the stack of \a _buffered_coordinates only when
+			 * we had its last reference.
 			 */
-			utils::AutoDeleter< char > _assigned_deleter;
+			utils::AutoDeleter< char > _stack_deleter;
 
 			/** A buffer of the local vector. */
 			IOType * _buffered_values;
@@ -138,7 +138,7 @@ namespace grb {
 			/** \internal No implementation notes. */
 			template< typename Coords >
 			PinnedVector( const Vector< IOType, BSP1D, Coords > &x, const IOMode mode ) :
-				_raw_deleter( x._raw_deleter ), _assigned_deleter( x._assigned_deleter ),
+				_raw_deleter( x._raw_deleter ), _stack_deleter( x._buffer_deleter ),
 				_buffered_values( mode == PARALLEL ? x._raw + x._offset : x._raw ),
 				_mode( mode ), _length( x._global._coordinates.size() )
 			{
@@ -147,17 +147,28 @@ namespace grb {
 				_P = data.P;
 				if( mode != PARALLEL ) {
 					assert( mode == SEQUENTIAL );
-					x.synchronize();
+					const RC rc = x.synchronize();
+					if( rc != SUCCESS ) {
+						throw std::runtime_error(
+							"Could not synchronise vector during pinning: " + toString( rc )
+						);
+					}
 					_buffered_coordinates = x._global._coordinates;
 				} else {
+					const RC rc = x.updateNnz();
+					if( rc != SUCCESS ) {
+						throw std::runtime_error(
+							"Could not update vector nonzero count during pinning: " + toString( rc )
+						);
+					}
 					_buffered_coordinates = x._local._coordinates;
 				}
 			}
 
 			/** \internal No implementation notes. */
 			PinnedVector( const PinnedVector< IOType, BSP1D > &other ) :
 				_raw_deleter( other._raw_deleter ),
-				_assigned_deleter( other._assigned_deleter ),
+				_stack_deleter( other._stack_deleter ),
 				_buffered_values( other._buffered_values ),
 				_buffered_coordinates( other._buffered_coordinates ),
 				_mode( other._mode ), _length( other._length ),
@@ -167,7 +178,7 @@ namespace grb {
 			/** \internal No implementation notes. */
 			PinnedVector( PinnedVector< IOType, BSP1D > &&other ) :
 				_raw_deleter( other._raw_deleter ),
-				_assigned_deleter( other._assigned_deleter ),
+				_stack_deleter( other._stack_deleter ),
 				_buffered_values( other._buffered_values ),
 				//_buffered_coordinates uses std::move, below
 				_mode( other._mode ), _length( other._length ),
@@ -181,7 +192,7 @@ namespace grb {
 				const PinnedVector< IOType, BSP1D > &other
 			) {
 				_raw_deleter = other._raw_deleter;
-				_assigned_deleter = other._assigned_deleter;
+				_stack_deleter = other._stack_deleter;
 				_buffered_values = other._buffered_values;
 				_buffered_coordinates = other._buffered_coordinates;
 				_mode = other._mode;
@@ -196,7 +207,7 @@ namespace grb {
 				PinnedVector< IOType, BSP1D > &&other
 			) {
 				_raw_deleter = other._raw_deleter;
-				_assigned_deleter = other._assigned_deleter;
+				_stack_deleter = other._stack_deleter;
 				_buffered_values = other._buffered_values;
 				_buffered_coordinates = std::move( other._buffered_coordinates );
 				_mode = other._mode;

include/graphblas/reference/coordinates.hpp

@@ -445,7 +445,10 @@ namespace grb {
 				 *                 valid state.
 				 */
 				void rebuild( const bool dense ) noexcept {
-					// catch most trivial case: a vector of dimension 0, and the dense case
+#ifdef _DEBUG
+					std::cout << "Coordinates::rebuild called with dense = " << dense << "\n";
+#endif
+					// catch most trivial case: a vector of dimension 0 (empty vector)
 					if( _cap == 0 ) {
 						return;
 					}
@@ -456,7 +459,8 @@ namespace grb {
 					}
 					assert( _assigned != nullptr );
 #endif
-					if( dense ) {
+					// catch the other trivial-ish case (since can delegate)
+					if( dense && _n != _cap ) {
 #ifdef _DEBUG
 						std::cout << "rebuildSparsity: dense case\n";
 #endif
@@ -1369,13 +1373,15 @@ namespace grb {
 						assert( t_start < t_end );
 						assert( local_cur < pfBuf[ t_start + 1 ] );
 						for( size_t t_cur = t_start; t_cur < t_end; ++t_cur ) {
+							// The below is a _DEBUG statement that is extremely noisy, yet sometimes
+							// useful. Hence kept disabled by default.
 /*#ifdef _DEBUG
 							#pragma omp critical
 							{
 								std::cout << "\t Thread " << t << " processing nonzero " << global_count
 									<< " / " << global_length << " using the local stack of thread "
 									<< t_cur << " starting from local index " << local_cur << ".\n";
-#endif*///DBG
+#endif*/
 							assert( local_cur <= pfBuf[ t_cur + 1 ] - pfBuf[ t_cur ] );
 							StackType * __restrict__ const cur_stack =
 								_buffer + t_cur * stack_offset + 1;

include/graphblas/reference/pinnedvector.hpp

@@ -50,10 +50,10 @@ namespace grb {
 			utils::AutoDeleter< IOType > _raw_deleter;
 
 			/**
-			 * Tell the system to delete \a _buffered_mask only when we had its last
-			 * reference.
+			 * Tell the system to delete the stack of the \a _buffered_coordinates only
+			 * when we had its last reference.
 			 */
-			utils::AutoDeleter< char > _assigned_deleter;
+			utils::AutoDeleter< char > _stack_deleter;
 
 			/** A buffer of the local vector. */
 			IOType * _buffered_values;
@@ -76,7 +76,7 @@ namespace grb {
 				> > &x,
 				const IOMode mode
 			) :
-				_raw_deleter( x._raw_deleter ), _assigned_deleter( x._assigned_deleter ),
+				_raw_deleter( x._raw_deleter ), _stack_deleter( x._buffer_deleter ),
 				_buffered_values( x._raw ), _buffered_coordinates( x._coordinates )
 			{
 				(void)mode; // sequential and parallel IO mode are equivalent for this
@@ -131,6 +131,7 @@ namespace grb {
 				assert( _buffered_coordinates.size() > 0 );
 				assert( _buffered_values != nullptr );
 				const size_t index = getNonzeroIndex( k );
+				assert( index < _buffered_coordinates.size() );
 				return _buffered_values[ index ];
 			}
 

tests/unit/CMakeLists.txt

@@ -249,6 +249,10 @@ add_grb_executables( buildMatrixUnique buildMatrixUnique.cpp
 	ADDITIONAL_LINK_LIBRARIES test_utils
 )
 
+add_grb_executables( pinnedVector pinnedVector.cpp
+	BACKENDS reference reference_omp bsp1d hybrid
+)
+
 # targets to list and build the test for this category
 get_property( unit_tests_list GLOBAL PROPERTY tests_category_unit )
 add_custom_target( "list_tests_category_unit"