Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Parse errors now raise a simgrid::ParseError that you may want to catch
authorMartin Quinson <martin.quinson@ens-rennes.fr>
Mon, 18 Nov 2019 05:05:08 +0000 (06:05 +0100)
committerMartin Quinson <martin.quinson@ens-rennes.fr>
Mon, 18 Nov 2019 05:05:08 +0000 (06:05 +0100)
ChangeLog
include/simgrid/Exception.hpp
src/kernel/EngineImpl.cpp
src/kernel/routing/FatTreeZone.cpp
src/s4u/s4u_Engine.cpp
src/surf/xml/platf.hpp
src/surf/xml/surfxml_sax_cb.cpp
src/xbt/exception.cpp
teshsuite/simdag/flatifier/bogus_disk_attachment.tesh
teshsuite/simdag/flatifier/bogus_missing_gateway.tesh

index 4d2b347..5ead4c9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -6,6 +6,9 @@ S4U:
 - Actor: Merge signals on_migration_start/end into on_host_change
 - Actor: Rename migrate() into set_host()
 
+XML:
+- Parse errors now raise a simgrid::ParseError that you may want to catch.
+
 Kernel:
 - In simgrid::kernel::resource::Model, the methods next_occuring_event*() have
   been renamed to fix a spelling error. As usual, the previous definitions are
index 5ea1158..7f3918f 100644 (file)
@@ -13,6 +13,7 @@
 
 #include <xbt/backtrace.hpp>
 #include <xbt/ex.h>
+#include <xbt/string.hpp>
 
 #include <atomic>
 #include <functional>
@@ -157,6 +158,23 @@ public:
   }
 };
 
+class XBT_PUBLIC ParseError : public Exception, public std::invalid_argument {
+  int line_;
+  std::string file_;
+  std::string msg_;
+
+public:
+  ParseError(int line, std::string& file, std::string&& msg)
+      : Exception(XBT_THROW_POINT, std::move(msg)), std::invalid_argument(msg), line_(line), file_(file), msg_(msg)
+  {
+  }
+
+  const char* what() const noexcept override
+  {
+    return bprintf("Parse error at %s:%d: %s", file_.c_str(), line_, msg_.c_str());
+  }
+};
+
 class XBT_PUBLIC ForcefulKillException {
   /** @brief Exception launched to kill an actor; DO NOT BLOCK IT!
    *
index fcd5140..ab8ce33 100644 (file)
@@ -47,16 +47,9 @@ void EngineImpl::load_deployment(const std::string& file)
   sg_platf_init();
 
   surf_parse_open(file);
-  try {
-    int parse_status = surf_parse();
-    surf_parse_close();
-    xbt_assert(not parse_status, "Parse error at %s:%d", file.c_str(), surf_parse_lineno);
-  } catch (const Exception&) {
-    xbt_die(
-        "Unrecoverable error at %s:%d. The full exception stack follows, in case it helps you to diagnose the problem.",
-        file.c_str(), surf_parse_lineno);
-    throw;
-  }
+  int parse_status = surf_parse();
+  surf_parse_close();
+  surf_parse_assert(not parse_status, std::string("Parse error in file ") + file);
 }
 void EngineImpl::register_function(const std::string& name, xbt_main_func_t code)
 {
index c277462..0c30abf 100644 (file)
@@ -363,51 +363,51 @@ void FatTreeZone::parse_specific_arguments(ClusterCreationArgs* cluster)
   std::vector<std::string> parameters;
   std::vector<std::string> tmp;
   boost::split(parameters, cluster->topo_parameters, boost::is_any_of(";"));
-  const std::string error_msg {"Fat trees are defined by the levels number and 3 vectors, see the documentation for more information"};
 
   // TODO : we have to check for zeros and negative numbers, or it might crash
-  if (parameters.size() != 4) {
-    surf_parse_error(error_msg);
-  }
+  surf_parse_assert(
+      parameters.size() == 4,
+      "Fat trees are defined by the levels number and 3 vectors, see the documentation for more information.");
 
   // The first parts of topo_parameters should be the levels number
   try {
     this->levels_ = std::stoi(parameters[0]);
   } catch (const std::invalid_argument&) {
-    throw std::invalid_argument(std::string("First parameter is not the amount of levels:") + parameters[0]);
+    surf_parse_error(std::string("First parameter is not the amount of levels: ") + parameters[0]);
   }
 
   // Then, a l-sized vector standing for the children number by level
   boost::split(tmp, parameters[1], boost::is_any_of(","));
-  if (tmp.size() != this->levels_) {
-    surf_parse_error(error_msg);
-  }
+  surf_parse_assert(tmp.size() == this->levels_, std::string("You specified ") + std::to_string(this->levels_) +
+                                                     " levels but the child count vector (the first one) contains " +
+                                                     std::to_string(tmp.size()) + " levels.");
+
   for (size_t i = 0; i < tmp.size(); i++) {
     try {
       this->num_children_per_node_.push_back(std::stoi(tmp[i]));
     } catch (const std::invalid_argument&) {
-      throw std::invalid_argument(std::string("Invalid lower level node number:") + tmp[i]);
+      surf_parse_error(std::string("Invalid child count: ") + tmp[i]);
     }
   }
 
   // Then, a l-sized vector standing for the parents number by level
   boost::split(tmp, parameters[2], boost::is_any_of(","));
-  if (tmp.size() != this->levels_) {
-    surf_parse_error(error_msg);
-  }
+  surf_parse_assert(tmp.size() == this->levels_, std::string("You specified ") + std::to_string(this->levels_) +
+                                                     " levels but the parent count vector (the second one) contains " +
+                                                     std::to_string(tmp.size()) + " levels.");
   for (size_t i = 0; i < tmp.size(); i++) {
     try {
       this->num_parents_per_node_.push_back(std::stoi(tmp[i]));
     } catch (const std::invalid_argument&) {
-      throw std::invalid_argument(std::string("Invalid upper level node number:") + tmp[i]);
+      surf_parse_error(std::string("Invalid parent count: ") + tmp[i]);
     }
   }
 
   // Finally, a l-sized vector standing for the ports number with the lower level
   boost::split(tmp, parameters[3], boost::is_any_of(","));
-  if (tmp.size() != this->levels_) {
-    surf_parse_error(error_msg);
-  }
+  surf_parse_assert(tmp.size() == this->levels_, std::string("You specified ") + std::to_string(this->levels_) +
+                                                     " levels but the port count vector (the third one) contains " +
+                                                     std::to_string(tmp.size()) + " levels.");
   for (size_t i = 0; i < tmp.size(); i++) {
     try {
       this->num_port_lower_level_.push_back(std::stoi(tmp[i]));
index c4b8905..583175b 100644 (file)
@@ -84,11 +84,7 @@ double Engine::get_clock()
 void Engine::load_platform(const std::string& platf)
 {
   double start = xbt_os_time();
-  try {
-    parse_platform_file(platf);
-  } catch (const Exception& e) {
-    xbt_die("Error while loading %s: %s", platf.c_str(), e.what());
-  }
+  parse_platform_file(platf);
 
   double end = xbt_os_time();
   XBT_DEBUG("PARSE TIME: %g", (end - start));
index 35d4bca..2e83473 100644 (file)
@@ -14,8 +14,8 @@ XBT_PUBLIC void sg_platf_exit();
 
 XBT_PUBLIC void surf_parse_open(const std::string& file);
 XBT_PUBLIC void surf_parse_close();
-XBT_PUBLIC void surf_parse_assert(bool cond, const std::string& msg);
-XBT_ATTRIB_NORETURN XBT_PUBLIC void surf_parse_error(const std::string& msg);
+XBT_PUBLIC void surf_parse_assert(bool cond, std::string&& msg);
+XBT_ATTRIB_NORETURN XBT_PUBLIC void surf_parse_error(std::string&& msg);
 XBT_PUBLIC void surf_parse_assert_netpoint(const std::string& hostname, const std::string& pre,
                                            const std::string& post);
 
index a9f2be5..9e4a03f 100644 (file)
@@ -3,6 +3,7 @@
 /* This program is free software; you can redistribute it and/or modify it
  * under the terms of the license (GNU LGPL) which comes with this package. */
 
+#include "simgrid/Exception.hpp"
 #include "simgrid/kernel/routing/NetPoint.hpp"
 #include "simgrid/s4u/Engine.hpp"
 #include "simgrid/sg_config.hpp"
@@ -33,19 +34,15 @@ std::vector<simgrid::kernel::resource::DiskImpl*> parsed_disk_list; /* temporary
 /*
  * Helping functions
  */
-void surf_parse_assert(bool cond, const std::string& msg)
+void surf_parse_assert(bool cond, std::string&& msg)
 {
   if (not cond)
-    surf_parse_error(msg);
+    surf_parse_error(std::move(msg));
 }
 
-void surf_parse_error(const std::string& msg)
+void surf_parse_error(std::string&& msg)
 {
-  int lineno = surf_parse_lineno;
-  cleanup();
-  XBT_ERROR("Parse error at %s:%d: %s", surf_parsed_filename.c_str(), lineno, msg.c_str());
-  surf_exit();
-  xbt_die("Exiting now");
+  throw simgrid::ParseError(surf_parse_lineno, surf_parsed_filename, std::move(msg));
 }
 
 void surf_parse_assert_netpoint(const std::string& hostname, const std::string& pre, const std::string& post)
@@ -76,7 +73,7 @@ void surf_parse_assert_netpoint(const std::string& hostname, const std::string&
       break;
     }
   }
-  surf_parse_error(msg);
+  surf_parse_error(std::move(msg));
 }
 
 double surf_parse_get_double(const std::string& s)
index c89f325..7e4366a 100644 (file)
@@ -84,8 +84,7 @@ static std::terminate_handler previous_terminate_handler = nullptr;
 
 static void handler()
 {
-  // Avoid doing crazy things if we get an uncaught exception inside
-  // an uncaught exception
+  // Avoid doing crazy things if we get an uncaught exception inside an uncaught exception
   static std::atomic_flag lock = ATOMIC_FLAG_INIT;
   if (lock.test_and_set()) {
     XBT_ERROR("Handling an exception raised an exception. Bailing out.");
@@ -99,6 +98,13 @@ static void handler()
     std::rethrow_exception(e);
   }
 
+  // Parse error are handled differently, as the call stack does not matter, only the file location
+  catch (simgrid::ParseError& e) {
+    XBT_ERROR("%s", e.what());
+    XBT_ERROR("Exiting now.");
+    std::abort();
+  }
+
   // We manage C++ exception ourselves
   catch (std::exception& e) {
     log_exception(xbt_log_priority_critical, "Uncaught exception", e);
@@ -108,8 +114,7 @@ static void handler()
 
   catch (const simgrid::ForcefulKillException&) {
     XBT_ERROR("Received a ForcefulKillException at the top-level exception handler. Maybe a Java->C++ call that is not "
-              "protected "
-              "in a try/catch?");
+              "protected in a try/catch?");
     show_backtrace(bt);
   }
 
@@ -124,6 +129,7 @@ static void handler()
       std::abort();
     }
   }
+  XBT_INFO("BAM");
 }
 
 void install_exception_handler()
index ae49cdc..c3ad26c 100644 (file)
@@ -2,4 +2,4 @@
 $ $VALGRIND_NO_LEAK_CHECK ${bindir:=.}/flatifier ../platforms/bogus_disk_attachment.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n"
 > [  0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks.
 > [  0.000000] [0:maestro@] Parse error at ../platforms/bogus_disk_attachment.xml:19: Unable to attach storage cdisk: host plouf does not exist.
-> [  0.000000] [0:maestro@] Exiting now
+> [  0.000000] [0:maestro@] Exiting now.
index 20f2519..f9e194d 100644 (file)
@@ -3,11 +3,11 @@ $ $VALGRIND_NO_LEAK_CHECK ${bindir:=.}/flatifier ../platforms/bogus_missing_src_
 > [  0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks.
 > [  0.000000] [0:maestro@] Parse error at ../platforms/bogus_missing_src_gateway.xml:14: zoneRoute gw_src='nod-cluster_router.cluster.us' does name a node. Existing netpoints: 
 > 'node-1.cluster.us','node-2.cluster.us','node-3.cluster.us','node-4.cluster.us','node-cluster_router.cluster.us','noeud-1.grappe.fr','noeud-2.grappe.fr','noeud-3.grappe.fr','noeud-4.grappe.fr','noeud-grappe_router.grappe.fr'
-> [  0.000000] [0:maestro@] Exiting now
+> [  0.000000] [0:maestro@] Exiting now.
 
 ! expect signal SIGABRT
 $ $VALGRIND_NO_LEAK_CHECK ${bindir:=.}/flatifier ../platforms/bogus_missing_dst_gateway.xml "--log=root.fmt:[%10.6r]%e[%i:%P@%h]%e%m%n"
 > [  0.000000] [0:maestro@] Switching to the L07 model to handle parallel tasks.
 > [  0.000000] [0:maestro@] Parse error at ../platforms/bogus_missing_dst_gateway.xml:14: zoneRoute gw_dst='neud-grappe_router.grappe.fr' does name a node. Existing netpoints: 
 > 'node-1.cluster.us','node-2.cluster.us','node-3.cluster.us','node-4.cluster.us','node-cluster_router.cluster.us','noeud-1.grappe.fr','noeud-2.grappe.fr','noeud-3.grappe.fr','noeud-4.grappe.fr','noeud-grappe_router.grappe.fr'
-> [  0.000000] [0:maestro@] Exiting now
+> [  0.000000] [0:maestro@] Exiting now.