Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug potential of memory_sub_partition::top function #75

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

hnts03
Copy link

@hnts03 hnts03 commented Aug 28, 2024

// 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.

@JRPan JRPan requested review from a team and Connie120 and removed request for a team August 28, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant