[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> the case from https://github.com/llvm/llvm-project/issues/57828 is not for a block-scope variable, but the case in the patch description is...

Thanks, Hubert. Yes, I found that the reported issue occurred for any case where thread safe guard variables are not required. I chose the block-scope variable example for the patch summary because I felt it better presented the issue.

Your question inspired me to do some additional testing though and I see both gcc and icc also exhibit the re-initialization behavior for that case, but not the case reported in https://github.com/llvm/llvm-project/issues/57828.

> we have to generate code that behaves correctly in the presence of exceptions

Thank you, John. Indeed, I didn't consider exception handling. And I locally confirmed that the changes break cases like https://godbolt.org/z/M8Penaxa1.

> providing access to an uninitialized variable is worse than recursively re-entering initialization; in either case, the program is incorrect, and the latter is much more likely to cause an immediate failure at runtime

My understanding is that, as long as the value of the uninitialized variable is not used, the program is not incorrect.

> the real fix requires a more elaborate code sequence to reliably generate a runtime failure
> Also, this needs Itanium discussion because it can affect interoperation.

I'll do some more analysis of how gcc and icc handle these scenarios and report back. If they appear to handle these cases right, perhaps Clang can be updated to match without the need for further ABI discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135919