This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] libomp cleanup: add check of input global tid parameter
ClosedPublic

Authored by AndreyChurbanov on Jul 17 2020, 11:24 AM.

Details

Summary

Added checks on negative gtid before indexing __kmp_threads.
This makes static analyzers happier.

Diff Detail

Event Timeline

jdoerfert added a subscriber: tianshilei1992.

@tianshilei1992 Does this impact the "unshakled thread" design?

@tianshilei1992 Does this impact the "unshakled thread" design?

I don't think so. We don't use negative gtid for unshackled threads, but thanks for the head up.

Applied same checks to couple more files.

tianshilei1992 added inline comments.Jul 17 2020, 1:13 PM
openmp/runtime/src/kmp.h
3082

Do we need to check whether gtid is less than the size of __kmp_threads?

Addressed @tianshilei1992 comment - added check of gtid vs array upper bound.

jdoerfert accepted this revision.Jul 18 2020, 9:43 PM

LGTM for the gtid changes. there are at least three other things in this commit that need to be split. I don't think they are bad but not described or related to this.

openmp/runtime/src/kmp_itt.inl
235 ↗(On Diff #278910)

This seems unrelated.

openmp/runtime/src/kmp_runtime.cpp
4961 ↗(On Diff #278910)

I guess this is unrelated too.

openmp/runtime/src/kmp_sched.cpp
68

This belongs in a different commit.

This revision is now accepted and ready to land.Jul 18 2020, 9:43 PM
AndreyChurbanov marked an inline comment as done.Jul 20 2020, 1:33 PM

OK, I will commit the patch in two parts - one pure gtid checks, another one with other arrays/pointers checks.

This revision was automatically updated to reflect the committed changes.