Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not hardcode clang/clang++ #110

Merged
merged 1 commit into from
Aug 15, 2021
Merged

Do not hardcode clang/clang++ #110

merged 1 commit into from
Aug 15, 2021

Conversation

dylanaraps
Copy link
Contributor

When the environment has CC/CXX set, the build system ignores them and hardcodes clang/clang++. This changes the Makefile to use the values from the environment if they exist. This allows building with gcc/g++ for example.

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

Could you read https://github.com/rui314/mold/blob/main/CONTRIBUTING.md and add a signed-off-by line to your commit message? Otherwise LGTM.

@dylanaraps
Copy link
Contributor Author

Done. Apologies for forgetting the signed-off line (I knew about this requirement too...).

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 15, 2021

On a semi-related note: Would you accept a PR that removes Cmake entirely and replaces its usage with a simple Makefile (for mimalloc)?

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 15, 2021

It looks as though the GCC in the CI is too old? Might have to force CC=clang CXX=clang++ (or find a way to update GCC). (Or we can see why CC/CXX are set in CI in the first place(?)).

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

Once we do that, don't we need to maintain our local patches to remove dependencies to CMake? CMake is a popular tool these days, so I don't have an urge to do that, but do you?

@dylanaraps
Copy link
Contributor Author

Once we do that, don't we need to maintain our local patches to remove dependencies to CMake? CMake is a popular tool these days, so I don't have an urge to do that, but do you?

Mold would just have a Makefile in its repository (can be stored outside of mimalloc/ so the directory can be updated without issue). No modifications to mimalloc/ should be needed. Another option is to just use cmake for mold as well (though I'd personally prefer pure make).

My intention is to use mold as my system's linker (I am actually already doing this). I created my own Linux distribution (https://kisslinux.org https://github.com/kisslinux) and would like to see mold added to its core repository. Issue is that the core repository has no cmake.

If you aren't open to dropping cmake, that is totally fine. I will maintain patches downstream instead, this is no problem. Just wanted to see which of my modifications are up-streamable. :)

(Thank you for creating mold, it works beautifully.)

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

It looks like this patches changes the default compiler to the default CC and CXX. I want to keep clang and clang++ as defaults, as they work fine in my environment.

Looks like with out making any modification to the Makefile, make CC=gcc CXX=g++ seems to work OK. Have you tried that?

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

If you already have a patch to build mimalloc and tbb without cmake, can you paste it here?

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 15, 2021

To fix CI, we need to remove CC=gcc CXX=g++ from: https://github.com/rui314/mold/blob/main/.github/workflows/ci.yml#L36
This will make the environment use the defaults (clang, clang++) in the Makefile. Edit: This may be wrong actually. Will look into it.

Looks like with out making any modification to the Makefile, make CC=gcc CXX=g++ seems to work OK. Have you tried that?

Yes. My intention is to use the values of CC and CXX from the environment. This way you can just run make (no arguments) and it inherits values from the environment. The majority of Makefiles I have come across support this method of setting CC/CXX/etc.

If you already have a patch to build mimalloc and tbb without cmake, can you paste it here?

I have not done this work yet. I wanted to hear your thoughts before getting started.

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 15, 2021

To (really) fix CI, CC=clang CXX=clang++ must be added here: https://github.com/rui314/mold/blob/main/.github/workflows/ci.yml#L19

Does GitHub (in workflows) set CC/CXX to GCC by default? That is what is causing issues here (as we now inherit from the environment with this PR).

@dylanaraps
Copy link
Contributor Author

Thinking about it some more. It's probably better (and less pain overall) for mold to build itself with cmake in the future. I will experiment with Makefiles regardless (to see how feasible it really is) and let you know how it goes.

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

This patch changes the default compiler in my environment from clang to gcc, even though I didn't set CC or CXX environment variables.

@dylanaraps
Copy link
Contributor Author

This patch changes the default compiler in my environment from clang to gcc, even though I didn't set CC or CXX environment variables.

What is your environment? It should only use GCC if CC and CXX are set accordingly. That is how the ?= feature of GNU make works is it not? ie, it only sets the value if unset. In other words, something is setting CC and CXX to GCC values in your environment(?).

@rui314
Copy link
Owner

rui314 commented Aug 15, 2021

Ubuntu 20.04. Without this patch, when I just run make, it uses clang and clang++ to compile source files. After this patch, it uses gcc and g++ instead. I'm not setting neither CC nor CXX environment variables.

Makefile Outdated Show resolved Hide resolved
Signed-off-by: Dylan Araps <dylan.araps@gmail.com>
@dylanaraps
Copy link
Contributor Author

Should be OK now.

@rui314 rui314 merged commit a46ba48 into rui314:main Aug 15, 2021
@dylanaraps dylanaraps deleted the fix-cc-cxx branch August 15, 2021 12:25
@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 15, 2021

Thanks for merging. :)

If you are wondering, here's the status of each package I am maintaining (and whether or not mold can be used): kisslinux/repo#318 (anything ticked works fine) (broken: grub (linker script), gcc (segfault in genmddeps))

@rui314
Copy link
Owner

rui314 commented Aug 16, 2021

@dylanaraps Is the segfaulting program for i386? mold's i386 support is not tested very well.

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 16, 2021

@dylanaraps Is the segfaulting program for i386? mold's i386 support is not tested very well.

I am certain it is not for i386.

  • GCC is compiled without multilib support (only x86_64).
  • My kernel has 32bit support disabled entirely.
  • GCC seems to use the i386 config directory for x86_64 (hybrid support: x86 or x86_64 or x86 + x86_64).

Here is the full compiler command (c++ == g++):

c++   -O3 -march=native -pipe -DIN_GCC     -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wca
st-qual -Wno-error=format-diag -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-s
trings   -DHAVE_CONFIG_H  -DGENERATOR_FILE -fno-PIE -static-libstdc++ -static-libgcc  -no-pie -o build/genmddeps \
    build/genmddeps.o build/read-md.o build/errors.o ../build-x86_64-pc-linux-musl/libiberty/libiberty.a
build/gengtype  \
                    -S ../../gcc/gcc -I gtyp-input.list -w tmp-gtype.state

Edit: Some details:

  • GCC is 11.2.0.
  • Mold is /usr/bin/ld
  • The C library is musl (genmddeps links to libc.so)
  • readelf -a genmddeps https://termbin.com/mcli

Running valgrind on the executable:

==17965== Command: ./genmddeps
==17965== 
--17965-- WARNING: Serious error when reading debug info
--17965-- When reading debug info from /tmp/24590/build/gcc/gcc-build/gcc/build/genmddeps:
--17965-- Can't make sense of .rodata section mapping
--17965-- WARNING: Serious error when reading debug info
--17965-- When reading debug info from /usr/lib/libc.so:
--17965-- Can't make sense of .rodata section mapping
vex amd64->IR: unhandled instruction bytes: 0x8F 0x6A 0x78 0x10 0xEF 0x2 0x1 0x0 0x0 0x83
vex amd64->IR:   REX=0 REX.W=0 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==17965== valgrind: Unrecognised instruction at address 0x40be4e5.
==17965==    at 0x40BE4E5: ??? (in /usr/lib/libc.so)
==17965==    by 0x1FFFF: ???

gdb:

(gdb) run
Starting program: /tmp/18111/build/gcc/gcc-build/gcc/build/genmddeps 

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ff1fcd in do_relocs (dso=0x7ffff7ffde00 <app>, rel=0x2024e8, rel_size=168, stride=3) at ldso/dynlink.c:427
427	ldso/dynlink.c: No such file or directory.
(gdb) bt
#0  0x00007ffff7ff1fcd in do_relocs (dso=0x7ffff7ffde00 <app>, rel=0x2024e8, rel_size=168, stride=3) at ldso/dynlink.c:427
#1  0x00007ffff7ff4d40 in reloc_all (p=0x7ffff7ffde00 <app>) at ldso/dynlink.c:1356
#2  0x00007ffff7ff6a6c in __dls3 (sp=0x7fffffffe9b0, auxv=0x7fffffffead8) at ldso/dynlink.c:1952
#3  0x00007ffff7ff5e7f in __dls2b (sp=0x7fffffffe9b0, auxv=0x7fffffffead8) at ldso/dynlink.c:1721
#4  0x00007ffff7ff5dad in __dls2 (base=0x7ffff7f13000 "\177ELF\002\001\001", sp=0x7fffffffe9b0) at ldso/dynlink.c:1697
#5  0x00007ffff7ff1334 in _dlstart_c (sp=0x7fffffffe9b0, dynv=0x7ffff7ff9088) at ldso/dlstart.c:147
#6  0x00007ffff7ff0fda in _dlstart () from /lib/ld-musl-x86_64.so.1
#7  0x0000000000000001 in ?? ()
#8  0x00007fffffffec33 in ?? ()
#9  0x0000000000000000 in ?? ()

@dylanaraps
Copy link
Contributor Author

dylanaraps commented Aug 16, 2021

GCC builds successfully when I link it statically.

Edit: When I just statically link genmddeps the build continues and then fails later on with:

xgcc: internal compiler error: Segmentation fault signal terminated program cc1
-> ./cc1
Segmentation fault

-> gdb ./cc1
(gdb) r
Starting program: /tmp/917/build/gcc/gcc-build/gcc/cc1 

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7ff1fcd in do_relocs (dso=0x7ffff7ffde00 <app>, rel=0x618258, rel_size=1752, stride=3) at ldso/dynlink.c:427
427	ldso/dynlink.c: No such file or directory.
(gdb) bt
#0  0x00007ffff7ff1fcd in do_relocs (dso=0x7ffff7ffde00 <app>, rel=0x618258, rel_size=1752, stride=3) at ldso/dynlink.c:427
#1  0x00007ffff7ff4d40 in reloc_all (p=0x7ffff7ffde00 <app>) at ldso/dynlink.c:1356
#2  0x00007ffff7ff6a6c in __dls3 (sp=0x7fffffffe9c0, auxv=0x7fffffffeae8) at ldso/dynlink.c:1952
#3  0x00007ffff7ff5e7f in __dls2b (sp=0x7fffffffe9c0, auxv=0x7fffffffeae8) at ldso/dynlink.c:1721
#4  0x00007ffff7ff5dad in __dls2 (base=0x7ffff7f13000 "\177ELF\002\001\001", sp=0x7fffffffe9c0) at ldso/dynlink.c:1697
#5  0x00007ffff7ff1334 in _dlstart_c (sp=0x7fffffffe9c0, dynv=0x7ffff7ff9088) at ldso/dlstart.c:147
#6  0x00007ffff7ff0fda in _dlstart () from /lib/ld-musl-x86_64.so.1
#7  0x0000000000000001 in ?? ()
#8  0x00007fffffffec4e in ?? ()
#9  0x0000000000000000 in ?? ()
(gdb) 

@rui314
Copy link
Owner

rui314 commented Aug 16, 2021

Can you rebuild cc1 with the environment variable MOLD_REPRO set to 1 and send the file to me? With that environment variable, mold embeds all input files to an output file (in the .repro section), which allows me to re-run the same build command on my machine.

@dylanaraps
Copy link
Contributor Author

Sha256sum: bc771e4f28b7b332e823de1b84de09751fb6751176e1c2d3f2e67914ecd9fac5

https://0x0.st/-yKI.tar (GitHub doesn't allow tarball uploads)

rui314 added a commit that referenced this pull request Aug 17, 2021
Previously, we created dynamic relocations against read-only sections
if the sections contain relocations against weak undefined symbols.
Since such dynamic relocations cannot be applied at load-time, the
generated executables failed with SEGV.

Now, such relocations are resolved to address 0.

Reported at #110
@rui314
Copy link
Owner

rui314 commented Aug 17, 2021

I believe your issue has been resolved by the most recent commit that I made to the main branch. Can you confirm?

@dylanaraps
Copy link
Contributor Author

Works! Thank you for fixing it.

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