dmd icon indicating copy to clipboard operation
dmd copied to clipboard

More robust invariant check, fix stack trace reentrancy

Open schveiguy opened this issue 2 years ago • 9 comments

Attempt to solve some of the bizarre situations we can get into when allocating a stack trace actually throws.

Also fix an issue where an object invariant will segfault if called on a destroyed object. e.g.:

    Object o = new Object;
    destroy(o);
    assert(o); // would segfault, with this change it will now throw an assert error

Please pay close attention to the notes and way this is written. I tried to think of all cases, but possibly I missed some! This is a very tricky piece of code to write, because we are writing the framework upon which D exceptions are built, in D, which can end up using exceptions!

ping @ibuclaw would appreciate a review.

schveiguy avatar Dec 30 '22 01:12 schveiguy

Thanks for your pull request, @schveiguy!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14760"

dlang-bot avatar Dec 30 '22 01:12 dlang-bot

Ignoring whitespace when reviewing is recommended.

schveiguy avatar Dec 30 '22 01:12 schveiguy

This would definitively need a test case.

deadalnix avatar Dec 30 '22 02:12 deadalnix

I'll have to turn that example above into a test case.

schveiguy avatar Dec 30 '22 02:12 schveiguy

@deadalnix done

schveiguy avatar Dec 30 '22 02:12 schveiguy

I can't for some reason log in to restart the azure pipeline test that failed. Having a hard time finding why it failed in the first place, hard to read all these stupid logs.

schveiguy avatar Dec 30 '22 03:12 schveiguy

There is something I don't get. How is the test causing the track trace generator to throw while generating the stack trace?

deadalnix avatar Dec 30 '22 13:12 deadalnix

There is something I don't get. How is the test causing the track trace generator to throw while generating the stack trace?

The implementation of DefaultTraceInfo fetches the current stack top and bottom

807│   auto  stackTop    = getBasePtr();
808├─> auto  stackBottom = cast(void**) thread_stackBottom();

The stack bottom function has a contract, contracts call the invariant.

1309│ extern (C) void* thread_stackBottom() nothrow @nogc
1310├>in (ThreadBase.getThis())
1311│ {
1312│     return ThreadBase.getThis().topContext().bstack;
1313│ }

You need to link in the /generated/debug/ version of druntime in order to trigger this (also, force finalize the thread reference).

ibuclaw avatar Dec 30 '22 14:12 ibuclaw

some of the bizarre situations we can get into when allocating a stack trace

I forgot what prompted this PR, it needs a bug report at least with the destroyed object bug, but I also think some other problem was present that isn't clear here.

I think I also couldn't view the azure pipelines.

schveiguy avatar Apr 06 '23 19:04 schveiguy