Sanitizer build

[jmgorius]

Add support for sanitizer builds and update the CI configuration accordingly. Simplify the CI configuration slightly by centralizing configuration options in the top-level environment and by using multi-line run commands.

[jmgorius]

There seems to be undefined references to sanitizer functions. I will try to figure out why this happens.

[jmgorius]

The build is now fixed. The sanitizers have detected a memory leak in the simulator.
The leak probably comes from

State *init_state() {
  auto state = new State();
  auto zeroSlot = Slot(Time());
  state->queue.push(zeroSlot);
  return state;
}

in lib/Simulator/signals-runtime-wrappers.cpp. The State instance is never deleted.

[jmgorius]

After some more investigation, it turns out the memory is lost in the call to engine->invoke here

int Engine::simulate(int n) {
  assert(engine && "engine not found");
  assert(state && "state not found");

  SmallVector<void *, 1> arg({&state});
  // initialize simulation state
  auto invocationResult = engine->invoke("llhd_init", arg);
  if (invocationResult) {
    llvm::errs() << "Failed invocation of llhd_init: " << invocationResult;
    return -1;
  }
  // ...
}

[rodonisi]

Thanks a lot for the PR and looking into it, that's awesome!
That specific ExecutionEngine invocation includes malloc'ing memory regions for the signal's values. Those pointers are then stored in the State struct but never free'd explicitly. What would you suggest to be a good way to handle this? Do you think having a State destructor that loops over signals and explicitly calls free for each of them would be fine?

[jmgorius]

Yes, I think it would be a good solution. Right now the leaks are small but I would expect real-world examples to have way more allocations, so taking care of it would not hurt.

If you want to stay C-friendly you could also have a free_state(State *state) function that takes care of the deallocation.

[rodonisi]

It works now!
I don't think there's a need for a C-friendly function for the deallocation, as the state lives and is mainly interacted with in simulator itself

[jmgorius]

I don't think there's a need for a C-friendly function for the deallocation, as the state lives and is mainly interacted with in simulator itself

True. Nice to see that this was the only detected issue!

[rodonisi]

Otherwise there's just a missing newline in the new cmake module, I'm not sure how important that is though

[jmgorius]

Fixed it.

[rodonisi]

Allright, thanks a lot :)