[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi all, it looks like this commit led to some unexpected behavior in our build. When compiling something like:

  // clang++ -c -S -o - /tmp/test.cc -std=c++17 -O1
  static int do_nothing(void* unused) {
    for (;;) {
    }
    return 0;
  }
  
  typedef int (*func_t)(void *);
  void call(func_t);
  
  void func() {
    call(&do_nothing);
  }

we get the following assembly for `do_nothing`:

  	.p2align	4, 0x90                         # -- Begin function _ZL10do_nothingPv
  	.type	_ZL10do_nothingPv, at function
  _ZL10do_nothingPv:                      # @_ZL10do_nothingPv
  	.cfi_startproc
  # %bb.0:
  .Lfunc_end1:
  	.size	_ZL10do_nothingPv, .Lfunc_end1-_ZL10do_nothingPv
  	.cfi_endproc
                                          # -- End function
  	.ident	"Fuchsia clang version 13.0.0 (https://llvm.googlesource.com/a/llvm-project 6555e53ab0f2bca3dc30f5ab3a2d6872d50b6ff8)"
  	.section	".note.GNU-stack","", at progbits
  	.addrsig
  	.addrsig_sym _ZL10do_nothingPv

It seems that the function doesn't have a return statement or halting instruction and it would just jump into the next function. While I do follow what this patch is trying to do, this behavior seems pretty unexpected from a C++ user perspective. I could be wrong, but it doesn't seem clear in this case that the infinite loop results in UB which would justify this assembly. Would it be fine to revert this for now to work out the kinks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96418