Description
Description
PointerMaybeCaptured
, as its name suggests, is supposed to take an arbitrary pointer and analyze whether it might be captured.
However, the current capture checking in PointerMayBeCaptured
may produce false negatives by directly tracking the def-use chain of the given pointer without first backtracking to its underlying object. This leads to missed captures when analyzing derived pointers (e.g., GEP results) with their base objects captured.
Reproducer
Analyze whether &arr[1]
is captured or not.
void foo(char *);
int main() {
int arr[2] = {0, 0};
pthread_t t;
// `arr` is captured through the pointer argument
foo(arr);
// Current implementation considers `arr + 1` as non-captured
arr[1] = 2;
...
}
Analysis of Current Usage
Currently, there are TWO different patterns of the pointer argument passed to PointerMayBeCaptured
.
Safe Patterns (Majority of Existing Uses)
-
Direct patterns with guaranteed base objects:
Alloca
instructions- Function arguments
- Return values with
noalias
attributes - Pointers processed through
getUnderlyingObject()
-
These account for uses in:
InstructionSimplify.cpp
InstCombineCompares.cpp
FunctionAttrs.cpp
(with manual object backtracking)DeadStoreElimination.cpp
SimplifyCFG.cpp
Potentially Problematic Cases
ThreadSanitizer.cpp
: Uses pointer operand of Store/Load directlySanitizerBinaryMetadata.cpp
: Processes pointer operand of Store/Load for GWP-San metadata
The FNs of capture relation of load/store addresses might lead to FN in TSan's race detection, observed in this Godbolt example:
TWO Potential Fixes
-
Fix of Restriction: Explicitly or implicitly restrict the pattern of pointer argument of
PointerMayBeCaptured
- Explicitly state in API comments that a pointer not from a base object might be mis-analyzed as non-captured.
- Or add an inner assertion to limit the passed pointer must be a base object.
If so, the use cases in
ThreadSanitizer.cpp
andSanitizerBinaryMetadata.cpp
need to be fixed -
Fix of Implementation (Recommended):
- Add extra underlying object resolution in the beginning of
PointerMayBeCaptured
:const Value *Obj = getUnderlyingObject(Ptr); return analyzeCaptures(Obj);
- As most of the usages of
PointerMayBeCaptured
just pass the underlying object pointer to it, this fix would not introduce obvious overhead, but eliminate the FN when the argument pointer is not of underlying object.
- Add extra underlying object resolution in the beginning of