lebedev.ri marked an inline comment as done. lebedev.ri added inline comments.
================ Comment at: clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp:3 +// FIXME: this check appears to be miscompiled? +// XFAIL: * ---------------- tentzen wrote: > lebedev.ri wrote: > > efriedma wrote: > > > lebedev.ri wrote: > > > > tentzen wrote: > > > > > lebedev.ri wrote: > > > > > > This test broke once we always started adding (outermost) UB scope > > > > > > for nounwind functions. > > > > > > I don't quite get what is going wrong. It could be a bug in SEH > > > > > > handling. > > > > > > Can someone who has some idea about that code take a look and > > > > > > suggest a fix? > > > > > > @tentzen ? > > > > > By definition, non-unwind function I think is for Synchronous EH. So > > > > > this Sanitizer check should exclude Asynchronous EH functions, those > > > > > with option -fasync-exceptions. > > > > > > > > > I do not understand. > > > > If the function can unwind, then why is it marked `nounwind`? > > > > This kind of thing is exactly what i was afraid of with those SEH > > > > patches. > > > clang should not be marking functions "nounwind" in -fasync-exceptions > > > mode; if it is, I'd consider that a bug. (I assume someone just forgot > > > to add a check to some code that adds nounwind.) > > My thoughts precisely. @tentzen please fix :) > This is copied from LLVM reference manual: > > nounwind > This function attribute indicates that the function never raises an > exception. If the function does raise an exception, its runtime behavior is > undefined. However, functions marked nounwind may still trap or generate > asynchronous exceptions. Exception handling schemes that are recognized by > LLVM to handle asynchronous exceptions, such as SEH, will still provide their > implementation defined semantics. > > So I think nounwind only implies no synchronous/software unwind, not HW traps > etc. Ok, good point. But i'm still not quite sure why the test is getting miscompiled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137381/new/ https://reviews.llvm.org/D137381 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
