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

Conversation

@wilzbach
Copy link
Contributor

Start to dig into the CircleCi failures seen here:

Reproducible locally with:

TEST_COVERAGE=1 make -f posix.mak unittest-debug

Failure:

****** FAIL debug64 core.time
core.exception.AssertError@src/core/time.d(527): unittest failure
----------------

@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 18, 2018

Thanks for your pull request, @wilzbach!

Bugzilla references

Auto-close Bugzilla Description
18296 [Reg2.078.1] invalid code with coverage and copy construction

src/core/time.d Outdated
foreach(T; _TypeTuple!(Duration, const Duration, immutable Duration))
{
foreach(U; _TypeTuple!(Duration, const Duration, immutable Duration))
foreach(U; _TypeTuple!(Duration, /*const Duration,*/ immutable Duration))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why this doesn't work anymore when compiled with -cov :/

Copy link
Contributor Author

@wilzbach wilzbach Jan 18, 2018

Choose a reason for hiding this comment

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

This is the first build that failed: https://circleci.com/gh/dlang/druntime/1607

So probably this is due to:

b38f88f

(and something else as #2032 did pass merge being merged - interestingly #2044 did pass too)

@schveiguy
Copy link
Member

This failure is bizarre. Can you print out what the value is if they are not equal?

@schveiguy
Copy link
Member

I wonder if it has something to do with the T t = 42? Typically, you would use 42.hnsecs, but it's calling the ctor directly since we are in the same file.

@wilzbach
Copy link
Contributor Author

Can you print out what the value is if they are not equal?

0 != 42 - we really need to get dmd to rewrite assert to be more user-friendly.

@jmdavis
Copy link
Member

jmdavis commented Jan 24, 2018

I see no reason why this should be failing, especially when Duration and immutable Duration pass. And if it works without -cov but doesn't work with it, then that reeks of a compiler bug.

0 != 42 - we really need to get dmd to rewrite assert to be more user-friendly.

It's been discussed before. I would be nice, but it's too expensive.

@PetarKirov
Copy link
Member

It's been discussed before. I would be nice, but it's too expensive.

It can be opt-in - problem solved!

@rainers
Copy link
Member

rainers commented Jan 24, 2018

I investigated this a bit: this code fails in the second iteration:

            foreach(U; _TypeTuple!(Duration, const Duration))
            {
                const Duration t = 42;
                U u = t;
                assert(t == u);
            }

but works if either -cov or -fPIC is removed.

This disassembly shows that an instruction is omitted:

  ...
  90:	48 8b 0d 00 00 00 00 	mov    0x0(%rip),%rcx        # 97 <_Dmain+0x97>
			93: R_X86_64_GOTPCREL	_D4core4time8Duration6__initZ-0x4
  97:	48 8b 19             	mov    (%rcx),%rbx
  9a:	48 89 5d f0          	mov    %rbx,-0x10(%rbp)
  9e:	be 2a 00 00 00       	mov    $0x2a,%esi
  a3:	48 8d 7d f0          	lea    -0x10(%rbp),%rdi
  a7:	e8 00 00 00 00       	callq  ac <_Dmain+0xac>
			a8: R_X86_64_PLT32	_D4core4time8Duration6__ctorMFNaNbNcNiNflZSQBpQBnQBl-0x4
  ac:	48 8b 05 00 00 00 00 	mov    0x0(%rip),%rax        # b3 <_Dmain+0xb3>  //// coverage code
			af: R_X86_64_GOTPCREL	_D1x10__coverageZ-0x4
  b3:	ff 40 24             	incl   0x24(%rax)                                //// coverage code
  >>>  48 8b 4d f0          	mov    -0x10(%rbp),%rbx          <<< this instruction missing
  b6:	48 89 5d f8          	mov    %rbx,-0x8(%rbp)
  ba:	48 8b 0d 00 00 00 00 	mov    0x0(%rip),%rcx        # c1 <_Dmain+0xc1>  //// coverage code
			bd: R_X86_64_GOTPCREL	_D1x10__coverageZ-0x4
  c1:	ff 41 28             	incl   0x28(%rcx)                                //// coverage code
  c4:	48 8d 75 f0          	lea    -0x10(%rbp),%rsi
  c8:	48 8d 7d f8          	lea    -0x8(%rbp),%rdi
  cc:	b9 08 00 00 00       	mov    $0x8,%ecx
  d1:	33 c0                	xor    %eax,%eax
  d3:	f3 a6                	repz cmpsb %es:(%rdi),%ds:(%rsi)
   ...

@MartinNowak
Copy link
Member

Well who know what that backend does, some stale com-subs or whatever, the IR gen for coverage looks ok though.
We recently changed how the coverage symbol is emitted, so that seems to have triggered a dormant backend bug.
18296 – [Reg2.078.1] invalid code with coverage and copy construction

@rainers
Copy link
Member

rainers commented Jan 24, 2018

I suspect that the switch from static to global symbol has uncovered the bug, because it now uses an additional indirection.

@wilzbach
Copy link
Contributor Author

@MartinNowak thanks for adding your more specific workaround and I would love to approve it, but it seems like GitHub doesn't allow me to approve my own PRs :/
(auto-merge requires at least one approval)

@wilzbach
Copy link
Contributor Author

Urgh now Jenkins is failing, but the temporary fix is on the merge queue: dlang/ci#132

So I'm force-merging this PR here.

@wilzbach wilzbach merged commit fca2892 into dlang:master Jan 24, 2018
@wilzbach wilzbach deleted the fix-circleci branch January 24, 2018 23:57
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.

7 participants