Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
[lgtm] Ensure that the type casting is done before multiplication to prevent overflow.
authorArnaud Giersch <arnaud.giersch@univ-fcomte.fr>
Fri, 16 Jul 2021 13:38:38 +0000 (15:38 +0200)
committerArnaud Giersch <arnaud.giersch@univ-fcomte.fr>
Fri, 16 Jul 2021 13:59:57 +0000 (15:59 +0200)
src/kernel/routing/DragonflyZone.cpp
src/kernel/routing/FatTreeZone.cpp
src/smpi/bindings/smpi_pmpi_type.cpp
src/smpi/internals/smpi_replay.cpp
src/smpi/mpi/smpi_datatype.cpp
src/smpi/mpi/smpi_datatype_derived.cpp

index 4096ba8..e75ea57 100644 (file)
@@ -166,7 +166,7 @@ void DragonflyZone::generate_routers(const s4u::ClusterCallbacks& set_callbacks)
     return limiter;
   };
 
-  routers_.reserve(num_groups_ * num_chassis_per_group_ * num_blades_per_chassis_);
+  routers_.reserve(static_cast<size_t>(num_groups_) * num_chassis_per_group_ * num_blades_per_chassis_);
   for (unsigned int i = 0; i < num_groups_; i++) {
     for (unsigned int j = 0; j < num_chassis_per_group_; j++) {
       for (unsigned int k = 0; k < num_blades_per_chassis_; k++) {
@@ -206,7 +206,7 @@ void DragonflyZone::generate_links()
   // Links from routers to their local nodes.
   for (unsigned int i = 0; i < numRouters; i++) {
     // allocate structures
-    routers_[i].my_nodes_.resize(num_links_per_link_ * num_nodes_per_blade_);
+    routers_[i].my_nodes_.resize(static_cast<size_t>(num_links_per_link_) * num_nodes_per_blade_);
     routers_[i].green_links_.resize(num_blades_per_chassis_);
     routers_[i].black_links_.resize(num_chassis_per_group_);
 
@@ -312,7 +312,8 @@ void DragonflyZone::get_local_route(const NetPoint* src, const NetPoint* dst, Ro
   }
 
   // node->router local link
-  add_link_latency(route->link_list_, myRouter->my_nodes_[myCoords.node * num_links_per_link_], latency);
+  add_link_latency(route->link_list_, myRouter->my_nodes_[static_cast<size_t>(myCoords.node) * num_links_per_link_],
+                   latency);
 
   if (targetRouter != myRouter) {
     // are we on a different group ?
index be69aeb..27665e6 100644 (file)
@@ -265,9 +265,10 @@ void FatTreeZone::generate_switches(const s4u::ClusterCallbacks& set_callbacks)
       k--;
       auto newNode = std::make_shared<FatTreeNode>(k, i + 1, j, get_limiter(i, j, k), nullptr);
       XBT_DEBUG("We create the switch %d(%u,%u)", newNode->id, newNode->level, newNode->position);
-      newNode->children.resize(this->num_children_per_node_[i] * this->num_port_lower_level_[i]);
+      newNode->children.resize(static_cast<size_t>(this->num_children_per_node_[i]) * this->num_port_lower_level_[i]);
       if (i != this->levels_ - 1) {
-        newNode->parents.resize(this->num_parents_per_node_[i + 1] * this->num_port_lower_level_[i + 1]);
+        newNode->parents.resize(static_cast<size_t>(this->num_parents_per_node_[i + 1]) *
+                                this->num_port_lower_level_[i + 1]);
       }
       newNode->label.resize(this->levels_);
       this->nodes_.emplace_back(newNode);
@@ -337,7 +338,7 @@ void FatTreeZone::add_processing_node(int id, resource::LinkImpl* limiter, resou
   static int position = 0;
   auto newNode = std::make_shared<FatTreeNode>(id, 0, position, limiter, loopback);
   position++;
-  newNode->parents.resize(this->num_parents_per_node_[0] * this->num_port_lower_level_[0]);
+  newNode->parents.resize(static_cast<size_t>(this->num_parents_per_node_[0]) * this->num_port_lower_level_[0]);
   newNode->label.resize(this->levels_);
   this->compute_nodes_.insert(make_pair(id, newNode));
   this->nodes_.emplace_back(newNode);
index 9bcc26d..651eb67 100644 (file)
@@ -156,7 +156,7 @@ int PMPI_Type_create_indexed_block(int count, int blocklength, const int* indice
   CHECK_COUNT(1, count)
   CHECK_MPI_NULL(4, MPI_DATATYPE_NULL, MPI_ERR_TYPE, old_type)
   CHECK_NULL(5, MPI_ERR_ARG, new_type)
-  auto* blocklens = static_cast<int*>(xbt_malloc(blocklength * count * sizeof(int)));
+  auto* blocklens = static_cast<int*>(xbt_malloc(sizeof(int) * blocklength * count));
   for (int i    = 0; i < count; i++)
     blocklens[i]=blocklength;
   int retval    = simgrid::smpi::Datatype::create_indexed(count, blocklens, indices, old_type, new_type);
@@ -183,7 +183,7 @@ int PMPI_Type_create_hindexed_block(int count, int blocklength, const MPI_Aint*
   CHECK_COUNT(1, count)
   CHECK_MPI_NULL(4, MPI_DATATYPE_NULL, MPI_ERR_TYPE, old_type)
   CHECK_NULL(5, MPI_ERR_ARG, new_type)
-  auto* blocklens = static_cast<int*>(xbt_malloc(blocklength * count * sizeof(int)));
+  auto* blocklens = static_cast<int*>(xbt_malloc(sizeof(int) * blocklength * count));
   for (int i     = 0; i < count; i++)
     blocklens[i] = blocklength;
   int retval     = simgrid::smpi::Datatype::create_hindexed(count, blocklens, indices, old_type, new_type);
index b2c2d76..a6b455a 100644 (file)
@@ -637,8 +637,8 @@ void AllToAllAction::kernel(simgrid::xbt::ReplayAction&)
                                                     Datatype::encode(args.datatype1),
                                                     Datatype::encode(args.datatype2)));
 
-  colls::alltoall(send_buffer(args.send_size * args.comm_size * args.datatype1->size()), args.send_size, args.datatype1,
-                  recv_buffer(args.recv_size * args.comm_size * args.datatype2->size()), args.recv_size, args.datatype2,
+  colls::alltoall(send_buffer(args.datatype1->size() * args.send_size * args.comm_size), args.send_size, args.datatype1,
+                  recv_buffer(args.datatype2->size() * args.recv_size * args.comm_size), args.recv_size, args.datatype2,
                   MPI_COMM_WORLD);
 
   TRACE_smpi_comm_out(get_pid());
@@ -655,7 +655,7 @@ void GatherAction::kernel(simgrid::xbt::ReplayAction&)
   if (get_name() == "gather") {
     int rank = MPI_COMM_WORLD->rank();
     colls::gather(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1,
-                  (rank == args.root) ? recv_buffer(args.recv_size * args.comm_size * args.datatype2->size()) : nullptr,
+                  (rank == args.root) ? recv_buffer(args.datatype2->size() * args.recv_size * args.comm_size) : nullptr,
                   args.recv_size, args.datatype2, args.root, MPI_COMM_WORLD);
   } else
     colls::allgather(send_buffer(args.send_size * args.datatype1->size()), args.send_size, args.datatype1,
index ecdef35..a30b964 100644 (file)
@@ -423,13 +423,14 @@ int Datatype::create_vector(int count, int block_length, int stride, MPI_Datatyp
     ub=((count-1)*stride+block_length-1)*old_type->get_extent()+old_type->ub();
   }
   if(old_type->flags() & DT_FLAG_DERIVED || stride != block_length){
-    *new_type = new Type_Vector(count * block_length * old_type->size(), lb, ub, DT_FLAG_DERIVED, count, block_length,
+    *new_type = new Type_Vector(old_type->size() * block_length * count, lb, ub, DT_FLAG_DERIVED, count, block_length,
                                 stride, old_type);
     retval=MPI_SUCCESS;
   }else{
     /* in this situation the data are contiguous thus it's not required to serialize and unserialize it*/
-    *new_type = new Datatype(count * block_length * old_type->size(), 0, ((count -1) * stride + block_length)*
-                         old_type->size(), DT_FLAG_CONTIGUOUS|DT_FLAG_DERIVED);
+    *new_type =
+        new Datatype(old_type->size() * block_length * count, 0,
+                     old_type->size() * ((count - 1) * stride + block_length), DT_FLAG_CONTIGUOUS | DT_FLAG_DERIVED);
     const std::array<int, 3> ints = {{count, block_length, stride}};
     (*new_type)->set_contents(MPI_COMBINER_VECTOR, 3, ints.data(), 0, nullptr, 1, &old_type);
     retval=MPI_SUCCESS;
@@ -450,12 +451,13 @@ int Datatype::create_hvector(int count, int block_length, MPI_Aint stride, MPI_D
     ub=((count-1)*stride)+(block_length-1)*old_type->get_extent()+old_type->ub();
   }
   if(old_type->flags() & DT_FLAG_DERIVED || stride != block_length*old_type->get_extent()){
-    *new_type = new Type_Hvector(count * block_length * old_type->size(), lb, ub, DT_FLAG_DERIVED, count, block_length,
+    *new_type = new Type_Hvector(old_type->size() * block_length * count, lb, ub, DT_FLAG_DERIVED, count, block_length,
                                  stride, old_type);
     retval=MPI_SUCCESS;
   }else{
     /* in this situation the data are contiguous thus it's not required to serialize and unserialize it*/
-    *new_type = new Datatype(count * block_length * old_type->size(), 0, count * block_length * old_type->size(), DT_FLAG_CONTIGUOUS|DT_FLAG_DERIVED);
+    *new_type = new Datatype(old_type->size() * block_length * count, 0, old_type->size() * block_length * count,
+                             DT_FLAG_CONTIGUOUS | DT_FLAG_DERIVED);
     const std::array<int, 2> ints = {{count, block_length}};
     (*new_type)->set_contents(MPI_COMBINER_HVECTOR, 2, ints.data(), 1, &stride, 1, &old_type);
     retval=MPI_SUCCESS;
@@ -517,7 +519,7 @@ int Datatype::create_hindexed(int count, const int* block_lengths, const MPI_Ain
     if(indices[i]+block_lengths[i]*old_type->ub()>ub)
       ub = indices[i]+block_lengths[i]*old_type->ub();
 
-    if ( (i< count -1) && (indices[i]+block_lengths[i]*(static_cast<int>(old_type->size())) != indices[i+1]) )
+    if ((i < count - 1) && (indices[i] + static_cast<MPI_Aint>(old_type->size()) * block_lengths[i] != indices[i + 1]))
       contiguous=false;
   }
   if (old_type->flags_ & DT_FLAG_DERIVED || lb!=0)
@@ -565,7 +567,8 @@ int Datatype::create_struct(int count, const int* block_lengths, const MPI_Aint*
     if (not forced_ub && indices[i] + block_lengths[i] * old_types[i]->ub() > ub)
       ub = indices[i]+block_lengths[i]*old_types[i]->ub();
 
-    if ( (i< count -1) && (indices[i]+block_lengths[i]*static_cast<int>(old_types[i]->size()) != indices[i+1]) )
+    if ((i < count - 1) &&
+        (indices[i] + static_cast<MPI_Aint>(old_types[i]->size() * block_lengths[i]) != indices[i + 1]))
       contiguous=false;
   }
   if (not contiguous) {
index 60ccd26..17c2f4c 100644 (file)
@@ -54,7 +54,7 @@ void Type_Contiguous::serialize(const void* noncontiguous_buf, void* contiguous_
 {
   auto* contiguous_buf_char          = static_cast<char*>(contiguous_buf);
   const auto* noncontiguous_buf_char = static_cast<const char*>(noncontiguous_buf) + lb();
-  memcpy(contiguous_buf_char, noncontiguous_buf_char, count * block_count_ * old_type_->size());
+  memcpy(contiguous_buf_char, noncontiguous_buf_char, old_type_->size() * count * block_count_);
 }
 
 void Type_Contiguous::unserialize(const void* contiguous_buf, void* noncontiguous_buf, int count, MPI_Op op)