[PATCH] D99784: [LICM] Hoist loads with invariant.group metadata

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Thanks for the repro!

  while (true) {
    std::string FPath = StrCat(dir, "index", Idx++, "/");
    std::ifstream f(StrCat(FPath, "size"));
    if (!f.is_open()) break;
    std::string suffix;
    int size;
    f >> size;
    if (f.fail()) // BUS error
      return;
    if (f.good()) {
      std::cout << "read " << size << "\n";
    }
  }

Clang is able to devirtualize the `fail` call on its own. However, it still has IR to perform the virtual base class adjustment:

  %13 = bitcast %"class.std::basic_ifstream"* %f to %"class.std::basic_istream"*
  %call3 = call nonnull align 8 dereferenceable(16) %"class.std::basic_istream"* @_ZNSirsERi(%"class.std::basic_istream"* nonnull dereferenceable(16) %13, i32* nonnull align 4 dereferenceable(4) %size)
  %14 = bitcast %"class.std::basic_ifstream"* %f to i8**
  %vtable = load i8*, i8** %14, align 8, !tbaa !10, !invariant.group !12
  %vbase.offset.ptr = getelementptr i8, i8* %vtable, i64 -24
  %15 = bitcast i8* %vbase.offset.ptr to i64*
  %vbase.offset = load i64, i64* %15, align 8
  %16 = bitcast %"class.std::basic_ifstream"* %f to i8*
  %add.ptr = getelementptr inbounds i8, i8* %16, i64 %vbase.offset
  %17 = bitcast i8* %add.ptr to %"class.std::basic_ios"*
  %call4 = call zeroext i1 @_ZNKSt9basic_iosIcSt11char_traitsIcEE4failEv(%"class.std::basic_ios"* nonnull dereferenceable(264) %17)

This vtable load is based directly on the `%f` alloca, and not the result of a launder operation. Because there is no launder operation, LICM hoists this vptr load to the entry block, so it loads uninit stack memory. The virtual base class offset load happens before calling `fail`, which is where the crash happens.

So: this is a frontend bug, and perhaps a frontend missed optimization. In general, clang does not launder local variable allocas, and that seems like it could be a problem. However, always laundering and stripping every dynamic local variable is probably way overkill, and would block SROA. Maybe we could only use laundered object pointers for the vptr loads.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99784/new/

https://reviews.llvm.org/D99784