C++-Runtime UB in TokenStreamRewriter

[lSoleyl]

The pattern:

delete rewrites[iop->instructionIndex];
rewrites[iop->instructionIndex] = nullptr;

was used throughout this method and is undefined behavior because iop and rewrites[iop->instructionIndex] actually refer to the same object and thus the object's fields are accessed after the object itself has been deleted. This has caused crashes in my application, which are fixed by this commit.

I fixed this by rearranging the object deletions to be performed after any code, which still references them.

I also removed the seemingly out of place std::cout << "new rop" << rop << std::endl; statement.

[mike-lischke]

I'm unsure if that is really a good approach. It requires that rewrites[iop->instructionIndex] is always the same as iop, or put differently: iop doesn't change its position in the rewrites vector. But look at TokenStreamRewriter::rollback where a program is replaced with a new vector that is constructed by copying something out of the existing vector without adjusting the instruction index members.

[lSoleyl]

Thanks for checking in on this pull request... It is a while ago that I thought about it and at the time of writing I came to the conclusion that this assumption of mine always holds... I might have missed somthing, so please check this thoroughly, but at least rollback doesn't change these position of iop

void TokenStreamRewriter::rollback(const std::string &programName, size_t instructionIndex) {
  std::vector<RewriteOperation*> is = _programs[programName];
  if (is.size() > 0) {
    _programs.insert({ programName, std::vector<RewriteOperation*>(is.begin() + MIN_TOKEN_INDEX, is.begin() + instructionIndex) });
  }
}

It just replaces the vector of operations with a new vector containing only the first n elements, which doesn't change the order or the index of the operations inside the vector.