Improve ASan & HWASan handling of pre-split coroutines to disable stack UAR instrumentation

[rnk]

ASan & HWASan instrumentation passes both contain logic to move all static allocas over to a dynamically allocated stack frame which can be used to support stack use-after-return detection. In the entry block, it introduces a runtime check if/else pattern to select between dynamically allocated stack memory and heap allocated memory when UAR detection is enabled.

This conflicts with the CoroSplit pass logic, which emits a fatal error for dynamic allocas, which would have to be heap allocated (i.e. rewrite to malloc) to live across suspend points.

There's something obvious here: The passes are both fundamentally doing the same thing. They are moving all stack allocations to the heap.

Normally the pass sequence is corosplit, then sanitizer instruentation. However, to be resilient to odd pass reorderings, it would be better if sanitizer passes simply didn't do UAR detection on pre-split coroutines. This eliminates the pass ordering requirement completely.

Right now, ASan is blind to bugs in coroutine functions in LTO configurations, because we just skip instrumenting pre-split coroutines. Fixing this would improve its defect detection capability.

References:

• [hwasan] Port "[Asan] Skip pre-split coroutine and noop coroutine frame (#99415)" #154803
• [Asan] Skip pre-split coroutine and noop coroutine frame #99415
• aa0776d

cc @thurstond @vitalybuka

One thing I'm not clear on is whether we can retain redzone poisoning in all coro / sanitizer pass sequencings.

[thurstond]

ASan & HWASan instrumentation passes both contain logic to move all static allocas over to a dynamically allocated stack frame which can be used to support stack use-after-return detection. In the entry block, it introduces a runtime check if/else pattern to select between dynamically allocated stack memory and heap allocated memory when UAR detection is enabled.

HWASan does not have the fake stack / heap-ified stack frames. But, unlike ASan, HWASan does have to replace alloca uses to use tagged addresses.

[rnk]

HWASan does not have the fake stack / heap-ified stack frames. But, unlike ASan, HWASan does have to replace alloca uses to use tagged addresses.

I see, thanks for the correction, these are the risks of attempting to use partial knowledge. :)

What gives rise to the dynamic alloca in hwasan, then? That's the thing that the corosplit pass is complaining about. The basic idea of the issue is that we could disable that in a more targeted way for pre-split coroutines.

[thurstond]

When I run a debug build of LLVM / HWASan, instead of "Coroutines cannot handle non static allocas yet" I hit an earlier assertion, that the PromiseArg cannot be cast to an AllocaInst:

/// This represents the llvm.coro.id instruction.
class CoroIdInst : public AnyCoroIdInst {
  enum { AlignArg, PromiseArg, CoroutineArg, InfoArg };

public:
  AllocaInst *getPromise() const {
    Value *Arg = getArgOperand(PromiseArg);
    return isa<ConstantPointerNull>(Arg)
               ? nullptr
               : cast<AllocaInst>(Arg->stripPointerCasts());
               // THIS CAST IS INVALID
  }

Normally, we have something like:

  %__promise = alloca %struct.Promise, align 8
  %0 = call token @llvm.coro.id(i32 16, ptr nonnull %__promise, ptr null, ptr null)

The second parameter to @llvm.coro.id, %__promise, is an alloca (with a fixed size) as expected.

However, HWASan is instrumenting the allocas to get a tagged stack address e.g.,:

  ; HWASan-added instrumentation
  ; StackBaseTag = IRB.CreateAShr(ThreadLong, 3);
  %4 = load i64, ptr @__hwasan_tls, align 8
  %5 = ashr i64 %4, 3

  %54 = alloca { %"class.c9::internal::LowCoPromise", [8 x i8] }, align 16

  ; HWASan-added instrumentation
  ; Tag the alloca
  %55 = xor i64 %5, 192 ; Change the tag so that objects in the same stack frame have different tags
  %56 = ptrtoint ptr %54 to i64
  %57 = and i64 %56, 72057594037927935 ; Constant is 1 << 56. This is masking off the old tag (stored in the top 8 bits).
  %58 = shl i64 %55, 56 ; Move the new tag into the top 8 bits
  %59 = or i64 %57, %58 ; Add the new tag to the pointer
  %60 = inttoptr i64 %59 to ptr

  %127 = call token @llvm.coro.id(i32 8, ptr nonnull %60, ptr nonnull @some_mangled_function_name, ptr null), !dbg !151
  ;                                                  ^
  ;                                              PromiseArg

Now, the second parameter to @llvm.coro.id is %60, which is an inttoptr of an or instruction etc., and very far removed from the base alloca instruction.

• In debug builds, that fails the cast<AllocaInst>
• In non-debug builds, because it can't find the alloca instruction, it is unable to obtain a constant size from the alloca, and therefore emits the "Coroutines cannot handle non static allocas yet" message.

[thurstond]

Likewise, with debug builds of ASan, I also hit the earlier assertion that the PromiseArg cannot be casted to an AllocaInst:

  ; Some IR omitted for brevity
  %0 = call i64 @__asan_stack_malloc_always_3(i64 320)
  %MyAlloca = alloca i8, i64 320, align 32
  %3 = ptrtoint ptr %MyAlloca to i64
  ...
  %5 = phi i64 [ %0, %entry ], [ %3, %2 ]
  %12 = add i64 %5, 128
  %13 = inttoptr i64 %12 to ptr
  ...
  %47 = call token @llvm.coro.id(i32 8, ptr nonnull %13, ptr nonnull @_ZN2c98internal17AsyncNotification8DoNotifyEv, ptr null), !dbg !173
  ;                                                  ^
  ;                                              PromiseArg

Same reasoning as HWASan - @llvm.coro.id's second parameter (%13) is no longer directly (or a pointer to) an alloca.

[thurstond]

it would be better if sanitizer passes simply didn't do UAR detection on pre-split coroutines.

For HWASan, the pointer tagging is the unified mechanism for both spatial and temporal memory safety. i.e., to keep spatial memory safety, we need to tag the pointers, and that will lead to the assertion failure.

---

For ASan: I tried disabling UAR by compiling using -mllvm -asan-use-after-return=never:

  %MyAlloca = alloca i8, i64 320, align 32
  %0 = ptrtoint ptr %MyAlloca to i64
  %1 = add i64 %0, 32
  %2 = inttoptr i64 %1 to ptr
  %3 = add i64 %0, 64
  %4 = inttoptr i64 %3 to ptr
  %5 = add i64 %0, 96
  %6 = inttoptr i64 %5 to ptr
  %7 = add i64 %0, 128
  %8 = inttoptr i64 %7 to ptr
  ...
  %42 = call token @llvm.coro.id(i32 8, ptr nonnull %8, ptr nonnull @_ZN2c98internal17AsyncNotification8DoNotifyEv, ptr null), !dbg !173
  ;                                                 ^
  ;                                             PromiseArg

This still fails the cast assertion, because @llvm.coro.id's second parameter is still not directly (or a pointer to) an alloca.

---

For completeness:

• The ASan output in Improve ASan & HWASan handling of pre-split coroutines to disable stack UAR instrumentation #154830 (comment) was using -mllvm -asan-use-after-return=always
• -mllvm -asan-use-after-return=runtime (the third and last option) also fails the assertion that the second @llvm.coro.id parameter must be an alloca:

  %asan_local_stack_base = alloca i64, align 8
  %0 = load i32, ptr @__asan_option_detect_stack_use_after_return, align 4
  %1 = icmp ne i32 %0, 0
  br i1 %1, label %2, label %4

2:                                                ; preds = %entry
  %3 = call i64 @__asan_stack_malloc_3(i64 320)
  br label %4

4:                                                ; preds = %2, %entry
  %5 = phi i64 [ 0, %entry ], [ %3, %2 ]
  %6 = icmp eq i64 %5, 0
  br i1 %6, label %7, label %9

7:                                                ; preds = %4
  %MyAlloca = alloca i8, i64 320, align 32
  %8 = ptrtoint ptr %MyAlloca to i64
  br label %9

9:                                                ; preds = %7, %4
  %10 = phi i64 [ %5, %4 ], [ %8, %7 ]
  store i64 %10, ptr %asan_local_stack_base, align 8
  %11 = add i64 %10, 32
  %12 = inttoptr i64 %11 to ptr
  %13 = add i64 %10, 64
  %14 = inttoptr i64 %13 to ptr
  %15 = add i64 %10, 96
  %16 = inttoptr i64 %15 to ptr
  %17 = add i64 %10, 128
  %18 = inttoptr i64 %17 to ptr
  %52 = call token @llvm.coro.id(i32 8, ptr nonnull %18, ptr nonnull @_ZN2c98internal17AsyncNotification8DoNotifyEv, ptr null), !dbg !173
  ;                                                  ^
  ;                                              PromiseArg

[thurstond]

For HWASan, it will take some investigation to see if not tagging the promise alloca suffices, or if there are later assertions that still faill