Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows SDK 22000+ missing __analysis_noreturn SAL #791

Closed
riverar opened this issue Feb 1, 2022 · 4 comments · Fixed by #793
Closed

Windows SDK 22000+ missing __analysis_noreturn SAL #791

riverar opened this issue Feb 1, 2022 · 4 comments · Fixed by #793
Assignees

Comments

@riverar
Copy link
Collaborator

riverar commented Feb 1, 2022

I suspect this is what led to a few APIs slipping under the radar when implementing noreturn support in metadata.

19042 winnt.h

NTSYSAPI
__analysis_noreturn
VOID
NTAPI
RtlRaiseException(
    _In_ PEXCEPTION_RECORD ExceptionRecord
    );

22000.0 and 22000.120 winnt.h

NTSYSAPI
VOID
NTAPI
RtlRaiseException(
    _In_ PEXCEPTION_RECORD ExceptionRecord
    );

@mikebattista Can we get an internal bug tracking this? Feel free to do what you wish with this issue afterwards. Thanks in advance!

@sotteson1
Copy link
Contributor

sotteson1 commented Feb 1, 2022

I looked up the change, and it was changed on purpose. The bug title was:

[EH] RaiseException can return but is decorated with __analysis_noreturn

The commit said:
Yes, it is perfectly legitimate for (Rtl)RaiseException to return.

So I guess it's possible for this function to return, although I don't know the specifics. I wonder if we should remove the attribute from the metadata to be consistent.

@kennykerr
Copy link
Contributor

That's surprising. Adding @lzybkr who was also expecting it not to return. And @JamesMcNellis who knows all things about exceptions.

@KalleOlaviNiemitalo
Copy link

I suppose RaiseException will return normally if an exception filter returns EXCEPTION_CONTINUE_EXECUTION and dwExceptionFlags does not include EXCEPTION_NONCONTINUABLE.

@lzybkr
Copy link
Member

lzybkr commented Feb 1, 2022

Under normal conditions, RtlRaiseException finishes by setting the thread (register) context - not by returning. If something goes wrong in setting the context, RtlRaiseException raises a different non-continuable exception, so - in theory it shouldn't reach the return instruction, but maybe it's not impossible?

That said, @KalleOlaviNiemitalo points out a scenario where that may look like an ordinary return. To make this concrete - this following example prints Hello world.

#include <windows.h>
#include <stdio.h>

int main() {
    __try {
        RaiseException(0x12345678, 0, 0, 0);
        printf("Hello ");
    } __except(EXCEPTION_CONTINUE_EXECUTION) {
    }

    printf("world\n");
}

Based on this example, I'd agree that the prototype should not include noreturn, even knowing that RtlRaiseException is not actually returning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants