[all-commits] [llvm/llvm-project] aa265c: [asan] isInterestingAlloca: remove the isAllocaPro...

[Fangrui Song via All-commits]

  Branch: refs/heads/users/MaskRay/spr/main.asan-isinterestingalloca-remove-the-isallocapromotable-condition
  Home:   https://github.com/llvm/llvm-project
  Commit: aa265c45914fcb80da787be5cb8e97d94f1b2859
      https://github.com/llvm/llvm-project/commit/aa265c45914fcb80da787be5cb8e97d94f1b2859
  Author: Fangrui Song <i at maskray.me>
  Date:   2024-02-15 (Thu, 15 Feb 2024)

  Changed paths:
    M llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
    M llvm/test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll
    M llvm/test/Instrumentation/AddressSanitizer/hoist-argument-init-insts.ll
    M llvm/test/Instrumentation/AddressSanitizer/local_stack_base.ll

  Log Message:
  -----------
  [asan] isInterestingAlloca: remove the isAllocaPromotable condition (#77221)

Commit 8ed1d8196bef89c3099be4ce4aa65f613ab819cc made an AllocaInst
interesting only if
`!ClSkipPromotableAllocas || !isAllocaPromotable(&AI)`, which greatly
removed memory operand instrumention for -O0. However, this optimization
is subsumed by StackSafetyAnalysis and therefore unnecessary when we
enable StackSafetyAnalysis by default. With this patch,
-fsanitize=address built clang does not change with -O0 or -O3.

Actually, having the `!ClSkipPromotableAllocas ||
!isAllocaPromotable(&AI)`
condition before `!(SSGI && SSGI->isSafe(AI)))` has an interesting false
positive involving MemIntrinsic (see `hoist-argument-init-insts.ll`):

* `isInterestingAlloca` is transitively called by
  `getInterestingMemoryOperands` and `FunctionStackPoisoner`.
* If `getInterestingMemoryOperands` never calls
  `StackSafetyGlobalInfo::getInfo`, and a MemIntrinsic is converted to
  `__asan_memcpy` by `instrumentMemIntrinsic`, when
  `StackSafetyGlobalInfo::getInfo` is called, StackSafetyAnalysis will
  consider `__asan_memcpy` as unsafe, leading to an unnecessary
  alloca instrumentation