Fix Issue 23814 - [Codegen] Calling member function of extern(C++) cl…

[naydef]

…ass with multiple inheritance doesn't preserve the EBX register in some cases

Remade.
Also not much idea of what I'm doing.
Not sure how to test this.

[dlang-bot]

Thanks for your pull request and interest in making D better, @naydef! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23814	normal	[Codegen] Calling member function of extern(C++) class with multiple inheritance doesn't preserve the EBX register in some cases

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#15063"

[RazvanN7]

As stated in the issue, I cannot reproduce this for neither 32 or 64 bit.

32 bit output:

00000000 <_ZN9MainClass5func4Ev>:
   0:   55                      push   ebp
   1:   8b ec                   mov    ebp,esp
   3:   83 ec 08                sub    esp,0x8
   6:   e8 00 00 00 00          call   b <_ZN9MainClass5func4Ev+0xb>
   b:   58                      pop    eax
   c:   05 02 00 00 00          add    eax,0x2
  11:   89 45 fc                mov    DWORD PTR [ebp-0x4],eax
  14:   8b 4d fc                mov    ecx,DWORD PTR [ebp-0x4]
  17:   8d 81 40 00 00 00       lea    eax,[ecx+0x40]
  1d:   c9                      leave  
  1e:   c3                      ret   

Does not use ebx.

64 bit output:

0000000000000000 <_ZN9MainClass5func4Ev>:
   0:   48 8d 05 00 00 00 00    lea    rax,[rip+0x0]        # 7 <_ZN9MainClass5func4Ev+0x7>
   7:   c3                      ret

Does not use rbx.

I don't think this issue is valid.

[naydef]

I also replied in the issue report:

I'm using DMD64 D Compiler v2.102.2 on Linux. Compiling with: "dmd app.d -m32"

I see you check func4 (judging by name of the disassembly _ZN9MainClass5func4Ev), while my example code calls func1.
Use the second example code. You can use a debugger and break where func1 is called in _Dmain, step into the function and you'll see the usage of EBX register in a function called _THUNK0, which at the end jumps to _ZN9MainClass5func1Ev.

This issue has actually caused me trouble when interoperating with C++ code on Linux, it doesn't seem to exist on Windows.

[Geod24]

1. This needs a test (which can then be used to repro the issue @RazvanN7 )
2. If the code is wrong, remove it, don't make the if always false.

[naydef]

I'm not sure how to test this I think I have a test for this, will add soon (code in issue report). It affected the C++ code which stored loop counter in this register in one place and some other used data in another case, causing crashing when the D proved object was used. DMD tends to keep stuff on the stack.

[naydef]

ping (Requested changes are applied)

[RazvanN7]

cc @WalterBright

[naydef]

@RazvanN7
Can this pull request be reviewed? It's blocking my project on Linux. I'll prefer It if the pull request get outright dismissed than rotting here for ages. If there's something I have to do, please tell me.

Requested changes are applied

[dkorpel]

Thanks for the ping, I've force pushed to restart the test suite and emailed Walter to request his review, since he wrote the backend.

[naydef]

@dkorpel
Can test suite be restarted (failure seems unrelated)?

[dkorpel]

I can rebase again, but @RazvanN7 has access to buildkite directly

[dkorpel]

Or merge despite the failure

[RazvanN7]

I restarted the buildkite testing pipeline. Adding the 72h no objection -> merge label.

[WalterBright]

"multiple inheritance doesn't preserve the EBX register in some cases"

Just removing all this code will break what it does. EBX register clobbering means it has to be saved/restored.

[naydef]

I don't have much clue how to do that (not familiar with compiler backends).
Edit: I don't see what useful thing this code does, loading GOT in EBX and then it goes unused. So yea, clueless here.

[WalterBright]

I wrote a more explanatory comment in the bu

[WalterBright]

... in the bug report

[adamdruppe]

Merged in OpenD.

[RazvanN7]

Since @WalterBright opposes the current implementation I see no point in keeping this PR open. If anyone wants to try and fix the issue there is a link from the bug report to the this PR.

[RazvanN7]

@naydef Thanks for your work and the time you put into this. I'm sorry that this is not the outcome you desired, however, maybe this will prompt @WalterBright to fix this issue.

[naydef]

Closing it was long overdue...