Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix 21484: Infinite loop in core.memory : GC.{get,set,clr}Attr(const scope void*...) #3314

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Dec 16, 2020

Those functions never worked, and would cause an infinite recursion.
In a non-optimized build that would exhaust the stack and trigger a SEGV,
however in the optimized build we distribute, it results in an infinite
loop as tail call optimization is performed.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Auto-close Bugzilla Severity Description
21484 major Infinite recursion in core.memory : GC.{get,set,clr}Attr(const scope void*...)

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 "stable + druntime#3314"

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Dec 16, 2020
@MoonlightSentinel
Copy link
Contributor

Tests?

@Geod24 Geod24 force-pushed the avoid-infinite-recursion branch from 81c2702 to f81ad89 Compare December 16, 2020 18:02
@PetarKirov
Copy link
Member

We were talking with @Geod24 on Slack PM so that's why I was quite fast with the approval, though on second thoughts I agree with @MoonlightSentinel that it would be good to cover this with a test. Otherwise the test case for the compiler bug that @Geod24 showed me was:

extern (C) uint gc_setAttr( void* p, uint a ) pure nothrow;
​
struct Baguette
{
    static uint setAttr( const scope void* p, uint a ) nothrow
    {
        return setAttr(cast()p, a);
    }
​
    static uint setAttr(void* p, uint a) pure nothrow
    {
        return gc_setAttr( p, a );
    }
}
​
inout(void)* bug (inout(void)* ptr)
{
    Baguette.setAttr(ptr, 42);
    return ptr;
}
​
void main ()
{
    auto ptr = new int*;
    bug(ptr);
}

@Geod24 Geod24 force-pushed the avoid-infinite-recursion branch from f81ad89 to 1725690 Compare December 16, 2020 18:19
@Geod24
Copy link
Member Author

Geod24 commented Dec 16, 2020

Usually I would say "it doesn't make sense to test for infinite loop here", but since the function were not covered at all, I added a small test.

…scope void*...)

Those functions never worked, and would cause an infinite recursion.
In a non-optimized build that would exhaust the stack and trigger a SEGV,
however in the optimized build we distribute, it results in an infinite
loop as tail call optimization is performed.
@Geod24 Geod24 force-pushed the avoid-infinite-recursion branch from aa599f7 to afec613 Compare December 16, 2020 18:38
@MoonlightSentinel
Copy link
Contributor

Thanks

@Geod24
Copy link
Member Author

Geod24 commented Dec 17, 2020

So, anything holding this back ?

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

Sgtm

@dlang-bot dlang-bot merged commit 027fca5 into dlang:stable Dec 17, 2020
@Geod24 Geod24 deleted the avoid-infinite-recursion branch December 17, 2020 08:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants