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

Fix Issue 17489 - ICE in ddmd/argtypes.d #6894

Closed
wants to merge 1 commit into from

Conversation

wilzbach
Copy link
Member

Funny thing - the ICE in Vibe.d 0.7.31-rc.2 was fixed #6875 and can't be observed anymore, but in the dustmited example (it just took quite a bit for dustmite too reduce it).

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
17489 ICE in ddmd/argtypes.d

@@ -0,0 +1,12 @@
// REQUIRED_ARGS: -Ifail_compilation/extra-files
Copy link
Member Author

Choose a reason for hiding this comment

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

If EXTRA_SOURCES is used, this ICE doesn't appear.

@WalterBright
Copy link
Member

As in the other PRs on this issue, the error should be checked for further up the call stack.

@WalterBright
Copy link
Member

WalterBright commented Jun 12, 2017

I can't reproduce this. I suggest looking at in finalizeSize() at where toArgtypes() is called. I suggest looking at https://github.com/dlang/dmd/pull/6875/files in particular why that fix is not working for you.

@wilzbach
Copy link
Member Author

I can't reproduce this.

Hmm is this maybe dependent on the platform?
At least CodeCov also shows that this appears

image

I suggest looking at in finalizeSize() at where toArgtypes() is called. I suggest looking at https://github.com/dlang/dmd/pull/6875/files in particular why that fix is not working for you.

It seems like there isn't an error in dstruct.d yet:

fail_compilation/extra-files/fail17489_file.d(2): Error: basic type expected, not )
fail_compilation/extra-files/fail17489_file.d(5): Error: enum fail17489_file.DirectoryChangeType enum DirectoryChangeType must have at least one member
-StructDeclaration::finalizeSize() Interface, fields.dim = 3, structsize = 32
-StructDeclaration::finalizeSize() OffsetTypeInfo, fields.dim = 2, structsize = 16
-StructDeclaration::finalizeSize() ModuleInfo, fields.dim = 2, structsize = 8
-StructDeclaration::finalizeSize() AARange, fields.dim = 2, structsize = 16
-StructDeclaration::finalizeSize() Path, fields.dim = 1, structsize = 1
-StructDeclaration::finalizeSize() DirectoryChange, fields.dim = 2, structsize = 8
core.exception.AssertError@ddmd/argtypes.d(203): Assertion failure
----------------
??:? _d_assertp [0x735ddc]
ddmd/argtypes.d:203 _ZN10toArgTypes10ToArgTypes12argtypemergeEP4TypeS2_j [0x500148]
ddmd/argtypes.d:385 _ZN10toArgTypes10ToArgTypes5visitEP10TypeStruct [0x5008c3]
ddmd/mtype.d:8787 _ZN10TypeStruct6acceptEP7Visitor [0x60b979]
ddmd/argtypes.d:462 _Z10toArgTypesP4Type [0x4ffcb9]
ddmd/dstruct.d:605 _ZN17StructDeclaration12finalizeSizeEv [0x579381]
ddmd/aggregate.d:338 _ZN20AggregateDeclaration13determineSizeE3Loc [0x4fdfcf]
ddmd/aggregate.d:152 _ZN20AggregateDeclaration9semantic2EP5Scope [0x4fd8f5]
ddmd/dmodule.d:1101 _ZN6Module9semantic2EP5Scope [0x56db88]
ddmd/dimport.d:412 _ZN6Import9semantic2EP5Scope [0x5526da]
ddmd/aggregate.d:158 _ZN20AggregateDeclaration9semantic2EP5Scope [0x4fd93f]
ddmd/dmodule.d:1101 _ZN6Module9semantic2EP5Scope [0x56db88]
ddmd/mars.d:1480 int ddmd.mars.tryMain(ulong, const(char)**) [0x5f68dc]
ddmd/mars.d:1730 _Dmain [0x5f7992]

I tried moving it a bit higher, but I am not sure how to get it into dstructs.d as there isn't a globally logged error (yet)?

@@ -341,6 +341,8 @@ extern (C++) TypeTuple toArgTypes(Type t)
{
VarDeclaration f = t.sym.fields[i];
//printf(" [%d] %s f.type = %s\n", cast(int)i, f.toChars(), f.type.toChars());
if (f.type.ty == Terror)
Copy link
Member

Choose a reason for hiding this comment

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

Indeed, here is not the right place to check errors in one of the fields, should happen when semantic for the fields runs in finalizeSize or so.

@WalterBright
Copy link
Member

ddmd/argtypes.d:462 _Z10toArgTypesP4Type [0x4ffcb9]
ddmd/dstruct.d:605 _ZN17StructDeclaration12finalizeSizeEv [0x579381]

It is finalizeSize() calling toArgTypes(). Check for the error in finalizeSize().

@MartinNowak
Copy link
Member

The root of the problem is that immutable() m_nodes; is a parse error which bumps global.errors and returns a Type.terror already during parseType.
This error is later not detected by StructDeclaration::semantic for Path when it runs semantic for all of it's members, b/c it only compares global.errors at the begin and end of StructDeclaration::semantic.
Since the error type isn't propagated from the field m_nodes to the struct Path, we make it all the way to finalizeSize of DirectoryChange, where the assertion is triggered when setting the offset for field path.
Doesn't happen when the module is part of the compilation, since we eagerly abort on any parse errors.

@MartinNowak
Copy link
Member

Closed in favor of #6902.

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