Skip to content

Musl Clean#4616

Merged
kinke merged 1 commit intoldc-developers:masterfrom
kassane:musl_fix
Apr 19, 2024
Merged

Musl Clean#4616
kinke merged 1 commit intoldc-developers:masterfrom
kassane:musl_fix

Conversation

@kassane
Copy link
Contributor

@kassane kassane commented Apr 7, 2024

$ /opt/bin/ldmd2 --version | head -n 8
LDC - the LLVM D compiler (1.38.0-git-bc00d4b):
  based on DMD v2.108.0 and LLVM 17.0.5
  built with LDC - the LLVM D compiler (1.33.0)
  Default target: x86_64-alpine-linux-musl
  Host CPU: znver3
  http://dlang.org - http://wiki.dlang.org/LDC

$ /opt/bin/ldmd2 -run hellod-sample.d 
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/lib/libdruntime-ldc.a(fiber.o): in function `_D4core6thread5fiber5Fiber10allocStackMFNbmmZv':
fiber.d:(.text._D4core6thread5fiber5Fiber10allocStackMFNbmmZv+0x7d): undefined reference to `mmap64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/lib/libdruntime-ldc.a(os.o): in function `_D4core8internal2gc2os10os_mem_mapFNbNimbZPv':
os.d:(.text._D4core8internal2gc2os10os_mem_mapFNbNimbZPv+0x21): undefined reference to `mmap64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/lib/libdruntime-ldc.a(io.o): in function `_D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z7ElfFile4openFNbNiPxaJSQDzQDxQDrQDq__TQDqTQDnTQCuVhi2ZQCcZb':
io.d:(.text._D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z7ElfFile4openFNbNiPxaJSQDzQDxQDrQDq__TQDqTQDnTQCuVhi2ZQCcZb+0xa6): undefined reference to `mmap64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/lib/libdruntime-ldc.a(io.o): in function `_D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z13NamedSections7opApplyMFNbNiMDFNbNimAxaSQErQEpQEjQEi__TQEiTQEfTQDmVhi2Z16ElfSectionHeaderZiZi':
io.d:(.text._D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z13NamedSections7opApplyMFNbNiMDFNbNimAxaSQErQEpQEjQEi__TQEiTQEfTQDmVhi2Z16ElfSectionHeaderZiZi+0xaf): undefined reference to `mmap64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: io.d:(.text._D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z13NamedSections7opApplyMFNbNiMDFNbNimAxaSQErQEpQEjQEi__TQEiTQEfTQDmVhi2Z16ElfSectionHeaderZiZi+0x16a): undefined reference to `mmap64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /opt/lib/libdruntime-ldc.a(io.o):io.d:(.text._D4core8internal3elf2io__T5ElfIOTSQBg3sysQz10Elf64_EhdrTSQCdQxQBu10Elf64_ShdrVhi2Z13NamedSections7opApplyMFNbNiMDFNbNimAxaSQErQEpQEjQEi__TQEiTQEfTQDmVhi2Z16ElfSectionHeaderZiZi+0x2a5): more undefined references to `mmap64' follow
collect2: error: ld returned 1 exit status
Error: /usr/bin/cc failed with status: 1

References

@kassane
Copy link
Contributor Author

kassane commented Apr 8, 2024

@JohanEngelen , ditto here: 1162004

@kassane
Copy link
Contributor Author

kassane commented Apr 8, 2024

new error on alpine-ARM64(amd64), after simplify commit. 1162004
dlang/dmd#16361 (comment)

Error on previous commit: LDC version identifier: 1.38.0-git-a3de5db not 1162004 - need rerun
https://github.com/kassane/alpine-ldc2-docker/actions/runs/8602260162/job/23572771904

@kassane
Copy link
Contributor Author

kassane commented Apr 8, 2024

Result:

$ ./build/bin/ldc2 --version | head -n 5
LDC - the LLVM D compiler (1.38.0-git-f7b2474):
  based on DMD v2.108.0 and LLVM 17.0.5
  built with LDC - the LLVM D compiler (1.38.0-git-f7b2474)
  Default target: x86_64-alpine-linux-musl
  Host CPU: znver3

@JohanEngelen
Copy link
Member

To save you some work, let's get the PR good in upstream, and then here we can cherry-pick from upstream.

@kassane kassane mentioned this pull request Apr 8, 2024
@kassane
Copy link
Contributor Author

kassane commented Apr 8, 2024

Now, aarch64/Linux error

https://github.com/kassane/alpine-ldc2-docker/actions/runs/8603500918/job/23580711641#step:6:2051

 > [linux/arm64  4/11] RUN cd ldc     && cmake -B build     -DCMAKE_INSTALL_PREFIX="/opt"     -GNinja     -DCMAKE_C_COMPILER=clang     -DCMAKE_CXX_COMPILER=clang++     -DCMAKE_BUILD_TYPE=Release     -DCMAKE_EXE_LINKER_FLAGS=-static-libstdc++     -DLLVM_IS_SHARED=OFF     -DD_COMPILER_FLAGS=-link-defaultlib-shared=false     -DBUILD_SHARED_LIBS=BOTH     -DBUILD_LTO_LIBS=ON     && cmake --build build --parallel     && ninja -C build install:
5330.9 /ldc/runtime/druntime/src/core/sys/posix/sys/stat.d(704): Error: undefined identifier `__USE_FILE_OFFSET64`
5331.6 /ldc/runtime/druntime/src/core/sys/posix/sys/stat.d(678): Error: undefined identifier `__USE_FILE_OFFSET64`
5331.6 /ldc/runtime/druntime/src/core/sys/posix/sys/stat.d(693): Error: undefined identifier `__USE_FILE_OFFSET64`
5331.6 /ldc/runtime/druntime/src/core/sys/posix/sys/stat.d(704): Error: undefined identifier `__USE_FILE_OFFSET64`

@kassane kassane marked this pull request as draft April 9, 2024 12:19
@kassane

This comment was marked as resolved.

@kassane
Copy link
Contributor Author

kassane commented Apr 9, 2024

Maybe done for review.
That contribution has extended beyond my expectations. 😅

@kassane kassane marked this pull request as ready for review April 9, 2024 15:58
@kassane kassane changed the title Alpine-musl: fix mmap64 error Musl Clean Apr 9, 2024
@JohanEngelen
Copy link
Member

I'm sorry, I did not realize that removing __USE_FILE_OFFSET64 for Musl would result in this much code duplication... Before doing more work, I think it's good to reconsider and see if there is not a better compromise. Would usercode ever use __USE_FILE_OFFSET64? (starts with double underscore so probably is druntime-only and should not be used by users?)

@kassane
Copy link
Contributor Author

kassane commented Apr 9, 2024

I'm sorry, I did not realize that removing __USE_FILE_OFFSET64 for Musl would result in this much code duplication... Before doing more work, I think it's good to reconsider and see if there is not a better compromise.

Yes, I am in agreement with your point. I have no idea how to avoid the redundant use of version (CRuntime_Musl).

Do you want to revert these latest commits?
Previously, it was enough to make __USE_FILE_OFFSET64=false. cb4c49b

Would usercode ever use __USE_FILE_OFFSET64? (starts with double underscore so probably is druntime-only and should not be used by users?)

Sadly, I don't know of any need for the user to use it. Not unless there's a low-level use case.

@JohanEngelen
Copy link
Member

I'm sorry, I did not realize that removing __USE_FILE_OFFSET64 for Musl would result in this much code duplication... Before doing more work, I think it's good to reconsider and see if there is not a better compromise.

Yes, I am in agreement with your point. I have no idea how to avoid the redundant use of version (CRuntime_Musl).

Do you want to revert these latest commits? Previously, it was enough to make __USE_FILE_OFFSET64=false. cb4c49b

Yes indeed, let's set __USE_FILE_OFFSET64=false.
You could still add code like this

version(CRuntime_Musl)
{
    alias mmap64 = mmap;
}

Would usercode ever use __USE_FILE_OFFSET64? (starts with double underscore so probably is druntime-only and should not be used by users?)

Sadly, I don't know of any need for the user to use it. Not unless there's a low-level use case.

If the user never uses it (it starts with double-underscore, which means that the user really should never use it, normally), then it is OK that it "lies" a little bit. __USE_FILE_OFFSET64 = false is a little lie, because file offset type is still 64bit with musl.

@JohanEngelen
Copy link
Member

Does this PR now fully work with Musl in your testing?

@kassane
Copy link
Contributor Author

kassane commented Apr 10, 2024

Does this PR now fully work with Musl in your testing?

Yes. My alpine-docker updated

#11 0.589 -- LDC version identifier: 1.38.0-git-cb4c49b

This solution is still used by Alpine officially.
I don't know if the maintainer of this package agrees with this inclusion of the patch.

cc: @Geod24

However, it doesn't seem to solve the another issue:

@kinke kinke merged commit faac493 into ldc-developers:master Apr 19, 2024
@kassane kassane deleted the musl_fix branch April 19, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants