Bug potential of memory_sub_partition::top function

[hnts03]

// l2cache.cc

mem_fetch *memory_sub_partition::pop() {
  mem_fetch *mf = m_L2_icnt_queue->pop();
  m_request_tracker.erase(mf);
  if (mf && mf->isatomic()) mf->do_atomic();
  if (mf && (mf->get_access_type() == L2_WRBK_ACC ||
             mf->get_access_type() == L1_WRBK_ACC)) {
    delete mf;
    mf = NULL;
  }
  return mf;
}

mem_fetch *memory_sub_partition::top() {
  mem_fetch *mf = m_L2_icnt_queue->top();
  if (mf && (mf->get_access_type() == L2_WRBK_ACC ||
             mf->get_access_type() == L1_WRBK_ACC)) {
    m_L2_icnt_queue->pop();
    m_request_tracker.erase(mf);
    delete mf;
    mf = NULL;
  }
  return mf;
}

// gpu-sim.cc
void gpgpu_sim::cycle(){
...

 if (clock_mask & ICNT) {
   // pop from memory controller to interconnect
   for (unsigned i = 0; i < m_memory_config->m_n_mem_sub_partition; i++) {
     mem_fetch *mf = m_memory_sub_partition[i]->top(); // << top(0)
     if (mf) {  // << Condition(1)
       unsigned response_size =
           mf->get_is_write() ? mf->get_ctrl_size() : mf->size();
       if (::icnt_has_buffer(m_shader_config->mem2device(i), response_size)) {
         // if (!mf->get_is_write())
         mf->set_return_timestamp(gpu_sim_cycle + gpu_tot_sim_cycle);
         mf->set_status(IN_ICNT_TO_SHADER, gpu_sim_cycle + gpu_tot_sim_cycle);
         ::icnt_push(m_shader_config->mem2device(i), mf->get_tpc(), mf,
                     response_size);
         m_memory_sub_partition[i]->pop(); // pop(2)
         partiton_replys_in_parallel_per_cycle++;
       } else {
         gpu_stall_icnt2sh++;
       }
     } else {
       m_memory_sub_partition[i]->pop(); // pop(3)
     }
   }
 }
...
}

When the gpgpu_sim::cycle() function started to ICNT cycle, the return of m_memory_sub_partition[I]->top() could be NULL when its access_type is L2_WRBK_ACC or L1_WRBK_ACC. However, it may cause a double delete & pop to a single mf object and m_L2_icnt_queue. The top function must not delete the element inside of the queue structure.

[tgrogers]

@William-An , @abhaumick - if you have some time, can you look into this this week?

[William-An]

Looks fair to me. Also, the delete mf at the top should also be removed to prevent a double delete.

[Copilot]

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)

• src/gpgpu-sim/l2cache.cc: Language not supported