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.

Sign up for GitHub

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

Jump to bottom

Assertion triggered while compiling vibe-d with master #3501

Closed
Geod24 opened this issue Jul 10, 2020 · 3 comments
Closed

Assertion triggered while compiling vibe-d with master #3501

Geod24 opened this issue Jul 10, 2020 · 3 comments

Comments

@Geod24
Copy link
Contributor

Geod24 commented Jul 10, 2020 

$ dub test --compiler=$HOME/dlang/ldc-0d09a171/bin/ldc2
Generating test runner configuration 'vibe-d-test-vibe-core' for 'vibe-core' (library).
Running pre-generate commands for vibe-d:tls...
Performing "unittest" build using /Users/geod24/dlang/ldc-0d09a171/bin/ldc2 for x86_64.
taggedalgebraic 0.11.8: target for configuration "library" is up to date.
eventcore 0.8.48: target for configuration "kqueue" is up to date.
stdx-allocator 2.77.5: target for configuration "library" is up to date.
vibe-core 1.8.1: building configuration "kqueue"...
../../../.dub/packages/eventcore-0.8.48/eventcore/source/eventcore/drivers/posix/watchers.d(405,44): Deprecation: precision argument th.msg.length for format specification "%.*s" must be int, not ulong
Assertion failed: (global->isDeclaration() && "Global variable already defined"), function defineGlobal, file /Users/runner/work/1/s/gen/llvmhelpers.cpp, line 1806.
0  ldc2                     0x000000010fb3ca85 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  ldc2                     0x000000010fb3b836 llvm::sys::RunSignalHandlers() + 198
2  ldc2                     0x000000010fb3d09c SignalHandler(int) + 268
3  libsystem_platform.dylib 0x00007fff700275fd _sigtramp + 29
4  libsystem_platform.dylib 0x00000000000078c0 _sigtramp + 18446603338636985056
5  libsystem_c.dylib        0x00007fff6fefd808 abort + 120
6  libsystem_c.dylib        0x00007fff6fefcac6 err + 0
7  ldc2                     0x000000010fe473bc (anonymous namespace)::DeclareOrDefineVisitor::visit(TypeInfoStructDeclaration*) + 4636
8  ldc2                     0x000000010fdad654 CodegenVisitor::visit(TypeInfoDeclaration*) + 116
9  ldc2                     0x000000010fdde8bf codegenModule(IRState*, Module*) + 4255
10 ldc2                     0x000000010fe60831 ldc::CodeGenerator::emit(Module*) + 881
11 ldc2                     0x000000010fe7a0b6 codegenModules(Array<Module*>&) + 822
12 ldc2                     0x000000010fd647ed mars_mainBody(Param&, Array<char const*>&, Array<char const*>&) + 19245
/Users/geod24/dlang/ldc-0d09a171/bin/ldc2 failed with exit code -6.
dub test --compiler=$HOME/dlang/ldc-0d09a171/bin/ldc2  4.94s user 0.69s system 98% cpu 5.734 total
$ git show HEAD                                        :(
commit 72f5453ab19cb52238f3831a592448b473ce6643 (HEAD -> master, upstream/master)
Merge: c5f6927b 151bb858
Author: Leonid Kramer <leonid.kramer@bildhuus.com>
Date:   Mon Jul 6 15:14:33 2020 +0200

    Merge pull request #2457 from vibe-d/fix_web_struct_array

    Fix parsing of arrays of structs in the web interface generator.

Mac OSX 10.15, but I think you'll be able to reproduce on all platforms (our CI caught it).
@Geod24 Geod24 mentioned this issue Jul 10, 2020
Merged
@kinke
Copy link
Member

kinke commented Jul 10, 2020

Thx, very helpful to know now about it, without having to wait for feedback for the upcoming beta. #3486 added a check in this regard, while the previous code would unconditionally set the initializer for the global var, without checking whether it was already defined.

kinke added a commit to kinke/ldc that referenced this issue Jul 10, 2020
@kinke
Fix issue ldc-developers#3501: Don't re-define struct TypeInfos
8de592f 
Surfacing now as ldc-developers#3486 added a corresponding assertion.

I've tested this with an older vibe-d v0.8.6 lying around on my box.
The assertion was hit for the TypeInfo of a nested struct:

https://github.com/vibe-d/vibe.d/blob/3c7ae13989003b7ebc59de23c5b6a0fb0466f119/http/vibe/http/router.d#L570-L574

I wasn't able to quickly reduce it to a stand-alone testcase and am
reluctant to put any more effort into this.
kinke added a commit to kinke/ldc that referenced this issue Jul 10, 2020
@kinke
Fix issue ldc-developers#3501: Don't re-define struct TypeInfos
d904351 
Surfacing now as ldc-developers#3486 added a corresponding assertion.

I've tested this with an older vibe-d v0.8.6 lying around on my box.
The assertion was hit for the TypeInfo of a nested struct:

https://github.com/vibe-d/vibe.d/blob/3c7ae13989003b7ebc59de23c5b6a0fb0466f119/http/vibe/http/router.d#L570-L574

I wasn't able to quickly reduce it to a stand-alone testcase and am
reluctant to put any more effort into this.
kinke added a commit that referenced this issue Jul 10, 2020
@kinke
Fix issue #3501: Don't re-define struct TypeInfos (#3502)
e6a2dd8 
Surfacing now as #3486 added a corresponding assertion.

I've tested this with an older vibe-d v0.8.6 lying around on my box.
The assertion was hit for the TypeInfo of a nested struct:

https://github.com/vibe-d/vibe.d/blob/3c7ae13989003b7ebc59de23c5b6a0fb0466f119/http/vibe/http/router.d#L570-L574

I wasn't able to quickly reduce it to a stand-alone testcase and am
reluctant to put any more effort into this.
@kinke kinke closed this as completed Jul 10, 2020
@Geod24
Copy link
Contributor Author

Geod24 commented Jul 21, 2020

@kinke : I'm still seeing this on Windows with 4240644.
Screen Shot 2020-07-21 at 10 50 47

Here is the error in the build log.
The project uses Vibe.d (although optionally, and only for the JSON parser), so it might be triggered by Vibe still.

@Geod24 Geod24 mentioned this issue Jul 21, 2020
Closed
@kinke
Copy link
Member

kinke commented Jul 21, 2020

You're seeing the same assertion, but the cause is entirely unrelated, Windows-specific and not a regression. Thx for letting us know & using the assertions-enabled master build.

kinke added a commit to kinke/ldc that referenced this issue Jul 22, 2020
@kinke
Fix colliding WinEH TypeDescriptors for exceptions with the same Type…
0c1ec37 
…Info_Class name

As reported in ldc-developers#3501 (comment).

Previously, LDC would simply override an existing TypeDescriptor global
with an equivalent definition if assertions were disabled.

Use different equivalent TypeDescriptors in this exotic case now, by
using proper mangling. [The linker can presumably still fold duplicates
via ICF.]

Ideally, these descriptors would be truly unique, but WinEH depends on
string comparisons to check for matching catch clauses, and currently
depends on TypeInfo_Class.name to generate these strings at runtime when
throwing an exception, see
https://github.com/ldc-developers/druntime/blob/19731a92a97dbe4d7f7a4e15ceaff8444a1f879a/src/ldc/eh_msvc.d#L192-L211.
So if those names were fully qualified (`cd->toPrettyChars(*true*)`
during ClassInfo generation), we'd be fine, but that'd obviously be a
breaking change and diverge from upstream.
@kinke kinke mentioned this issue Jul 22, 2020
Closed
@kinke kinke mentioned this issue Nov 13, 2020
Merged
kinke added a commit to kinke/ldc that referenced this issue Nov 13, 2020
@kinke
Fix colliding WinEH TypeDescriptors for exceptions with the same Type…
b7c6a2f 
…Info_Class name

As reported in ldc-developers#3501 (comment).

Previously, LDC would simply override an existing TypeDescriptor global
with an equivalent definition if assertions were disabled.

Use different equivalent TypeDescriptors in this exotic case now, by
using proper mangling. [The linker can presumably still fold duplicates
via ICF.]

Ideally, these descriptors would be truly unique, but WinEH depends on
string comparisons to check for matching catch clauses, and currently
depends on TypeInfo_Class.name to generate these strings at runtime when
throwing an exception, see
https://github.com/ldc-developers/druntime/blob/19731a92a97dbe4d7f7a4e15ceaff8444a1f879a/src/ldc/eh_msvc.d#L192-L211.
So if those names were fully qualified (`cd->toPrettyChars(*true*)`
during ClassInfo generation), we'd be fine, but that'd obviously be a
breaking change and diverge from upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kinke @Geod24