[Openmp-commits] [PATCH] D70412: [OpenMP][Tool] Runtime warning for missing TSan-option

Tue Jan 14 23:59:00 PST 2020

Hahnfeld added a comment.

In D70412#1821136 <https://reviews.llvm.org/D70412#1821136>, @protze.joachim wrote:

> In D70412#1820406 <https://reviews.llvm.org/D70412#1820406>, @Hahnfeld wrote:
>
> > This revision was not accepted before being committed!
>
>
> It was a misunderstanding on my side. Johannes told me that he is fine with the changes before I pushed. I didn't check that the accepted flag was not on the patch.
>  What procedure do you suggest?

Now that the patch is already in the repository, I don't know how much sense it would make to revert it in full. We should still fix the memory leak asap and make sure it's in the release.

================
Comment at: openmp/tools/archer/ompt-tsan.cpp:868
+  options = getenv("TSAN_OPTIONS");
+  tsan_flags = new TsanFlags(options);

----------------
Hahnfeld wrote:
> This memory is leaked, it should be `delete`d in `ompt_tsan_finalize`
Do we really need global lifetime for `tsan_flags`? Right now it would be enough to just have the object allocated on the stack and let the compiler handle the cleanup.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70412/new/

https://reviews.llvm.org/D70412