Page MenuHomePhabricator

Fixes a memory leak related to task dependencies (patch from Alex Duran)
ClosedPublic

Authored by AndreyChurbanov on Oct 12 2016, 1:32 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

AndreyChurbanov retitled this revision from to Fixes a memory leak related to task dependencies (patch from Alex Duran).
AndreyChurbanov updated this object.
AndreyChurbanov added reviewers: tlwilmar, jlpeyton.
AndreyChurbanov set the repository for this revision to rL LLVM.
AndreyChurbanov added a subscriber: openmp-commits.

Please run Clang-format over changed code.

Addressed Eugene's comment - passed new code through clang-format.

Please run Clang-format over changed code.

My last information is that the reformatting is still outstanding which means that clang-format will currently lead to inconsistencies.

runtime/src/kmp_taskdeps.cpp
122–148

For example these two functions will be heavily reformatted for no actual benefit

Adjusted output of clang-format to match the style of surrounded code - 4 space indent, function name at the start of line.

tlwilmar accepted this revision.Oct 26 2016, 12:06 PM
tlwilmar edited edge metadata.

LGTM.

Clang-format can wait, as the clang-formatting of entire code base is in the works.

This revision is now accepted and ready to land.Oct 26 2016, 12:06 PM

committed r285283.

pawosm01 reopened this revision.Nov 16 2016, 11:01 AM
pawosm01 added a subscriber: pawosm01.

Segfault at the fery first line of kmp_dephash_free_entries() while trying to dereference a pointer passed as a parameter. The pointer to memory allocated by kmp_fast_allocate() became invalid after __kmp_free_fast_memory() was called.

runtime/src/kmp_runtime.c
5704

See comment below (line 5757)

5757

Calling this after __kmp_free_fast_memory( thread ); was called in line 5704 causes crash on sparselu_taskdep benchmark from kastors suite.

This revision is now accepted and ready to land.Nov 16 2016, 11:01 AM
pawosm01 requested changes to this revision.Nov 16 2016, 11:02 AM
pawosm01 added a reviewer: pawosm01.
This revision now requires changes to proceed.Nov 16 2016, 11:02 AM
Hahnfeld accepted this revision.Feb 15 2017, 12:34 AM

Accepting to allow closing - should have been fixed with rL287551

Hmm, doesn't work. Maybe @pawosm01 has to accept and close this...

pawosm01 accepted this revision.Feb 15 2017, 2:27 AM
This revision is now accepted and ready to land.Feb 15 2017, 2:27 AM