Skip to content

[CaptureTracking] False Negatives in PointerMayBeCaptured Due to Missing Underlying Object Backtracking #132739

Open
@Camsyn

Description

@Camsyn

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 directly
  • SanitizerBinaryMetadata.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

  1. 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 and SanitizerBinaryMetadata.cpp need to be fixed

  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions