Fix assert; arguments were in reverse order

[Connor-GH]

So sometime between glibc 2.35 and glibc 2.38, assert for betterC stopped working. After a wild goosechase, it was determined that not only was the wrong function being called ("__assert" instead of "__assert_fail"), but the arguments were being passed to el_bin backwards! The fix for this is obviously to fix the order of the arguments, and this is reflected in the generated code. Musl was untested for this, but the fix should not affect otherwise working musl systems due to version(CRuntime_Musl).

[dlang-bot]

Thanks for your pull request and interest in making D better, @Connor-GH! 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
✓	24558	normal	C asserts segfault on Glibc

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

[Connor-GH]

Whoops, I accidently closed this.

[schveiguy]

This needs a test. And I suspect it might be dependent on the glibc version.

In discord chat, we found that on my glibc 2.35 version, it worked, but on @Connor-GH 's glibc 2.38 version it had a segfault.

If this is the case, it's going to be a tricky thing to get right.

Edit: Still needs a test, but I'm realizing this is a different function being called! It's __assert_fail and not __assert, which we agreed is the correct call.

[ibuclaw]

This needs a test. And I suspect it might be dependent on the glibc version.

Assuming that there aren't any per-arch overrides, implementation hasn't changed since 2002.

https://github.com/bminor/glibc/blob/f83e461f1014598a5cb4c89407ce303b9f0bd8ac/assert/__assert.c#L23

In discord chat, we found that on my glibc 2.35 version, it worked, but on @Connor-GH 's glibc 2.38 version it had a segfault.

If this is the case, it's going to be a tricky thing to get right.

Edit: Still needs a test, but I'm realizing this is a different function being called! It's __assert_fail and not __assert, which we agreed is the correct call.

Arguably both are wrong because this is relying on glibc internals that are not intended to be called directly like this.

[Connor-GH]

This needs a test. And I suspect it might be dependent on the glibc version.

Assuming that there aren't any per-arch overrides, implementation hasn't changed since 2002.

https://github.com/bminor/glibc/blob/f83e461f1014598a5cb4c89407ce303b9f0bd8ac/assert/__assert.c#L23

In discord chat, we found that on my glibc 2.35 version, it worked, but on @Connor-GH 's glibc 2.38 version it had a segfault. If this is the case, it's going to be a tricky thing to get right. Edit: Still needs a test, but I'm realizing this is a different function being called! It's __assert_fail and not __assert, which we agreed is the correct call.

Arguably both are wrong because this is relying on glibc internals that are not intended to be called directly like this.

I purposefully didn't touch the musl version as I did not test it. I would rather only touch code that I can test.

[Connor-GH]

I'd also like to mention that the CI is pretty outdated. None of the CI machines run glibc 2.38.

Edit: even godbolt.org runs glibc 2.35.

[Connor-GH]

Tested on void linux musl, the assert was backwards for musl too and that I was mistaken. The implementation correct for OSX as-is.

[Connor-GH]

(This is after the latest patch which also fixes the musl version. Before it would also segfault)

[schveiguy]

Arguably both are wrong because this is relying on glibc internals that are not intended to be called directly like this.

Agreed. It is something I was thinking importC might actually be better at solving. But of course, this then requires a C compiler for building anything.

[Connor-GH]

Simply waiting on approval now for CI.

[thewilsonator]

Is there a bugzilla issue number for this? if not please create one, or add a changelog entry for this. This otherwise looks good, the one failure I think will go away if you rebase this PR

[Connor-GH]

https://issues.dlang.org/show_bug.cgi?id=24558