[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added subscribers: pcc, klimek.
a.sidorin added a comment.

Hi Gabor,

> Can't we have the same problem with FileName?

As I can see, no. FileName is copied into std::string while building compilation arguments.

> Perhaps an other alternative would be to make the members real strings.

That was the initial solution and you was correct with it. However, it is still a workaround. The ultimate solution is to fix `buildASTFromCodeWithArgs()` so it can copy the buffer into a persistent storage if it is not null-terminated:

  StringRef NullTerminated = Code.toNullTerminatedStringRef(CodeStorage);
  auto CodeBuffer =
      CodeStorage.empty()
          ? llvm::MemoryBuffer::getMemBuffer(NullTerminated)
          : llvm::MemoryBuffer::getMemBufferCopy(NullTerminated);
  InMemoryFileSystem->addFile(FileNameRef, 0, std::move(CodeBuffer));

Unfortunately, this patch has two problems. The first one is double copying of the input Twine, but it can be avoided with some additional code. The second one is copying of null-terminated StringRefs which, I guess, cannot be avoided at all because there is no legal way to check if StringRef is null-terminated or not. So, I'm summoning the initial authors of the code for the help.

@pcc, @klimek Could you propose a better idea?


Repository:
  rC Clang

https://reviews.llvm.org/D46398