From 28939b220193360a911c2c638e8804e9af1b0c51 Mon Sep 17 00:00:00 2001 From: Arnaud Giersch Date: Thu, 12 Oct 2017 23:25:14 +0200 Subject: [PATCH] In MSG_parallel_task_create(), take a copy of flops_amount and bytes_amount. Fix undefined behavior when they were not dynamically allocated, and permit to replace more malloc/free. --- src/msg/msg_private.hpp | 7 ++++++- src/msg/msg_task.cpp | 11 ++++++++--- src/simdag/sd_task.cpp | 19 ++++++++----------- src/simix/smx_host.cpp | 6 +++--- src/surf/HostImpl.cpp | 7 +++++-- src/surf/ptask_L07.cpp | 16 ++++++++-------- 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/msg/msg_private.hpp b/src/msg/msg_private.hpp index c1affe16fe..094dd62f4e 100644 --- a/src/msg/msg_private.hpp +++ b/src/msg/msg_private.hpp @@ -28,7 +28,12 @@ public: /********************************* Task **************************************/ struct s_simdata_task_t { - ~s_simdata_task_t() { delete[] this->host_list; /* parallel tasks only */ } + ~s_simdata_task_t() + { + /* parallel tasks only */ + delete[] this->host_list; + /* flops_parallel_amount and bytes_parallel_amount are automatically deleted in ~L07Action */ + } void setUsed(); void setNotUsed() { this->isused = false; } diff --git a/src/msg/msg_task.cpp b/src/msg/msg_task.cpp index f07beb0c02..0f6c705f65 100644 --- a/src/msg/msg_task.cpp +++ b/src/msg/msg_task.cpp @@ -92,10 +92,15 @@ msg_task_t MSG_parallel_task_create(const char *name, int host_nb, const msg_hos /* Simulator Data specific to parallel tasks */ simdata->host_nb = host_nb; simdata->host_list = new sg_host_t[host_nb]; - simdata->flops_parallel_amount = flops_amount; - simdata->bytes_parallel_amount = bytes_amount; - std::copy_n(host_list, host_nb, simdata->host_list); + if (flops_amount != nullptr) { + simdata->flops_parallel_amount = new double[host_nb]; + std::copy_n(flops_amount, host_nb, simdata->flops_parallel_amount); + } + if (bytes_amount != nullptr) { + simdata->bytes_parallel_amount = new double[host_nb * host_nb]; + std::copy_n(bytes_amount, host_nb * host_nb, simdata->bytes_parallel_amount); + } return task; } diff --git a/src/simdag/sd_task.cpp b/src/simdag/sd_task.cpp index 978bb6b1ba..b75326d994 100644 --- a/src/simdag/sd_task.cpp +++ b/src/simdag/sd_task.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2006-2016. The SimGrid Team. +/* Copyright (c) 2006-2017. The SimGrid Team. * All rights reserved. */ /* This program is free software; you can redistribute it and/or modify it @@ -7,6 +7,7 @@ #include "simdag_private.hpp" #include "src/surf/HostImpl.hpp" #include "src/surf/surf_interface.hpp" +#include XBT_LOG_NEW_DEFAULT_SUBCATEGORY(sd_task, sd, "Logging specific to SimDag (task)"); @@ -798,20 +799,16 @@ void SD_task_run(SD_task_t task) /* Copy the elements of the task into the action */ int host_nb = task->allocation->size(); - sg_host_t *hosts = xbt_new(sg_host_t, host_nb); - int i =0; - for (auto const& host : *task->allocation) { - hosts[i] = host; - i++; - } + sg_host_t* hosts = new sg_host_t[host_nb]; + std::copy_n(task->allocation->begin(), host_nb, hosts); - double *flops_amount = xbt_new0(double, host_nb); - double *bytes_amount = xbt_new0(double, host_nb * host_nb); + double* flops_amount = new double[host_nb](); + double* bytes_amount = new double[host_nb * host_nb](); if(task->flops_amount) - memcpy(flops_amount, task->flops_amount, sizeof(double) * host_nb); + std::copy_n(task->flops_amount, host_nb, flops_amount); if(task->bytes_amount) - memcpy(bytes_amount, task->bytes_amount, sizeof(double) * host_nb * host_nb); + std::copy_n(task->bytes_amount, host_nb * host_nb, bytes_amount); task->surf_action = surf_host_model->executeParallelTask(host_nb, hosts, flops_amount, bytes_amount, task->rate); diff --git a/src/simix/smx_host.cpp b/src/simix/smx_host.cpp index 855b4912d8..b33bc4fa0e 100644 --- a/src/simix/smx_host.cpp +++ b/src/simix/smx_host.cpp @@ -10,6 +10,7 @@ #include "src/plugins/vm/VirtualMachineImpl.hpp" #include "src/surf/surf_interface.hpp" #include "xbt/ex.hpp" +#include XBT_LOG_NEW_DEFAULT_SUBCATEGORY(simix_host, simix, "SIMIX hosts"); @@ -192,9 +193,8 @@ SIMIX_execution_parallel_start(const char* name, int host_nb, sg_host_t* host_li simgrid::kernel::activity::ExecImplPtr(new simgrid::kernel::activity::ExecImpl(name, nullptr)); /* set surf's synchro */ - sg_host_t *host_list_cpy = xbt_new0(sg_host_t, host_nb); - for (int i = 0; i < host_nb; i++) - host_list_cpy[i] = host_list[i]; + sg_host_t* host_list_cpy = new sg_host_t[host_nb]; + std::copy_n(host_list, host_nb, host_list_cpy); /* Check that we are not mixing VMs and PMs in the parallel task */ bool is_a_vm = (nullptr != dynamic_cast(host_list[0])); diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp index f3530f5a69..870c6fc3f9 100644 --- a/src/surf/HostImpl.cpp +++ b/src/surf/HostImpl.cpp @@ -80,13 +80,16 @@ Action* HostModel::executeParallelTask(int host_nb, simgrid::s4u::Host** host_li xbt_die("Cannot have a communication that is not a simple point-to-point in this model. You should consider " "using the ptask model"); } - } else + } else { xbt_die( "This model only accepts one of the following. You should consider using the ptask model for the other cases.\n" " - execution with one host only and no communication\n" " - Self-comms with one host only\n" " - Communications with two hosts and no computation"); - xbt_free(host_list); + } + delete[] host_list; + delete[] flops_amount; + delete[] bytes_amount; return action; } diff --git a/src/surf/ptask_L07.cpp b/src/surf/ptask_L07.cpp index b3d5beab5b..1c56280809 100644 --- a/src/surf/ptask_L07.cpp +++ b/src/surf/ptask_L07.cpp @@ -226,14 +226,14 @@ L07Action::L07Action(Model *model, int host_nb, sg_host_t *host_list, this->setCost(1.0); this->setRemains(0.0); } - xbt_free(host_list); + delete[] host_list; } Action* NetworkL07Model::communicate(s4u::Host* src, s4u::Host* dst, double size, double rate) { - sg_host_t*host_list = xbt_new0(sg_host_t, 2); - double *flops_amount = xbt_new0(double, 2); - double *bytes_amount = xbt_new0(double, 4); + sg_host_t* host_list = new sg_host_t[2](); + double* flops_amount = new double[2](); + double* bytes_amount = new double[4](); host_list[0] = src; host_list[1] = dst; @@ -280,8 +280,8 @@ LinkL07::LinkL07(NetworkL07Model* model, const std::string& name, double bandwid Action *CpuL07::execution_start(double size) { - sg_host_t*host_list = xbt_new0(sg_host_t, 1); - double *flops_amount = xbt_new0(double, 1); + sg_host_t* host_list = new sg_host_t[1](); + double* flops_amount = new double[1](); host_list[0] = getHost(); flops_amount[0] = size; @@ -392,8 +392,8 @@ LinkL07::~LinkL07() = default; L07Action::~L07Action(){ delete hostList_; - free(communicationAmount_); - free(computationAmount_); + delete[] communicationAmount_; + delete[] computationAmount_; } void L07Action::updateBound() -- 2.20.1