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

dmd-cxx: Fix broken 'stat' definitions for ARM, AArch64 & more #2898

Merged
merged 5 commits into from
Jan 9, 2020

Conversation

Geod24
Copy link
Member

@Geod24 Geod24 commented Jan 7, 2020

I'm mostly interested in fixing ARMv7, AArch64 and SystemZ (s390x) for Alpine Linux, but added porting for a few other archs while I was at it.
Will PR against stable as well so we get it fixed in v2.090.1 and hopefully LDC 1.20.0

CC @Cogitri

@Geod24 Geod24 requested a review from ibuclaw January 7, 2020 01:28
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "dmd-cxx + druntime#2898"

@Geod24
Copy link
Member Author

Geod24 commented Jan 7, 2020

@Cogitri : I built GDC with those patches and provided the packages here.

AArch64 "Hello World" works (testing DUB / LDC ATM), but ARMv7 is still broken. I'll try to find a board to test natively instead of using qemu though, as debugging is impossible.

@Cogitri
Copy link
Contributor

Cogitri commented Jan 7, 2020

Great, I can test on the aarch64 container of mine.

@Cogitri
Copy link
Contributor

Cogitri commented Jan 7, 2020

Works for me, seems like isDir() works now (or at least rmdirRecurse works for my tests on aarch64) :)

@Geod24
Copy link
Member Author

Geod24 commented Jan 7, 2020

Tried to use Dub, it went very wrong. Found out that:

import std.process;
import std.exception;
import std.format;

void main ()
{
        auto result = executeShell("ls -l -a");
        enforce(result.status == 0, format("Failed: %d: %s", result.status, result.output));
}

SEGV. I think I'll just have to go over all version (CRuntime_Musl)...

@Geod24
Copy link
Member Author

Geod24 commented Jan 7, 2020

Found a few more issues.

Rebuilding the AArch64 now (based on https://gitlab.alpinelinux.org/Geod24/aports/commit/68059efc1f9140c0e810385a174544addacb60ed).
I think this can already go in, if someone is willing to review.
I used https://github.com/bminor/musl/tree/v1.1.24 to double-check the headers.

@Cogitri
Copy link
Contributor

Cogitri commented Jan 7, 2020

I can test again on my Aarch64 container tomorrow.

@Geod24
Copy link
Member Author

Geod24 commented Jan 8, 2020

@Cogitri : Could you test the following program:

import std.process;
import std.exception;
import std.format;
import std.stdio;

void main ()
{
        auto result = execute(["ls"]);
        if (result.status != 0)
            writeln(format("Failed: %d: %s", result.status, result.output));
}

On your machine and tell me where the SEGV happens ?
It is in the forked process, but I have no way of debugging with qemu. valgrind or GDB might help, they both trace forks.

Newly built packages: http://pkg.mimiks.net/packages/main/
Or on the MR (but I keep on updating it): https://gitlab.alpinelinux.org/alpine/aports/merge_requests/2841

@Cogitri
Copy link
Contributor

Cogitri commented Jan 8, 2020

Seems to work fine on my machine:

cogitri-edge-aarch64:~$ gdb main
GNU gdb (GDB) 8.3.1
...

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from main...
(gdb) run
Starting program: /home/rasmus/main 
[Detaching after fork from child process 11475]
[Inferior 1 (process 11472) exited normally]

If I modify the program to

import std.process;
import std.exception;
import std.format;
import std.stdio;

void main ()
{
        auto result = execute(["ls"]);
        if (result.status != 0)
            writeln(format("Failed: %d: %s", result.status, result.output));
        else
            writeln(result.output);
}

It prints

main
main.d

, as expected.

cogitri-edge-aarch64:~$ cat /etc/os-release 
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.12_alpha20191219
PRETTY_NAME="Alpine Linux edge"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"
cogitri-edge-aarch64:~$ uname -a
Linux cogitri-edge-aarch64 4.19.79-0-vanilla #1-Alpine SMP Thu Oct 17 14:18:12 UTC 2019 aarch64 GNU/Linux

@Geod24
Copy link
Member Author

Geod24 commented Jan 9, 2020

It's great that it works, but it also looks like I've reached the limits of qemu, which means I can't really test myself anymore. I'll see if I can abuse the Alpine machines then :)

@Geod24
Copy link
Member Author

Geod24 commented Jan 9, 2020

@thewilsonator : The autotester doesn't pass on dmd-cxx, that's why it's not required. Merging manually then.

@Geod24 Geod24 merged commit 7915b6a into dlang:dmd-cxx Jan 9, 2020
@Geod24 Geod24 deleted the musl-stat-dmdcxx branch January 9, 2020 04:44
@Cogitri
Copy link
Contributor

Cogitri commented Jan 9, 2020

Thanks for your work! :)

kraj pushed a commit to kraj/gcc that referenced this pull request Mar 14, 2020
Includes port fixes for Musl on ARM, AArch64, and SystemZ targets.

Reviewed-on: dlang/druntime#2751
             dlang/druntime#2843
             dlang/druntime#2844
             dlang/druntime#2898
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.

4 participants