Page MenuHomePhabricator

[libc] Don't pass -fpie/-ffreestanding on Windows
ClosedPublic

Authored by caitlyncano on Jul 8 2021, 10:49 AM.

Details

Summary

The current compile options function hardcodes the -fpie and
-ffreestanding flags, which don't exist on Windows. This patch sets the
compilation flags conditionally based on the OS specifics.

Diff Detail

Event Timeline

caitlyncano created this revision.Jul 8 2021, 10:49 AM
caitlyncano requested review of this revision.Jul 8 2021, 10:49 AM
aeubanks added inline comments.Jul 8 2021, 10:58 AM
libc/cmake/modules/LLVMLibCObjectRules.cmake
5

missing a space

7

I'd recommend setting the common flags outside the if, then adding -fpie -ffreestanding / /Qfreestanding in the if

I think

set(${output_var} ${output_var} -extraflags)

should append flags

[libc] Fixing space and shifting set configuration

Moving outside the common flags outside the if.

caitlyncano marked 2 inline comments as done.Jul 8 2021, 1:53 PM

[libc] Fixing flag overwrites

Previous amendment overwrote the output by not including the flags set outside
the if block

the commit title should be imperative, see https://chris.beams.io/posts/git-commit/ for good git commit tips

and I'd be clearer and say something like "Don't pass -fpie/-ffreestanding on Windows"

libc/cmake/modules/LLVMLibCObjectRules.cmake
6

actually I don't see this available in clang-cl or msvc, where did you find this?

caitlyncano retitled this revision from [libc] Specificying compilation flags for Windows to [libc] Don't pass -fpie/-ffreestanding on Windows.Jul 8 2021, 3:17 PM
caitlyncano added inline comments.
libc/cmake/modules/LLVMLibCObjectRules.cmake
6
aeubanks added inline comments.Jul 8 2021, 3:23 PM
libc/cmake/modules/LLVMLibCObjectRules.cmake
6

that looks like it only works on the intel C++ compiler, it won't work with msvc or clang which is what we care about on windows

let's try not adding any alternatives for -fpie/-ffreestanding for now on windows

8

nit: spacing

caitlyncano added inline comments.Jul 8 2021, 3:24 PM
libc/cmake/modules/LLVMLibCObjectRules.cmake
6

But it seems like in this article (https://nullprogram.com/blog/2016/01/31/) -ffreestanding is available on MinGW so maybe that's where we'd have to build llvmlibc to ensure the freestanding environment?

caitlyncano marked 2 inline comments as done.Jul 8 2021, 3:27 PM

[libc] Don't include -ffreestanding equivalent on Windows

/Qfreestanding is only available on Intel C++ compiler, so it won't affect a
compilation in clang/msvc.

aeubanks added inline comments.Jul 8 2021, 3:31 PM
libc/cmake/modules/LLVMLibCObjectRules.cmake
6

/Qfreestanding is only available on the intel C++ compiler for windows

-ffreestanding is available on gcc/clang/intel for Linux

mingw is similar to Linux (even it's on windows) and uses gcc, but we currently don't care about mingw, at least for llvm libc

6

we should skip this if block since it's not doing anything

[libc] Simplify conditional

Unecessary inclusion of an else case removed since Windows will not use any
flags outside the standard compiler flags.

caitlyncano marked an inline comment as done.Jul 8 2021, 3:38 PM
aeubanks accepted this revision.Jul 8 2021, 3:41 PM

lgtm

This revision is now accepted and ready to land.Jul 8 2021, 3:41 PM
sivachandra accepted this revision.Jul 12 2021, 9:06 AM
This revision was landed with ongoing or failed builds.Jul 13 2021, 1:40 PM
This revision was automatically updated to reflect the committed changes.