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

Unbreak build with Clang 7 #4989

Closed
wants to merge 1 commit into from
Closed

Unbreak build with Clang 7 #4989

wants to merge 1 commit into from

Conversation

jbeich
Copy link
Contributor

@jbeich jbeich commented Aug 14, 2018

--image-base=value is only supported by LLD but Clang doesn't forward many options to linker by default. See error log

$ clang60 -image-base=0x10000 -fuse-ld=lld /path/to/test.c
clang-6.0: warning: argument unused during compilation: '-image-base=0x10000' [-Wunused-command-line-argument]
$ clang70 -image-base=0x10000 -fuse-ld=lld  /path/to/test.c
clang-7.0: error: unknown argument: '-image-base=0x10000'

@hcorion
Copy link
Member

hcorion commented Aug 14, 2018

I'm not entirely sure if image-base is needed, it was an attempted solution to #2516 but I believe the actual solution was just -no-pie.
Also I think image-base only works for 32-bit targets, which we don't even support. I think it's safe to remove it.

@JohnHolmesII
Copy link
Contributor

Well you broke 5 instead 😄

@jbeich
Copy link
Contributor Author

jbeich commented Sep 14, 2018

Can someone test runtime on macOS? 2fcd38c added -pagezero_size to the same conditional, so I wonder if it's required without -image_base.

CC @rawrasaur

@hcorion
Copy link
Member

hcorion commented Sep 15, 2018

I tested recently and image_base is needed for MacOS.

@Nekotekina
Copy link
Member

Need to compare clang version then.

Clang 7 requires -Wl prefix but adding it exposes lack of -image-base
support in BFD and Gold linkers, often used on Linux. Drop -image-base
as st11range issues were apparently fixed by -no-pie.
@AniLeo
Copy link
Member

AniLeo commented Nov 1, 2018

Any updates?

@jbeich
Copy link
Contributor Author

jbeich commented Nov 1, 2018

macOS change is gone to avoid adding more code just to test linker type (or Clang version). Otherwise, my Clang 7 build crashes since 1b74099 (see LLVM PR39246).

@hcorion
Copy link
Member

hcorion commented Nov 2, 2018

I think it would be best to test if -image-base works/exist, via CHECK_CXX_COMPILER_FLAG and using -Wl,image-base,0x10000 which I think should work?

@JohnHolmesII
Copy link
Contributor

Any chance we could move this along? I think it's sane as is. Only apple seems to need the flag, only apple gets the flag. It would really help my iteration time.

@JohnHolmesII JohnHolmesII mentioned this pull request Jun 8, 2019
@Nekotekina
Copy link
Member

Closing as merged.

@Nekotekina Nekotekina closed this Jun 27, 2019
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.

None yet

5 participants