Merge pull request #3460 from heplesser/fix_sanitizer_errors

Fix corner cases uncovered by Clang sanitizer

Author: heplesser

libnestutil/config.h.in

@@ -96,10 +96,10 @@
 /* Define to 1 if you have usable long long type. */
 #cmakedefine HAVE_LONG_LONG 1
 
-/* Define to 1 if you have u_int46_t type. */
+/* Define to 1 if you have u_int64_t type. */
 #cmakedefine HAVE_U_INT64_T 1
 
-/* Define to 1 if you have uint46_t type. */
+/* Define to 1 if you have uint64_t type. */
 #cmakedefine HAVE_UINT64_T 1
 
 /* Define to 1 if you have the <mach/mach.h> header file. */

nestkernel/connector_base_impl.h

@@ -50,7 +50,8 @@ Connector< ConnectionT >::send_weight_event( const size_t tid,
     wr_e.set_port( e.get_port() );
     wr_e.set_rport( e.get_rport() );
     wr_e.set_stamp( e.get_stamp() );
-    wr_e.set_sender( e.get_sender() );
+    // Sender is not available for SecondaryEvents, and not needed, so we do not
+    // set it to avoid undefined behavior.
     wr_e.set_sender_node_id( kernel().connection_manager.get_source_node_id( tid, syn_id_, lcid ) );
     wr_e.set_weight( e.get_weight() );
     wr_e.set_delay_steps( e.get_delay_steps() );

nestkernel/event.h

@@ -930,12 +930,14 @@ Event::set_sender_node_id_info( const size_t tid, const synindex syn_id, const s
 inline Node&
 Event::get_receiver() const
 {
+  assert( receiver_ );
   return *receiver_;
 }
 
 inline Node&
 Event::get_sender() const
 {
+  assert( sender_ );
   return *sender_;
 }
 

nestkernel/exceptions.h

@@ -219,15 +219,15 @@ class UnknownSynapseType : public KernelException
 
 class UnknownNode : public KernelException
 {
-  int id_;
+  long id_;
 
 public:
   UnknownNode()
     : KernelException( "UnknownNode" )
     , id_( -1 )
   {
   }
-  UnknownNode( int id )
+  UnknownNode( long id )
     : KernelException( "UnknownNode" )
     , id_( id )
   {

nestkernel/modelrange_manager.cpp

@@ -82,15 +82,15 @@ ModelRangeManager::get_model_id( size_t node_id ) const
 {
   if ( not is_in_range( node_id ) )
   {
-    throw UnknownNode( node_id );
+    throw UnknownNode( static_cast< long >( node_id ) );
   }
 
-  int left = -1;
-  int right = modelranges_.size();
+  long left = -1;
+  long right = static_cast< long >( modelranges_.size() );
   assert( right >= 1 );
 
   // to ensure thread-safety, use local range_idx
-  size_t range_idx = right / 2; // start in center
+  long range_idx = right / 2; // start in center
   while ( not modelranges_[ range_idx ].is_in_range( node_id ) )
   {
     if ( node_id > modelranges_[ range_idx ].get_last_node_id() )

nestkernel/mpi_manager.cpp

@@ -326,7 +326,12 @@ nest::MPIManager::communicate( std::vector< long >& local_nodes, std::vector< lo
   num_nodes_per_rank[ get_rank() ] = local_nodes.size();
   communicate( num_nodes_per_rank );
 
-  size_t num_globals = std::accumulate( num_nodes_per_rank.begin(), num_nodes_per_rank.end(), 0 );
+  const size_t num_globals = std::accumulate( num_nodes_per_rank.begin(), num_nodes_per_rank.end(), 0 );
+  if ( num_globals == 0 )
+  {
+    return; // must return here to avoid passing address to empty global_nodes below
+  }
+
   global_nodes.resize( num_globals, 0L );
 
   // Set up displacements vector. Entry i specifies the displacement (relative
@@ -337,7 +342,9 @@ nest::MPIManager::communicate( std::vector< long >& local_nodes, std::vector< lo
     displacements.at( i ) = displacements.at( i - 1 ) + num_nodes_per_rank.at( i - 1 );
   }
 
-  MPI_Allgatherv( &( *local_nodes.begin() ),
+  // avoid dereferencing empty vector
+  const auto send_ptr = local_nodes.empty() ? nullptr : &local_nodes[ 0 ];
+  MPI_Allgatherv( send_ptr,
     local_nodes.size(),
     MPI_Type< long >::type,
     &global_nodes[ 0 ],

nestkernel/nest_time.cpp

@@ -66,8 +66,7 @@ Time::compute_max()
   const tic_t tmax = std::numeric_limits< tic_t >::max();
 
   tic_t tics;
-  if ( lmax
-    < static_cast< tic_t >( static_cast< double >( tmax ) * Range::TICS_PER_STEP_INV ) ) // step size is limiting factor
+  if ( lmax < tmax * Range::TICS_PER_STEP_INV ) // step size is limiting factor
   {
     tics = Range::TICS_PER_STEP * ( lmax / Range::INF_MARGIN );
   }

sli/interpret.cc

@@ -895,15 +895,16 @@ SLIInterpreter::message( std::ostream& out,
         // space available (as long as the word is shorter than the
         // total width of the printout).
         if ( i != 0 and text_str.at( i - 1 ) == ' '
-          and static_cast< int >( space - i ) > static_cast< int >( width - pos ) )
+          and static_cast< int >( space ) - static_cast< int >( i )
+            > static_cast< int >( width ) - static_cast< int >( pos ) )
         {
           out << std::endl << std::string( indent, ' ' );
           pos = 0;
         }
 
         // Only print character if we're not at the end of the
         // line and the last character is a space.
-        if ( not( width - pos == 0 and text_str.at( i ) == ' ' ) )
+        if ( not( static_cast< int >( width ) - static_cast< int >( pos ) == 0 and text_str.at( i ) == ' ' ) )
         {
           // Print the actual character.
           out << text_str.at( i );

sli/scanner.cc

@@ -558,7 +558,7 @@ Scanner::operator()( Token& t )
     // so we cannot use unsigned char c as argument.  The
     // get() is not picky.  --- HEP 2001-08-09
     //     in->get(c);
-    c = in->get();
+    c = static_cast< unsigned char >( in->get() );
     if ( col++ == 0 )
     {
       ++line;

sli/slistack.cc

@@ -261,7 +261,7 @@ RollFunction::execute( SLIInterpreter* i ) const
     throw ArgumentType( 0 );
   }
 
-  long& n = idn->get();
+  const long n = idn->get();
   if ( n < 0 )
   {
     i->raiseerror( i->RangeCheckError );
@@ -273,10 +273,12 @@ RollFunction::execute( SLIInterpreter* i ) const
     return;
   }
 
+  const long k = idk->get();
+
   i->EStack.pop();
   i->OStack.pop( 2 );
 
-  i->OStack.roll( n, idk->get() );
+  i->OStack.roll( n, k );
 }
 
 /** @BeginDocumentation

sli/tokenstack.h

@@ -158,7 +158,7 @@ class TokenStack : private TokenArrayObj
   }
 
   void
-  roll( size_t n, long k )
+  roll( long n, long k )
   {
     if ( n < 2 or k == 0 )
     {
@@ -180,6 +180,7 @@ class TokenStack : private TokenArrayObj
   {
     return TokenArrayObj::capacity();
   }
+
   Index
   load() const
   {

testsuite/pytests/test_getnodes.py

@@ -66,6 +66,16 @@ def test_GetNodes_with_params(self):
 
         self.assertEqual(nodes_exp_ref, nodes_exp)
 
+    def test_GetNodes_no_match(self):
+        """
+        Ensure we get an empty result if nothing matches.
+
+        This would lead to crashes in MPI-parallel code before #3460.
+        """
+
+        nodes = nest.GetNodes({"V_m": 100.0})
+        self.assertEqual(len(nodes), 0)
+
 
 def suite():
     suite = unittest.makeSuite(GetNodesTestCase, "test")