Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

fix nothrow issues#974

Merged
9rnsr merged 1 commit intodlang:masterfrom
MartinNowak:nothrowIssues
Oct 2, 2014
Merged

fix nothrow issues#974
9rnsr merged 1 commit intodlang:masterfrom
MartinNowak:nothrowIssues

Conversation

@MartinNowak
Copy link
Member

  • revealed by enabling nothrow checks in functions
    with inline asm

@WalterBright
Copy link
Member

Add a blank line before the nothrow: line. (github is insisting I'm not logged in, sigh, but it lets me post this!)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same fix would be necessary in rt/arraybyte.d, rt/arraydouble.d, rt/arrayfloat.d, rt/arrayreal.d, rt/arrayshort.d.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, I only got an error for arrayint. Will recheck.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you just found a copy&paste bug in arrayint.d, it's the only one that performs a runtime check for sse2 support on X86_64.
#976

@9rnsr
Copy link
Contributor

9rnsr commented Sep 30, 2014

In windows platforms, enumProcessThreads function in core/sys/windows/threadaux.d is not nothrow. Inside dll_fixTLS function in core/sys/windows/dll.d, its call silently violates the nothrow attribute.

To fix the issue, we should assume the enumProcessThreads function call as nothrow, like follows:

    bool dll_fixTLS( HINSTANCE hInstance, void* tlsstart, void* tlsend, void* tls_callbacks_a, int* tlsindex ) nothrow
    {
[snip]
        scope(failure) assert(0);    // Add this line to tell to the compiler that the exception code path will never be reached.

        if( !enumProcessThreads(
            function (uint id, void* context) nothrow {
                dll_aux.LdrpTlsListEntry* entry = cast(dll_aux.LdrpTlsListEntry*) context;
                return dll_aux.addTlsData( getTEB( id ), entry.tlsstart, entry.tlsend, entry.tlsindex );
            }, entry ) )
            return false;

[snip]
    }

@MartinNowak MartinNowak force-pushed the nothrowIssues branch 2 times, most recently from 4e75e10 to 2a88d52 Compare September 30, 2014 22:40
@MartinNowak
Copy link
Member Author

To fix the issue, we should assume the enumProcessThreads function call as nothrow, like follows:

Just tried to fix it properly and got stuck in core.thread with the dreaded _d_monitorenter isn't nothrow issue, so I went with your scope(failure) assert(0);.

@braddr
Copy link
Member

braddr commented Oct 1, 2014

Martin, file a bug to track fixing it more correctly and note the bug number in the source.

@MartinNowak
Copy link
Member Author

Martin, file a bug to track fixing it more correctly and note the bug number in the source.

Issue 13561

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the bugzilla number 13561 in the comment.

@9rnsr
Copy link
Contributor

9rnsr commented Oct 1, 2014

Others looks good to me. After the comment fix, I'll toggle on auto-merging.

- revealed by enabling nothrow checks in functions
  with inline asm
@MartinNowak
Copy link
Member Author

done

@9rnsr
Copy link
Contributor

9rnsr commented Oct 2, 2014

Auto-merge toggled on

9rnsr added a commit that referenced this pull request Oct 2, 2014
@9rnsr 9rnsr merged commit b8a107a into dlang:master Oct 2, 2014
@MartinNowak MartinNowak deleted the nothrowIssues branch August 30, 2022 06:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants