Logo AND Algorithmique Numérique Distribuée

Public GIT Repository
Merge remote-tracking branch 'adfaure/master'
authorGabriel Corona <gabriel.corona@loria.fr>
Wed, 20 Jul 2016 14:39:20 +0000 (16:39 +0200)
committerGabriel Corona <gabriel.corona@loria.fr>
Wed, 20 Jul 2016 14:39:20 +0000 (16:39 +0200)
28 files changed:
doc/doxygen/community_giveback.doc
doc/doxygen/module-xbt.doc
doc/doxygen/uhood.doc
include/simgrid/chrono.hpp [new file with mode: 0644]
include/simgrid/s4u/actor.hpp
include/simgrid/s4u/conditionVariable.hpp
include/xbt/dict.h
include/xbt/dynar.h
include/xbt/fifo.h
include/xbt/heap.h
include/xbt/swag.h
src/mc/AddressSpace.hpp
src/mc/Channel.hpp
src/mc/Checker.hpp
src/mc/ChunkedData.hpp
src/mc/DwarfExpression.hpp
src/mc/Frame.cpp
src/mc/LocationList.hpp
src/mc/ModelChecker.cpp
src/mc/ObjectInformation.cpp
src/mc/ObjectInformation.hpp
src/mc/RemotePtr.hpp
src/mc/mc_dwarf.cpp
src/mc/mc_record.h
src/mc/mc_replay.h
tools/cmake/DefinePackages.cmake
tools/internal/spell_comments.pl
tools/internal/travis-sonarqube.sh

index f1469be..a1e8a61 100644 (file)
@@ -112,69 +112,200 @@ are easy, though):
 @subsection contributing_todo_cxxification Migration to C++
 
 The code is being migrated to C++ but a large part is still C (or C++ with
- idioms). It would be valuable to replace C idioms with C++ ones:
+C idioms). It would be valuable to replace C idioms with C++ ones:
 
- - replace XBT structures with C++ containers;
+ - replace XBT structures and C dynamic arrays with C++ containers;
 
- - replace `char*` with `std::string`;
+ - replace `char*` strings with `std::string`;
 
- - use RAII (`std::unique_ptr`, etc.) instead of explicit `malloc/free` or
-   `new/delete`.
+ - use exception-safe RAII (`std::unique_ptr`, etc.) instead of explicit
+   `malloc/free` or `new/delete`;
 
-@subsection contributing_todo_exceptions Migration to C++
+ - use `std::function` (or template functionoid arguments) instead of function
+   pointers;
+
+@subsubsection contributing_todo_exceptions Exceptions
 
 SimGrid used to implement exceptions in C. This has been replaced with C++
 exceptions but some bits of the C exceptions are still remaining:
 
  - `xbt_ex` was the type of C exceptions. It is now a standard C++ exception.
-    We might want to remove this and use a more idiomatic C++ solution.
-    `std::system_error` might be used for some error categories.
+    We might want to remove this exception and use a more idiomatic C++
+    solution with dedicated exception classes for different errors.
+    `std::system_error` might be used as well by replacing some `xbt_errcat_t`
+    with custom subclasses of `std::error_category`.
+
+ - The C API currently throws exceptions. Throwing exceptions out of a C API is
+   not very friendly. C code does not expect them, cannot catch them and cannot
+   handle resource management properly in face of exceptions. We should clearly
+   separate the C++ API and the C API and catch all exceptions before they get
+   ouf of C APIs.
+
+@subsubsection contributing_todo_time Time and duration
 
- - The C API currently throws exceptions exceptions. Throwing exceptions out
-   of C API is not very friendly. C code does not expect them, cannot catch
-   them and cannot handle resource management properly with exceptions.
-   We should clearly separate the C++ API and the C API and catch all exceptions
-   before they get ouf of C APIs.
+Some support for C++11-style time/duration is implemented (see `chrono.hpp`)
+but only available in some (S4U) APIs. It would be nice to add support for
+them in the rest of the C++ code.
 
-@subsection contributing_todo_futures Additions to the futures
+@subsubsection contributing_todo_futures Futures
 
  - Some features are missing in the Maestro future implementation
-  (`simgrid::simix::Future`, `simgrid::simix::Promise`)
+  (`simgrid::kernel::Future`, `simgrid::kernel::Promise`)
   could be extended to support additional features:
   `when_any`, `shared_future`, etc.
 
  - The corresponding feature might then be implemented in the user process
-   futures.
+   futures (`simgrid::simix::Future`).
+
+ - Currently `.then()` is not available for user futures. We would need to add
+   a basic user event loop in order to queue the pending continuations.
 
  - We might need to provide the option to cancel a pending operation. This
    might be achieved by defining some `Action` or `Operation` class with an
    API compatible with `Future` (and convertiable to it) but with an
    additional `.cancel()` method.
 
- - Currently `.then()` is not available for user futures. We would need to add
-   a basic user event loop in order to queue the pending continuations.
+@subsection contributing_todo_smpi SMPI
+
+@subsubsection contributing_smpi_split_process Process-based privatization
+
+Currently, all the simulated processes live in the same process as the SimGrid
+simulator. The benefit is that we don't have to do context switches and IPC
+between the simulator and the processes.
+
+The fact that they share the same address space means that one memory corruption
+in one simulated process can propagate to the other ones and the SimGrid
+simulator itself.
+
+Moreover, the current design for SMPI applications is to compile the MPI code
+normally and execute it once per simulated process in the same system process:
+This means that all the existing simulated MPI processes share the same virtual
+address space and share by default sthe same global variables. This is not
+correct as each MPI process is expected to use its own address space and have
+its own global variables. In order to fix, this problem we have an optional
+SMPI privatization feature which creates a instanciation of the executable
+data segment per MPI process and map the correct one (using `mmap`) at each
+context switch.
+
+This approach has many problems:
+
+ 1. It is not completely safe. We only handle SMPI privatization for the global
+    variables in the execute data segment. Shared objects are ignored but some
+    may contain global variables which may need to be privatized:
+
+    - libsimgrid for example must not be privatized because it contains
+      shared state for the simulator;
+
+    - libc must not be privatized for the same reason (but some global variables
+      in the libc may not be privatized);
+
+    - if we use global variables of some shared object in the executable, this
+      global variable will be instanciated in the executable (because of copy
+      relocation) and will be privatized even if it shoud not.
+
+ 2. We cannot execute the MPI processes in parallel. Only one can execute at
+    the same time because only one privatization segment can be mapped at a
+    given time.
 
-@subsection contributing_todo_simcalls Simcalls cleanup
+In order to fix this, the standard solution is to move each MPI process in its
+system process and use IPC to communicate with the simulator. One concern would
+be the impact on performance and memory consumption:
 
- - Remove simcalls by using the generic ones. One issue with this is that we
-   didn't devise a good way to deal with generic simcalls in the model-checker
-   yet.
+ - It would introduce a lot of context switches and IPC communications between
+   the MPI processes and the SimGrid simulator. However, currently every context
+   switch needs a `mmap` for SMPI privatization which is costly as well
+   (TLB flush).
+
+ - Instanciating a lot of processes might consume more memory which might be a
+   problem if we want to simulate a lot of MPI processes. Compiling MPI programs
+   as static executables with a lightweight libc might help and we might want to
+   support that. The SMPI processes should probably not embed all the SimGrid
+   simulator and its dependencies, the C++ runtime, etc.
+
+We would need to modify the model-checker as well which currently can only
+manage on model-checked process. For the model-checker we can expect some
+benefits from this approach: if a process did not execute, we know its state
+did not change and we don't need to take its snapshot and compare its state.
+
+Other solutions for this might include:
+
+ - Mapping each MPI process in the process of the simulator but in a different
+   symbol namespace (see `dlmopen`). Each process would have its own separate
+   instanciation and would not share libraries.
+
+ - Instanciate each MPI process in a separate lightweight VM (for example based
+   on WebAssembly) in the simualtor process.
 
 @subsection contributing_todo_mc Model-checker
 
- - Find a good solution to handle generic simcalls in the model-checker.
+@subsubsection contributing_todo_mc_state_compare Overhaul the state comparison code
+
+The state comparison code is quite complicated. It has very long functions and
+is programmed mostly using C idioms and is difficult to understanda and debug.
+It is in need or an overhaul:
+
+  - cleanup, refactorisation, usage of C++ features.
+
+  - The state comparison code works by infering types of blocks allocated on the
+    heap by following pointers from known roots (global variables, local
+    variables). Usually the first type found for a given block is used even if
+    a better one could be found later. By using a first pass of type inference,
+    on each snapshot before comparing the states, we might use a better type
+    information on the different blocks.
+
+  - We might benefit from adding logic for handling some known types. For
+    example, both `std::string` and `std::vector` have a capacity which might
+    be larger than the current size of the container. We should might ignore
+    the corresponding elements when comparing the states and infering the types.
+
+  - Another difficulty in the state comparison code is the detection of
+    dangling pointers. We cannot easily know if a pointer is dangling and
+    dangling pointers might lead us to choose the wrong type when infering
+    heap blocks. We might mitigate this problem by delaying the reallocation of
+    a freed block until there is no blocks pointing to it anymore using some
+    sort of basic garbage-collector.
+
+@subsubsection contributing_todo_mc_separation Separate the model-checker code from libsimgrid
+
+@subsubsection contributing_todo_mc_mced_interface Interface with the model-checked processes
+
+The model-checker reads many informations about the model-checked process
+by `process_vm_readv()`-ing brutally the data structure of the model-checked
+process leading to some horrible code such as walking a swag from another
+process. It prevents us as well from replacing some XBT data structures with
+standard C++ ones. We need a sane way to expose the relevant informations to
+the model-checker.
+
+@subsubsection contributing_todo_mc_generic_simcalls Generic simcalls
+
+We have introduced some generic simcalls which can be used to execute a
+callback in SimGrid Maestro context. It makes it a lot easier to interface
+the simulated process with the maestro. However, the callbacks for the
+model-checker which cannot decide how it should handle them. We would need a
+solution for this if we want to be able to replace the simcalls the
+model-checker cares about by generic simcalls.
+
+@subsubsection contributing_todo_mc_api Definig an API for writing Model-Checking algorithms
+
+Currently, writing a new model-checking algorithms in SimGridMC is quite
+difficult: the logic of the model-checking algorithm is mixed with a lot of
+low-level concerns about the way the model-checker is implemented. This makes it
+difficult to write new algorithms and difficult to understand, debug and modify
+the existing ones. We need a clean API to express the model-checking algorithms
+in a form which is closer to the text-book/paper description. This API muste
+be exposed in a a language which is more adequate to this task.
+
+Tasks:
 
- - Define a clear interface to be used by model-checking algorithms. The
-   `Session` class in intended to expose this interface but it is not a thing
-   yet.
+  1. Design and implement a clean API for expression model-checking algorithms.
+     A `Session` class currently exists for this but is not feature complete
+     and should probably be rewritten. It should be easy to create bindings
+     for different languages on top of this API.
 
- - Rewrite the different algorithms as implementations of the `Checker` class
-   using the `Session` inteface.
+  2. Create a binding to some better suited, dynamic, scripting language
+     (eg. Lua).
 
- - Currently a lot of informations the model-checker reads many informations
-   about the model-checked process by `process_vm_readv()`-ing brutally the
-   data structure leading to some horrible code such as walking a swag from
-   another process. It would be nice to have a sane way for the model-checker
-   to expose the relevant information to the model-checker.
+  3. Rewrite the existing model-checking algorithms in this language using the
+     new API.
 
 */
\ No newline at end of file
index ef58360..1ecc29e 100644 (file)
   @{ */
      /** @defgroup XBT_dynar  Dynar: generic dynamic array */
      /** @defgroup XBT_dict   Dict: generic dictionnary */
-     /** @defgroup XBT_set    Set: generic set datatype */
      /** @defgroup XBT_fifo   Fifo: generic workqueue */
      /** @defgroup XBT_swag   Swag: O(1) set datatype */
      /** @defgroup XBT_heap Heap: generic heap data structure */
-     /** @defgroup XBT_dd Data description */
 /** @} */
 
 
index 6a75337..b1ef22d 100644 (file)
@@ -25,7 +25,7 @@ We provide an uniform interface to them:
   (and share the same reference count).
 
 The ability to manipulate thge objects thought pointers and have the ability
-to use explicite reference count management is useful for creating C wrappers
+to use explicit reference count management is useful for creating C wrappers
 to the S4U and should play nicely with other language bindings (such as
 SWIG-based ones).
 
diff --git a/include/simgrid/chrono.hpp b/include/simgrid/chrono.hpp
new file mode 100644 (file)
index 0000000..4395b41
--- /dev/null
@@ -0,0 +1,55 @@
+/* Copyright (c) 2016. The SimGrid Team. All rights reserved.          */
+
+/* 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. */
+
+#ifndef SIMGRID_CHRONO_HPP
+#define SIMGRID_CHRONO_HPP
+
+/** @file chrono.hpp Time support
+ *
+ *  Define clock, duration types, time point types compatible with the standard
+ *  C++ library API.
+ */
+
+#include <chrono>
+#include <ratio>
+
+#include <simgrid/simix.h>
+
+namespace simgrid {
+
+/** A C++ compatible TrivialClock working with simulated-time */
+struct SimulationClock {
+  using rep        = double;
+  using period     = std::ratio<1>;
+  using duration   = std::chrono::duration<rep, period>;
+  using time_point = std::chrono::time_point<SimulationClock, duration>;
+  static constexpr bool is_steady = true;
+  static time_point now()
+  {
+    return time_point(duration(SIMIX_get_clock()));
+  }
+};
+
+/** Default duration for simulated time */
+using SimulationClockDuration  = SimulationClock::duration;
+
+/** Default time point for simulated time */
+using SimulationClockTimePoint = SimulationClock::time_point;
+
+// Durations based on doubles:
+using nanoseconds = std::chrono::duration<double, std::nano>;
+using microseconds = std::chrono::duration<double, std::micro>;
+using milliseconds = std::chrono::duration<double, std::milli>;
+using seconds = std::chrono::duration<double>;
+using minutes = std::chrono::duration<double, std::ratio<60>>;
+using hours = std::chrono::duration<double, std::ratio<3600>>;
+
+/** A time point in the simulated time */
+template<class Duration>
+using SimulationTimePoint = std::chrono::time_point<SimulationClock, Duration>;
+
+}
+
+#endif
index 8e1b703..5039f84 100644 (file)
@@ -7,6 +7,7 @@
 #define SIMGRID_S4U_ACTOR_HPP
 
 #include <atomic>
+#include <chrono>
 #include <functional>
 #include <memory>
 #include <stdexcept>
@@ -20,6 +21,7 @@
 #include <xbt/base.h>
 #include <xbt/functional.hpp>
 
+#include <simgrid/chrono.hpp>
 #include <simgrid/simix.h>
 #include <simgrid/s4u/forward.hpp>
 
@@ -258,6 +260,13 @@ namespace this_actor {
   /** Block the actor sleeping for that amount of seconds (may throws hostFailure) */
   XBT_PUBLIC(void) sleep(double duration);
 
+  template<class Rep, class Period>
+  inline void sleep(std::chrono::duration<Rep, Period> duration)
+  {
+    auto seconds = std::chrono::duration_cast<SimulationClockDuration>(duration);
+    sleep(seconds.count());
+  }
+
   /** Block the actor, computing the given amount of flops */
   XBT_PUBLIC(e_smx_state_t) execute(double flop);
 
index cad7507..3b03bca 100644 (file)
@@ -6,6 +6,7 @@
 #ifndef SIMGRID_S4U_COND_VARIABLE_HPP
 #define SIMGRID_S4U_COND_VARIABLE_HPP
 
+#include <chrono>
 #include <condition_variable>
 #include <future>
 #include <mutex>
@@ -16,6 +17,7 @@
 #include <xbt/base.h>
 
 #include <simgrid/simix.h>
+#include <simgrid/chrono.hpp>
 #include <simgrid/s4u/mutex.hpp>
 
 namespace simgrid {
@@ -45,20 +47,20 @@ public:
 
   static Ptr createConditionVariable();
 
-  //  Wait functions:
+  //  Wait functions without time:
 
   void wait(std::unique_lock<Mutex>& lock);
-  std::cv_status wait_until(std::unique_lock<Mutex>& lock, double timeout_time);
-  std::cv_status wait_for(std::unique_lock<Mutex>& lock, double duration);
-
-  // Variants which takes a predicate:
-
   template<class P>
   void wait(std::unique_lock<Mutex>& lock, P pred)
   {
     while (!pred())
       wait(lock);
   }
+
+  // Wait function taking a plain double as time:
+
+  std::cv_status wait_until(std::unique_lock<Mutex>& lock, double timeout_time);
+  std::cv_status wait_for(std::unique_lock<Mutex>& lock, double duration);
   template<class P>
   bool wait_until(std::unique_lock<Mutex>& lock, double timeout_time, P pred)
   {
@@ -73,6 +75,39 @@ public:
     return this->wait_until(lock, SIMIX_get_clock() + duration, std::move(pred));
   }
 
+  // Wait function taking a C++ style time:
+
+  template<class Rep, class Period, class P>
+  bool wait_for(
+    std::unique_lock<Mutex>& lock, std::chrono::duration<Rep, Period> duration,
+    P pred)
+  {
+    auto seconds = std::chrono::duration_cast<SimulationClockDuration>(duration);
+    return this->wait_for(lock, seconds.count(), pred);
+  }
+  template<class Rep, class Period>
+  std::cv_status wait_for(
+    std::unique_lock<Mutex>& lock, std::chrono::duration<Rep, Period> duration)
+  {
+    auto seconds = std::chrono::duration_cast<SimulationClockDuration>(duration);
+    return this->wait_for(lock, seconds.count());
+  }
+  template<class Duration>
+  std::cv_status wait_until(std::unique_lock<Mutex>& lock,
+    const SimulationTimePoint<Duration>& timeout_time)
+  {
+    auto timeout_native = std::chrono::time_point_cast<SimulationClockDuration>(timeout_time);
+    return this->wait_until(lock, timeout_native.time_since_epoch().count());
+  }
+  template<class Duration, class P>
+  bool wait_until(std::unique_lock<Mutex>& lock,
+    const SimulationTimePoint<Duration>& timeout_time, P pred)
+  {
+    auto timeout_native = std::chrono::time_point_cast<SimulationClockDuration>(timeout_time);
+    return this->wait_until(lock, timeout_native.time_since_epoch().count(),
+      std::move(pred));
+  }
+
   // Notify functions
 
   void notify_one();
index b3cfd35..59eb624 100644 (file)
@@ -21,7 +21,7 @@ SG_BEGIN_DECL()
  *  This section describes the API to a dictionary structure that  associates as string to a void* key. It provides the
  *  same functionality than an hash table.
  *
- *  If you are using C++, you might want to use `std::unordered_map` instead.
+ *  @deprecated If you are using C++, you might want to use `std::unordered_map` instead.
  *
  *  Here is a little example of use:
 
index 874eb46..fa25a74 100644 (file)
@@ -25,7 +25,7 @@ SG_BEGIN_DECL()
   * \ref XBT_dict section). You thus have to provide the function which will be used to free the content at
   * structure creation (of type void_f_ppvoid_t or void_f_pvoid_t).
   *
-  *  If you are using C++, you might want to use `std::vector` instead.
+  * @deprecated If you are using C++, you might want to use `std::vector` instead.
   *
   * \section XBT_dynar_exscal Example with scalar
   * \dontinclude dynar.cpp
index 5f6432b..66ccf3d 100644 (file)
@@ -17,8 +17,8 @@ SG_BEGIN_DECL()
  * These functions provide the same kind of functionality as dynamic arrays
  * but in time O(1). However these functions use malloc/free way too much often.
  *
- *  If you are using C++, you might want to used std::list, std::deque or
- *  std::queue instead.
+ *  @deprecated If you are using C++, you might want to used `std::list`,
+ *  `std::deque` or `std::queue instead`.
  */
  
 /** @defgroup XBT_fifo_cons Fifo constructor and destructor
index 3fe2951..89ee7d1 100644 (file)
@@ -15,7 +15,8 @@ SG_BEGIN_DECL()
 /** @addtogroup XBT_heap
  *  @brief This section describes the API to generic heap with O(log(n)) access.
  *
- *  If you are using C++ you might want to use std::priority_queue instead.
+ *  @deprecated If you are using C++ you might want to use `std::priority_queue`
+ *  instead.
  *
  *  @{
  */
index 318be64..a0f6b3e 100644 (file)
@@ -24,7 +24,8 @@ SG_BEGIN_DECL()
  *  It is basically a fifo but with restrictions so that it can be used as a set. Any operation (add, remove, belongs)
  *  is O(1) and no call to malloc/free is done.
  *
- *  If you are using C++, you might want to use boost::intrusive::set instead.
+ *  @deprecated If you are using C++, you might want to use
+ *  `boost::intrusive::set` instead.
  */
 /** @defgroup XBT_swag_type Swag types
     @ingroup XBT_swag
index 2fc18ea..1fd9e49 100644 (file)
 namespace simgrid {
 namespace mc {
 
-/** Process index used when no process is available
+/** Process index used when no process is available (SMPI privatization)
  *
  *  The expected behavior is that if a process index is needed it will fail.
  * */
 const int ProcessIndexMissing = -1;
 
-/** Process index used when we don't care about the process index
+/** Process index used when we don't care about the process index (SMPI privatization)
  * */
 const int ProcessIndexDisabled = -2;
 
-/** Constant used when any process will do.
+/** Constant used when any process will do (SMPI privatization)
  *
- *  This is is index of the first process.
+ *  Note: This is is index of the first process.
  */
 const int ProcessIndexAny = 0;
 
@@ -102,6 +102,10 @@ public:
  *  * the current state of an existing process;
  *
  *  * a snapshot.
+ *
+ *  In order to support SMPI privatization, the can read the memory from the
+ *  context of a given SMPI process: if specified, the code reads data from the
+ *  correct SMPI privatization VMA.
  */
 class AddressSpace {
 private:
@@ -110,12 +114,16 @@ public:
   AddressSpace(Process* process) : process_(process) {}
   virtual ~AddressSpace();
 
+  /** The process of this addres space
+   *
+   *  This is where we can get debug informations, memory layout, etc.
+   */
   simgrid::mc::Process* process() const { return process_; }
 
   /** Read data from the address space
    *
    *  @param buffer        target buffer for the data
-   *  @param size          number of bytes
+   *  @param size          number of bytes to read
    *  @param address       remote source address of the data
    *  @param process_index which process (used for SMPI privatization)
    *  @param options
@@ -137,7 +145,10 @@ public:
     this->read_bytes(buffer.getBuffer(), sizeof(T), ptr, process_index);
   }
 
-  /** Read a given data structure from the address space */
+  /** Read a given data structure from the addres space
+   *
+   *  This version returns by value.
+   */
   template<class T> inline
   Remote<T> read(RemotePtr<T> ptr, int process_index = ProcessIndexMissing) const
   {
@@ -146,13 +157,13 @@ public:
     return res;
   }
 
+  /** Read a string of known size */
   std::string read_string(RemotePtr<char> address, std::size_t len) const
   {
-    // TODO, use std::vector with .data() in C++17 to avoid useless copies
-    std::vector<char> buffer(len);
-    buffer[len] = '\0';
-    this->read_bytes(buffer.data(), len, address);
-    return std::string(buffer.data(), buffer.size());
+    std::string res;
+    res.resize(len);
+    this->read_bytes(&res[0], len, address);
+    return res;
   }
 
 };
index 64219c4..30e03c6 100644 (file)
@@ -18,8 +18,8 @@ namespace mc {
 
 /** A channel for exchanging messages between model-checker and model-checked
  *
- *  This hides the way the messages are transferred. Currently, they are sent
- *  over a SOCK_DGRAM socket.
+ *  This abstracts away the way the messages are transferred. Currently, they
+ *  are sent over a (connected) `SOCK_DGRAM` socket.
  */
 class Channel {
   int socket_ = -1;
index 70a9031..4575439 100644 (file)
@@ -20,13 +20,16 @@ namespace mc {
 
 /** A model-checking algorithm
  *
- *  The goal is to move the data/state/configuration of a model-checking
- *  algorithms in subclasses. Implementing this interface will probably
- *  not be really mandatory, you might be able to write your
- *  model-checking algorithm as plain imperative code instead.
+ *  This is an abstract base class used to group the data, state, configuration
+ *  of a model-checking algorithm.
  *
- *  It works by manipulating a model-checking Session.
- */
+ *  Implementing this interface will probably not be really mandatory,
+ *  you might be able to write your model-checking algorithm as plain
+ *  imperative code instead.
+ *
+ *  It is expected to interact with the model-checking core through the
+ * `Session` interface (but currently the `Session` interface does not
+ *  have all the necessary features). */
 // abstract
 class Checker {
   Session* session_;
@@ -38,16 +41,23 @@ public:
   Checker& operator=(Checker const&) = delete;
 
   virtual ~Checker();
+
+  /** Main function of this algorithm */
   virtual int run() = 0;
 
-  // Give me your internal state:
+  /* These methods are callbacks called by the model-checking engine
+   * to get and display information about the current state of the
+   * model-checking algorithm: */
 
   /** Show the current trace/stack
    *
-   *  Could this be handled in the Session/ModelChecker instead?
-   */
+   *  Could this be handled in the Session/ModelChecker instead? */
   virtual RecordTrace getRecordTrace();
+
+  /** Generate a textual execution trace of the simulated application */
   virtual std::vector<std::string> getTextualTrace();
+
+  /** Log additional information about the state of the model-checker */
   virtual void logState();
 
 protected:
index ae6efb1..076e607 100644 (file)
 namespace simgrid {
 namespace mc {
 
-/** A byte-string represented as a sequence of chunks from a PageStor */
+/** A byte-string represented as a sequence of chunks from a PageStore
+ *
+ *  In order to save memory when taking memory snapshots, a given byte-string
+ *  is split in fixed-size chunks. Identical chunks (either from the same
+ *  snapshot or more probably from different snpashots) share the same memory
+ *  storage.
+ *
+ *  Thus a chunked is represented as a sequence of indices of each chunk.
+ */
 class ChunkedData {
+  /** This is where we store the chunks */
   PageStore* store_ = nullptr;
-  /** Indices of the chunks */
+  /** Indices of the chunks in the `PageStore` */
   std::vector<std::size_t> pagenos_;
 public:
 
@@ -72,10 +81,16 @@ public:
     return *this;
   }
 
+  /** How many pages are used */
   std::size_t page_count()          const { return pagenos_.size(); }
+
+  /** Get a chunk index */
   std::size_t pageno(std::size_t i) const { return pagenos_[i]; }
+
+  /** Get a view of the chunk indices */
   const std::size_t* pagenos()      const { return pagenos_.data(); }
 
+  /** Get a a pointer to a chunk */
   const void* page(std::size_t i) const
   {
     return store_->get_page(pagenos_[i]);
index 4f16164..4b1f202 100644 (file)
@@ -19,9 +19,9 @@
 #include "src/mc/mc_forward.hpp"
 #include "src/mc/AddressSpace.hpp"
 
-/** @file DwarfExession.hpp
+/** @file DwarfExpression.hpp
  *
- *  Evaluation of DWARF location expressions
+ *  Evaluation of DWARF location expressions.
  */
 
 namespace simgrid {
@@ -30,7 +30,7 @@ namespace dwarf {
 /** A DWARF expression
  *
  *  DWARF defines a simple stack-based VM for evaluating expressions
- *  (such as locations of variables, etc.): A DWARF expressions is
+ *  (such as locations of variables, etc.): a DWARF expression is
  *  just a sequence of dwarf instructions. We currently directly use
  *  `Dwarf_Op` from `dwarf.h` for dwarf instructions.
  */
@@ -71,6 +71,7 @@ public:
   typedef std::uintptr_t value_type;
   static const std::size_t max_size = 64;
 private:
+  // Values of the stack (the top is stack_[size_ - 1]):
   uintptr_t stack_[max_size];
   size_t size_;
 public:
@@ -103,7 +104,7 @@ public:
   void push(value_type value)
   {
     if (size_ == max_size)
-      throw evaluation_error("Dwarf stack overflow");
+      throw evaluation_error("DWARF stack overflow");
     stack_[size_++] = value;
   }
 
@@ -111,7 +112,7 @@ public:
   value_type pop()
   {
     if (size_ == 0)
-      throw evaluation_error("Stack underflow");
+      throw evaluation_error("DWARF stack underflow");
     return stack_[--size_];
   }
 
index 40f9ed3..135b79c 100644 (file)
@@ -22,8 +22,8 @@ void* Frame::frame_base(unw_cursor_t& unw_cursor) const
     return location.address();
   else if (location.in_register()) {
     // This is a special case.
-    // The register if not the location of the frame base
-    // (a frame base cannot be located in a register)
+    // The register is not the location of the frame base
+    // (a frame base cannot be located in a register).
     // Instead, DWARF defines this to mean that the register
     // contains the address of the frame base.
     unw_word_t word;
index 52efb5b..830f024 100644 (file)
 namespace simgrid {
 namespace dwarf {
 
-/** \brief A DWARF expression with optional validity constraints */
+/** A DWARF expression with optional validity constraints */
 class LocationListEntry {
 public:
   typedef simgrid::xbt::Range<std::uint64_t> range_type;
 private:
   DwarfExpression expression_;
+  // By default, the expression is always valid:
   range_type range_ = {0, UINT64_MAX};
 public:
   LocationListEntry() {}
index 2023655..5d0e4af 100644 (file)
@@ -294,10 +294,10 @@ bool ModelChecker::handle_message(char* buffer, ssize_t size)
   return true;
 }
 
-/** Terminate the model-checker aplication */
+/** Terminate the model-checker application */
 void ModelChecker::exit(int status)
 {
-  // TODO, terminate the model checker politely instead of exiting rudel
+  // TODO, terminate the model checker politely instead of exiting rudely
   if (process().running())
     kill(process().pid(), SIGKILL);
   ::exit(status);
index 59280f9..96835b8 100644 (file)
 namespace simgrid {
 namespace mc {
 
-ObjectInformation::ObjectInformation()
-{
-  this->flags = 0;
-  this->start = nullptr;
-  this->end = nullptr;
-  this->start_exec = nullptr;
-  this->end_exec = nullptr;
-  this->start_rw = nullptr;
-  this->end_rw = nullptr;
-  this->start_ro = nullptr;
-  this->end_ro = nullptr;
-}
+ObjectInformation::ObjectInformation() {}
 
-/** Find the DWARF offset for this ELF object
- *
- *  An offset is applied to address found in DWARF:
- *
- *  * for an executable object, addresses are virtual address
- *    (there is no offset) i.e.
- *    \f$\text{virtual address} = \{dwarf address}\f$;
+/* For an executable object, addresses are virtual address
+ * (there is no offset) i.e.
+ * \f$\text{virtual address} = \{dwarf address}\f$;
  *
- *  * for a shared object, the addreses are offset from the begining
- *    of the shared object (the base address of the mapped shared
- *    object must be used as offset
- *    i.e. \f$\text{virtual address} = \text{shared object base address}
+ * For a shared object, the addreses are offset from the begining
+ * of the shared object (the base address of the mapped shared
+ * object must be used as offset
+ * i.e. \f$\text{virtual address} = \text{shared object base address}
  *             + \text{dwarf address}\f$.
  */
 void *ObjectInformation::base_address() const
 {
+  // For an executable (more precisely for a ET_EXEC) the base it 0:
   if (this->executable())
     return nullptr;
 
+  // For an a shared-object (ET_DYN, including position-independant executables)
+  // the base address is its lowest address:
   void *result = this->start_exec;
   if (this->start_rw != nullptr && result > (void *) this->start_rw)
     result = this->start_rw;
@@ -55,7 +43,6 @@ void *ObjectInformation::base_address() const
   return result;
 }
 
-/* Find a function by instruction pointer */
 simgrid::mc::Frame* ObjectInformation::find_function(const void *ip) const
 {
   /* This is implemented by binary search on a sorted array.
@@ -146,16 +133,16 @@ void ObjectInformation::remove_global_variable(const char* name)
   }
 }
 
-/** \brief Ignore a local variable in a scope
+/** Ignore a local variable in a scope
  *
  *  Ignore all instances of variables with a given name in
  *  any (possibly inlined) subprogram with a given namespaced
  *  name.
  *
- *  \param var_name        Name of the local variable (or parameter to ignore)
- *  \param subprogram_name Name of the subprogram to ignore (nullptr for any)
- *  \param subprogram      (possibly inlined) Subprogram of the scope
- *  \param scope           Current scope
+ *  @param var_name        Name of the local variable to ignore
+ *  @param subprogram_name Name of the subprogram to ignore (nullptr for any)
+ *  @param subprogram      (possibly inlined) Subprogram of the scope current scope
+ *  @param scope           Current scope
  */
 static void remove_local_variable(simgrid::mc::Frame& scope,
                             const char *var_name,
index 9a55d87..e65d9d3 100644 (file)
@@ -33,23 +33,23 @@ struct FunctionIndexEntry {
   simgrid::mc::Frame* function;
 };
 
-/** Information about an (ELF) executable/sharedobject
+/** Information about an ELF module (executable or shared object)
+ *
+ *  This contains all the information we need about an executable or
+ *  shared-object in the model-checked process:
  *
- *  This contain sall the information we have at runtime about an
- *  executable/shared object in the target (modelchecked) process:
  *  - where it is located in the virtual address space;
- *  - where are located it's different memory mapping in the the
- *    virtual address space ;
- *  - all the debugging (DWARF) information,
- *    - location of the functions,
- *    - types
- *  - etc.
  *
- *  It is not copyable because we are taking pointers to Types/Frames.
- *  We'd have to update/rebuild some data structures in order to copy
- *  successfully.
+ *  - where are located its different memory mappings in the the
+ *    virtual address space;
+ *
+ *  - all the debugging (DWARF) information
+ *    - types,
+ *    - location of the functions and their local variables,
+ *    - global variables,
+ *
+ *  - etc.
  */
-
 class ObjectInformation {
 public:
   ObjectInformation();
@@ -62,45 +62,109 @@ public:
   static const int Executable = 1;
 
   /** Bitfield of flags */
-  int flags;
+  int flags = 0;
   std::string file_name;
-  const void* start;
-  const void *end;
-  char *start_exec;
-  char *end_exec; // Executable segment
-  char *start_rw;
-  char *end_rw; // Read-write segment
-  char *start_ro;
-  char *end_ro; // read-only segment
+  const void* start = nullptr;
+  const void *end = nullptr;
+  // Location of its text segment:
+  char *start_exec = nullptr;
+  char *end_exec = nullptr;
+  // Location of the read-only part of its data segment:
+  char *start_rw = nullptr;
+  char *end_rw = nullptr;
+  // Location of the read/write part of its data segment:
+  char *start_ro = nullptr;
+  char *end_ro = nullptr;
+
+  /** All of its subprograms indexed by their address */
   std::unordered_map<std::uint64_t, simgrid::mc::Frame> subprograms;
+
+  /** Index of functions by instruction address
+   *
+   * We need to efficiently find the function from any given instruction
+   * address inside its range. This index is sorted by low_pc
+   * 
+   * The entries are sorted by low_pc and a binary search can be used to look
+   * them up. In order to have a better cache locality, we only keep the
+   * information we need for the lookup in this vector. We could probably
+   * replace subprograms by an ordered vector of Frame and replace this one b
+   * a parallel `std::vector<void*>`.
+   */
+  std::vector<FunctionIndexEntry> functions_index;
+
   // TODO, remove the mutable (to remove it we'll have to add a lot of const everywhere)
   mutable std::vector<simgrid::mc::Variable> global_variables;
+
+  /** Types indexed by DWARF ID */
   std::unordered_map<std::uint64_t, simgrid::mc::Type> types;
-  std::unordered_map<std::string, simgrid::mc::Type*> full_types_by_name;
 
-  /** Index of functions by IP
+  /** Types indexed by name
    *
-   * The entries are sorted by low_pc and a binary search can be used to look
-   * them up. Should we used a binary tree instead?
+   *  Different compilation units have their separate type definitions
+   *  (for the same type). When we find an opaque type in one compilation unit,
+   *  we use this in order to try to find its definition in another compilation
+   *  unit.
    */
-  std::vector<FunctionIndexEntry> functions_index;
+  std::unordered_map<std::string, simgrid::mc::Type*> full_types_by_name;
 
+  /** Whether this module is an executable
+   *
+   *  More precisely we check if this is an ET_EXE ELF. These ELF files
+   *  use fixed addresses instead of base-addres relative addresses.
+   *  Position independant executables are in fact ET_DYN.
+   */
   bool executable() const
   {
     return this->flags & simgrid::mc::ObjectInformation::Executable;
   }
 
+  /** Base address of the module
+   *
+   *  All the location information in ELF and DWARF are expressed as an offsets
+   *  from this base address:
+   *
+   *  - location of the functions and globale variables;
+   *
+   *  - the DWARF instruction `OP_addr` pushes this on the DWARF stack.
+   **/
   void* base_address() const;
 
+  /** Find a function by instruction address
+   *
+   *  @param ip instruction address
+   *  @return corresponding function (if any) or nullptr
+   */
   simgrid::mc::Frame* find_function(const void *ip) const;
+
+  /** Find a global variable by name
+   *
+   *  This is used to ignore global variables and to find well-known variables
+   *  (`__mmalloc_default_mdp`).
+   *
+   *  @param name scopes name of the global variable (`myproject::Foo::count`)
+   *  @return corresponding variable (if any) or nullptr
+   */
   simgrid::mc::Variable* find_variable(const char* name) const;
+
+  /** Remove a global variable (in order to ignore it)
+   *
+   *  This is used to ignore a global variable for the snapshot comparison.
+   */
   void remove_global_variable(const char* name);
+
+  /** Remove a loval variables (in order to ignore it)
+   *
+   *  @param name Name of the globale variable
+   *  @param scope Namespaceed name of the function (or null for all functions)
+   */
   void remove_local_variable(
     const char* name, const char* scope);
 };
 
 XBT_PRIVATE std::shared_ptr<ObjectInformation> createObjectInformation(
   std::vector<simgrid::xbt::VmMap> const& maps, const char* name);
+
+/** Augment the current module with informations about the other ones */
 XBT_PRIVATE void postProcessObjectInformation(
   simgrid::mc::Process* process, simgrid::mc::ObjectInformation* info);
 
index 5c11ddc..e69af01 100644 (file)
 namespace simgrid {
 namespace mc {
 
-template<class M, class T, class Enable = void>
-struct pointer_to_data_member {};
-template<class M, class T>
-struct pointer_to_data_member<M,T,typename std::enable_if< std::is_union<M>::value || std::is_class<M>::value >::type> {
-  typedef T M::* type;
-};
-
-template<class M, class T>
-using pointer_to_data_member_t = typename pointer_to_data_member<M,T>::type;
-
 /** HACK, A value from another process
  *
  *  This represents a value from another process:
index d619a96..3d08678 100644 (file)
@@ -1044,6 +1044,9 @@ void read_dwarf_info(simgrid::mc::ObjectInformation* info, Dwarf* dwarf)
 static
 std::vector<char> get_build_id(Elf* elf)
 {
+  // Summary: the GNU build ID is stored in a ("GNU, NT_GNU_BUILD_ID) note
+  // found in a PT_NOTE entry in the program header table.
+
   size_t phnum;
   if (elf_getphdrnum (elf, &phnum) != 0)
     xbt_die("Could not read program headers");
@@ -1059,22 +1062,21 @@ std::vector<char> get_build_id(Elf* elf)
 
     // Iterate over the notes and find the NT_GNU_BUILD_ID one:
     size_t pos = 0;
-    while (1) {
+    while (pos < data->d_size) {
       GElf_Nhdr nhdr;
+      // Location of the name within Elf_Data:
       size_t name_pos;
       size_t desc_pos;
       pos = gelf_getnote(data, pos, &nhdr, &name_pos, &desc_pos);
-      // A note is identified by a name "GNU" and a integer type within
-      // the namespace defined by this name (here NT_GNU_BUILD_ID):
+      // A build ID note is identified by the pair ("GNU", NT_GNU_BUILD_ID)
+      // (a namespace and a type within this namespace):
       if (nhdr.n_type == NT_GNU_BUILD_ID
           && nhdr.n_namesz == sizeof("GNU")
           && memcmp((char*) data->d_buf + name_pos, "GNU", sizeof("GNU")) == 0) {
-
-        // Found the NT_GNU_BUILD_ID note:
+        XBT_DEBUG("Found GNU/NT_GNU_BUILD_ID note");
         char* start = (char*) data->d_buf + desc_pos;
         char* end = (char*) start + nhdr.n_descsz;
         return std::vector<char>(start, end);
-
       }
     }
 
@@ -1129,18 +1131,25 @@ const char* debug_paths[] = {
  *  This is one of the mechanisms used for
  *  [separate debug files](https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html).
  */
+// Example:
+// /usr/lib/debug/.build-id/0b/dc77f1c29aea2b14ff5acd9a19ab3175ffdeae.debug
 static
 std::string find_by_build_id(std::vector<char> id)
 {
   std::string filename;
+  std::string hex = to_hex(id);
   for (const char* debug_path : debug_paths) {
-    filename = debug_path;
-    filename += ".build-id/" + to_hex(id.data(), 1) + '/'
+    // Example:
+    filename = std::string(debug_path) + ".build-id/"
+      + to_hex(id.data(), 1) + '/'
       + to_hex(id.data() + 1, id.size() - 1) + ".debug";
     XBT_DEBUG("Checking debug file: %s", filename.c_str());
-    if (access(filename.c_str(), F_OK) == 0)
+    if (access(filename.c_str(), F_OK) == 0) {
+      XBT_DEBUG("Found debug file: %s\n", hex.c_str());
       return filename;
+    }
   }
+  XBT_DEBUG("Not debuf info found for build ID %s\n", hex.data());
   return std::string();
 }
 
@@ -1150,7 +1159,7 @@ std::string find_by_build_id(std::vector<char> id)
  *  lists of types, variables, functions.
  */
 static
-void MC_dwarf_get_variables(simgrid::mc::ObjectInformation* info)
+void MC_load_dwarf(simgrid::mc::ObjectInformation* info)
 {
   if (elf_version(EV_CURRENT) == EV_NONE)
     xbt_die("libelf initialization error");
@@ -1161,12 +1170,12 @@ void MC_dwarf_get_variables(simgrid::mc::ObjectInformation* info)
     xbt_die("Could not open file %s", info->file_name.c_str());
   Elf* elf = elf_begin(fd, ELF_C_READ, nullptr);
   if (elf == nullptr)
-    xbt_die("Not an ELF file 1");
+    xbt_die("Not an ELF file");
   Elf_Kind kind = elf_kind(elf);
   if (kind != ELF_K_ELF)
-    xbt_die("Not an ELF file 2");
+    xbt_die("Not an ELF file");
 
-  // Remember if this is a `ET_EXEC` (fixed location) or `ET_DYN` (relocatable):
+  // Remember if this is a `ET_EXEC` (fixed location) or `ET_DYN`:
   Elf64_Half type = get_type(elf);
   if (type == ET_EXEC)
     info->flags |= simgrid::mc::ObjectInformation::Executable;
@@ -1182,8 +1191,15 @@ void MC_dwarf_get_variables(simgrid::mc::ObjectInformation* info)
   }
   dwarf_end(dwarf);
 
-  // If there was no DWARF in the file, try to find it in a separate file
-  // with NT_GNU_BUILD_ID:
+  // If there was no DWARF in the file, try to find it in a separate file.
+  // Different methods might be used to store the DWARF informations:
+  //  * GNU NT_GNU_BUILD_ID;
+  //  * .gnu_debuglink.
+  // See https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html
+  // for reference of what we are doing.
+
+  // Try with NT_GNU_BUILD_ID: we find the build ID in the ELF file and then
+  // use this ID to find the file in some known locations in the filesystem.
   std::vector<char> build_id = get_build_id(elf);
   if (!build_id.empty()) {
     elf_end(elf);
@@ -1193,8 +1209,7 @@ void MC_dwarf_get_variables(simgrid::mc::ObjectInformation* info)
     std::string debug_file = find_by_build_id(build_id);
     if (debug_file.empty()) {
       std::string hex = to_hex(build_id);
-      xbt_die(
-        "Missing debug info for %s with build-id %s\n"
+      xbt_die("Missing debug info for %s with build-id %s\n"
         "You might want to install the suitable debugging package.\n",
         info->file_name.c_str(), hex.c_str());
     }
@@ -1215,9 +1230,10 @@ void MC_dwarf_get_variables(simgrid::mc::ObjectInformation* info)
     return;
   }
 
-  // TODO, try to find DWARF info using debug-link.
-  // Is this method really used anywhere?
+  // TODO, try to find DWARF info using .gnu_debuglink.
 
+  elf_end(elf);
+  close(fd);
   xbt_die("Debugging information not found for %s\n"
     "Try recompiling with -g\n",
     info->file_name.c_str());
@@ -1343,7 +1359,7 @@ std::shared_ptr<simgrid::mc::ObjectInformation> createObjectInformation(
     std::make_shared<simgrid::mc::ObjectInformation>();
   result->file_name = name;
   simgrid::mc::find_object_address(maps, result.get());
-  MC_dwarf_get_variables(result.get());
+  MC_load_dwarf(result.get());
   MC_post_process_variables(result.get());
   MC_post_process_types(result.get());
   for (auto& entry : result.get()->subprograms)
index 7901b67..ec24b70 100644 (file)
@@ -4,7 +4,8 @@
 /* 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. */
 
-/** \file
+/** \file mc_record.h
+ *
  *  This file contains the MC replay/record functionnality.
  *  A MC path may be recorded by using ``-cfg=model-check/record:1`'`.
  *  The path is written in the log output and an be replayed with MC disabled
index d92703c..f9324c8 100644 (file)
@@ -4,8 +4,6 @@
 /* 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. */
 
-/* simgrid/modelchecker.h - Formal Verification made possible in SimGrid    */
-
 #ifndef SIMGRID_MC_REPLAY_H
 #define SIMGRID_MC_REPLAY_H
 
index 28348f0..87bdcd2 100644 (file)
@@ -627,7 +627,7 @@ set(headers_to_install
   include/msg/datatypes.h
   include/simdag/simdag.h
   include/simdag/datatypes.h
-  
+  include/simgrid/chrono.hpp
   include/simgrid/plugins/energy.h
   include/simgrid/instr.h
   include/simgrid/msg.h
index 961254f..6c19744 100755 (executable)
@@ -30,7 +30,7 @@ $DICTFILE="./spell_dict.txt" unless (-e $DICTFILE);
 die "Call this script from its location or from the SimGrid root directory\n" unless (-e $DICTFILE);
 
 die "Usage: ". ($DICTFILE eq "./spell_dict.txt"? "./":"tools/internal/")."spell_comments.pl "
-           ."`find ". ($DICTFILE eq "./spell_dict.txt"? "../../":".")." -name '*.[ch]' -o -name '*.hpp' -o -name '*.cpp' |grep -v umpire|grep -v smpi/mpich3-test|grep -v NAS`\n"
+           ."`find ". ($DICTFILE eq "./spell_dict.txt"? "../../":".")." -name '*.[ch]' -o -name '*.hpp' -o -name '*.cpp' |grep -v umpire|grep -v smpi/mpich3-test|grep -v NAS | grep -v src/smpi/colls`\n"
   unless scalar(@ARGV)>0;
 
 my $total = 0;
index 908ed6e..6201f57 100755 (executable)
@@ -27,7 +27,7 @@ installSonarQubeScanner() {
   export SONAR_SCANNER_OPTS="-server"
 }
 installBuildWrapper() {
-  curl -LsS https://nemo.sonarqube.org/static/cpp/build-wrapper-linux-x86.zip > build-wrapper-linux-x86.zip
+  curl -LsS https://sonarqube.com/static/cpp/build-wrapper-linux-x86.zip > build-wrapper-linux-x86.zip
   unzip build-wrapper-linux-x86.zip
 }
 installSonarQubeScanner
@@ -37,4 +37,5 @@ installBuildWrapper
 ./build-wrapper-linux-x86/build-wrapper-linux-x86-64 --out-dir bw-outputs "$@"
 
 # and finally execute the actual SonarQube analysis (the SONAR_TOKEN is set from the travis web interface, to not expose it)
-sonar-scanner -Dsonar.host.url=https://nemo.sonarqube.org -Dsonar.login=$SONAR_TOKEN
+# See https://docs.travis-ci.com/user/sonarqube/ for more info on tokens
+sonar-scanner -Dsonar.host.url=https://sonarqube.com -Dsonar.login=$SONAR_TOKEN