[x86] Clang >= 13 strips optimises function to completely empty label, and allows fallthrough, in generated Assembly

[whisperity]

Given the following (arguably sketchy, because it violates (AFAIK) the "forward progress" assumption) example code:

#include <iostream>

int main()
{
  while (1);
  return 0;
}

int unreachable()
{
  std::cout << "Hello!" << std::endl;
  return 1;
}

Clang 13, 14, and 15 are all affected. It seems G++ versions don't "see through" this optimisation "opportunity", even at -O3 keeping the infinite loop.

At optimisation levels -O1 and higher, the resulting generated Assembly body for main is completely empty. The label itself is kept, allowing execution to flow immediately to whatever instruction happens to be following the label. It might even be an invalid instruction or an invalid state (such as the body of a function expecting parameters on the stack while an emptied parameter-less function was called).

main:                        # @main
unreachable:                 # @unreachable()
    push     rbx
    mov      edi, offset std::cout
    # ...

This is problematic because a dangling label remains in the object file whose contents are poorly controlled. Perhaps Clang should, instead, when realising that a function's body is completely emptied, omit even the label (and thus catch such "sketchy codes" with a potential linker error) or leave an instruction in the body that kills the program when reached?

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[llvmbot]

@llvm/issue-subscribers-c-1

[shafik]

See relevant issues: #60622 and #60588 and #48943 (comment)

This feels like a duplicate.

[AaronBallman]

IMO, we should never optimize code such that it results in execution flowing off the end of a function unless someone has concrete performance numbers justifying the shocking behaviors that come out of the optimization. It may be a legal transformation thanks to UB, but it also breaks expectations that are not entirely unreasonable.

[shafik]

There are two questions here.

Is this allowed: Yes.

Should we do this: Probably not, I will note that in #48943 (comment) there was a proposal to turn this into ud2 instead which I think is more reasonable.

[inclyc]

Recently many new issues filed here duplicated. Lets talk about this problem in #48943.

#60637
#60622
#60588
#48943