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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D96418#2780687 <https://reviews.llvm.org/D96418#2780687>, @leonardchan wrote:

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

Well, no, I'm afraid it is actually clear that that code does have UB according to the C++ standard.  Perhaps you mean that it *shouldn't* have UB, or that Clang should define its behavior despite the standard.

I might agree with you that I don't see the value in using this stronger rule in C++, but I think it would help to understand the problem a little better.  I assume this is causing problems for a less trivial test case?  Or do you really have code that's relying on that loop not terminating?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96418