[clang][flang] Disable defaulting to `-fpie` for LLVM Flang

[awarzynski]

Summary

In, https://reviews.llvm.org/D120305, CLANG_DEFAULT_PIE_ON_LINUX was set
to On by default. However, neither -fpie nor -fpic are currently
supported in LLVM Flang. Hence, in this patch, the behavior controlled
with CLANG_DEFAULT_PIE_ON_LINUX is refined not to apply to Flang.

Another way to look at this is that CLANG_DEFAULT_PIE_ON_LINUX is
currently affecting both Clang and Flang. However, IIUC, the intention for this
CMake variable has always been to only apply to Clang. This patch makes
sure that that's indeed the case.

Without this change, you might see errors like this on X86_64:

/usr/bin/ld: main.o: relocation R_X86_64_32 against `.bss' can not be used when making a PIE object; recompile with -fPIC

I've not experienced any issues on AArch64. That's probably because on
AArch64 some object files happen to be position independent without
needing -fpie or -fpic.

We can revisit this once support for -fpie and -fpic is available in LLVM Flang.
I'm not aware of anyone actively working in this area.

[kiranchandramohan]

Is the longer-term plan to support this in Flang as well?
Does this affect building object files too or is it just the executable? How would this affect mixed C++/Fortran programs if the Clang and Flang settings are different?

[awarzynski]

In D128333#3601299, @kiranchandramohan wrote:

Is the longer-term plan to support this in Flang as well?

I don't see why not. AFAIK, the switch in Clang took a while and happened gradually - so we probably shouldn't rush this in Flang. We'd also need to check the behavior in gfortran (so that we maintain consistency).

Does this affect building object files too or is it just the executable?

Just the executable.

AFAIK, isPIEDefault is only used in tools::ParsePICArgs . In Flang, we don't support any of -fpic/-fPIC/-fpie/-fPIE. And CLANG_DEFAULT_PIE_ON_LINUX leads to -fpie being added by default to the final linker invocation (when generating the executable). But that doesn't make sense if the object files were generated with -fpic. Hence the change.

How would this affect mixed C++/Fortran programs if the Clang and Flang settings are different?

Good question! We've not experimented with mixed C++/Fortran programs enough, so I don't have an answer. But we should probably create a GitHub ticket to restore consistency between Clang and Flang.

[kiranchandramohan]

LGTM.

[MaskRay]

gfortran defaults to PIE as well.

We can revisit this once support for -fpie and -fpic is available in LLVM Flang. I'm not aware of anyone actively working in this area.

Disagree. The PIE default should be fine. When PIE support is added, the default mode naturally becomes PIE.
Note: -fpie object files can be linked with either -no-pie or -pie. -fno-pic object files can only be linked with -no-pie.
-fpie is more portable than -fno-pic.

(Why does flang use clang/lib/Driver? For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

[raghavendhra]

LGTM

[raghavendhra]

LGTM

[awarzynski]

@MaskRay, thank for taking a look!

In D128333#3605745, @MaskRay wrote:

gfortran defaults to PIE as well.

While we strive to be compatible with gfortan, there's a lot relatively "basic" things still missing in LLVM Flang. So this is not the highest priority ATM.

We can revisit this once support for -fpie and -fpic is available in LLVM Flang. I'm not aware of anyone actively working in this area.

Disagree. The PIE default should be fine. When PIE support is added, the default mode naturally becomes PIE.
Note: -fpie object files can be linked with either -no-pie or -pie. -fno-pic object files can only be linked with -no-pie.
-fpie is more portable than -fno-pic.

But an object file with R_X86_64_32 relocations cannot be linked with a -pie object, right? How can I make sure that there are no such relocations then?

(Why does flang use clang/lib/Driver? For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

Otherwise, we'd have to re-implement clang/lib/Driver for LLVM Flang. This design was proposed and discussed here.

For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

Isn't this a bit similar to e.g. LLVM developers? Or MLIR developers?

[MaskRay]

In D128333#3613665, @awarzynski wrote:

@MaskRay, thank for taking a look!

In D128333#3605745, @MaskRay wrote:

gfortran defaults to PIE as well.

While we strive to be compatible with gfortan, there's a lot relatively "basic" things still missing in LLVM Flang. So this is not the highest priority ATM.

We can revisit this once support for -fpie and -fpic is available in LLVM Flang. I'm not aware of anyone actively working in this area.

Disagree. The PIE default should be fine. When PIE support is added, the default mode naturally becomes PIE.
Note: -fpie object files can be linked with either -no-pie or -pie. -fno-pic object files can only be linked with -no-pie.
-fpie is more portable than -fno-pic.

But an object file with R_X86_64_32 relocations cannot be linked with a -pie object, right? How can I make sure that there are no such relocations then?

True. If it is difficult to override the -pie default from flang side, I am fine with the code change.

(Why does flang use clang/lib/Driver? For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

Otherwise, we'd have to re-implement clang/lib/Driver for LLVM Flang. This design was proposed and discussed here.

OK, so you did bring this up. I guess I'll have to accept...

For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

Isn't this a bit similar to e.g. LLVM developers? Or MLIR developers?

flang/test/Driver/no-pie.f90

3

The ! RUN COMMANDS and EXPECTED OUTPUT comments seem rather redundant. I'd remove them.

11

This NOT pattern can easily get stale. You need a test that driver -pie forwards -pie to the linker and by default -pie does not pass the the linker.

[awarzynski]

In D128333#3613696, @MaskRay wrote:

True. If it is difficult to override the -pie default from flang side, I am fine with the code change.

Thanks! The proper/long-term fix will require extending Flang's frontend driver so that it supports -mrelocation-model, -fpic and -fpic-level (and probably a few more). But I don't want to rush that.

(Why does flang use clang/lib/Driver? For clang developers, it seems that check-clang check-clang-tools is not sufficient. check-flang needs to be used as well.)

Otherwise, we'd have to re-implement clang/lib/Driver for LLVM Flang. This design was proposed and discussed here.

OK, so you did bring this up. I guess I'll have to accept...

With clangDriver being effectively shared between Clang and Flang, it would make a lot of sense to lift clangDriver out of Clang so that it's an independent sub-project. We've proposed that and folks have been mostly in favor. Relevant RFCs:

• RFC: refactoring libclangDriver/libclangFrontend to share with Flang
• RFC: refactoring clangDriver - diagnostics classes

Not much is really happening in this area ATM, but I'm hoping that https://discourse.llvm.org/t/rfc-improving-clang-s-diagnostics could unblock some progress.

flang/test/Driver/no-pie.f90

3

This style is quite common in this directory, see e.g. https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/emit-asm-aarch64.f90. I'm hoping that this makes these tests easier to follow for folks less familiar with LIT in general.

If that's OK, I'll leave this here (but I don't expect others to follow similar approach, IMO it's a matter of personal preference).

11

Good point, will update!

[awarzynski]

Update the test following the comments from @MaskRay

Also added a comment in Linux.cpp and renamed no-pie.f90 as pic-flags.f90 (to avoid FileCheck matching e.g. ! CHECK: pie against the file name).

[MaskRay]

flang/test/Driver/no-pie.f90

3

Since this is precedent, I assume this is fine. I will note here that this is contrary to the convention almost everywhere in llvm-project...

flang/test/Driver/pic-flags.f90

6 ↗

(On Diff #440628)

In clang driver, -target is a deprecated form (Separate style options are very uncommon in external command line utilities). The preferred spelling is --target=

15 ↗

(On Diff #440628)

fpic is not waterproof. The temporary object file name or the working directory may have fpic as a substring. Use "-fpic"

20 ↗

(On Diff #440628)

! TODO: may possibly be recognized a future FileCheck/lit as a stale check prefix. Personally I may use something like !! TODO:. I guess you are just following existing practice so this may be fine.

[awarzynski]

Update the test

clang/lib/Driver/ToolChains/Linux.cpp

if (GCCInstallation.isValid()) {
     CC1Args.push_back("-isystem");
     CC1Args.push_back(DriverArgs.MakeArgString(
         GCCInstallation.getParentLibPath() + "/../" +
         GCCInstallation.getTriple().str() + "/include"));
   }
 }
 
 bool Linux::isPIEDefault(const llvm::opt::ArgList &Args) const {
-  return CLANG_DEFAULT_PIE_ON_LINUX || getTriple().isAndroid() ||
-         getTriple().isMusl() || getSanitizerArgs(Args).requiresPIE();
+  return !getDriver().IsFlangMode() &&
+         (CLANG_DEFAULT_PIE_ON_LINUX || getTriple().isAndroid() ||
+          getTriple().isMusl() || getSanitizerArgs(Args).requiresPIE());
 }
 
 bool Linux::IsAArch64OutlineAtomicsDefault(const ArgList &Args) const {
   // Outline atomics for AArch64 are supported by compiler-rt
   // and libgcc since 9.3.1
   assert(getTriple().isAArch64() && "expected AArch64 target!");
   ToolChain::RuntimeLibType RtLib = GetRuntimeLibType(Args);
   if (RtLib == ToolChain::RLT_CompilerRT)

flang/test/Driver/no-pie.f90

+! Verify that in contrast to Clang, Flang does not default to generating position independent executables/code
+
+!-------------
+! RUN COMMANDS
+!-------------
+! RUN: %flang -### -flang-experimental-exec %S/Inputs/hello.f90 2>&1 | FileCheck %s
+
+!----------------
+! EXPECTED OUTPUT
+!----------------
+! CHECK-NOT: fpie
+! CHECK-NOT: fpic