HomePhabricator

[PGO] Don't call calloc(0, sizeof(ValueProfNode *))

Authored by MaskRay on Jul 22 2020, 6:46 PM.

Description

[PGO] Don't call calloc(0, sizeof(ValueProfNode *))

A malloc implementation may return a pointer to some allocated space. It is
undefined for libclang_rt.profile- to access the object - which actually happens
in instrumentTargetValueImpl, where ValueCounters[CounterIndex] may access a
ValueProfNode (from another allocated object) and crashes when the code accesses
the object referenced by CurVNode->Next.

Details

Committed
MaskRayJul 22 2020, 6:49 PM
Parents
rG9e4ab439c2ee: [flang][OpenMP] Added support for lowering OpenMP taskyield construct
Branches
Unknown
Tags
Unknown

Event Timeline

MaskRay added subscribers: yamauchi, xur, davidxl.EditedJul 22 2020, 7:06 PM

@davidxl @xur @yamauchi I ran into a case where calloc(0, sizeof(ValueProfNode *)) was called. POSIX allows this to return either 0 or another allocate space but access to the object is undefined.
Both glibc and tcmalloc seem to return a non-null pointer.

TCMalloc tends to reuse the last deallocated object for calloc(0, 8). In many cases the object was an APInt whose value was either 1 or -1 and clang crashed due to a pointer being 1 or -1. This might be the root cause of https://reviews.llvm.org/D81682#2168130.
(Reverting D81682 did fix a crash for one application for me, so D81682 was probably a snowflake)

It doesn't seem to that NumVSites == 0 is a valid case so you probably need to investigate more. In anycase, if (NumVSites == 0) should probably be retained even after the potential root case is found and fixed, because this can protect a heap reuse vulnerability.