[clang] [Driver,sanitizer] Remove RequiresPIE and msan's NeedPIE setting (PR #77689)

Fangrui Song via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 18 15:07:56 PST 2024


MaskRay wrote:

> > > @MaskRay I see that in [3bbc912](https://github.com/llvm/llvm-project/commit/3bbc912d37f03d9ad3be330b81d91c2eaf6c37f2) you removed some tests that fail because of this change. Why do you think that is an appropriate solution? I have some other tests in a downstream product that are failing because we build with CLANG_DEFAULT_PIE_ON_LINUX=OFF. You said in your initial comment here that "most builds set CLANG_DEFAULT_PIE_ON_LINUX to 1, making RequiresPIE/NeedPIE redundant on Linux." But apparently it's not redundant for builds that don't use that setting.
> > > Do you have another solution in progress (or already committed that I haven't seen yet)? It seems that as long as this is a configurable option, we need to support both settings. Intel's SYCL project ([intel/llvm](https://github.com/intel/llvm)) currently sets CLANG_DEFAULT_PIE_ON_LINUX to zero for compatibility with gcc in Fedora releases (at least, I think that's the reason).
> > 
> > 
> > My internal users also use `CLANG_DEFAULT_PIE_ON_LINUX OFF`, so I definitely want to support both flavors.
> > The RUN lines removed by [3bbc912](https://github.com/llvm/llvm-project/commit/3bbc912d37f03d9ad3be330b81d91c2eaf6c37f2) no longer made sense (the commit message could have been reworded). They wanted to check that we defaulted to `-fPIE` even when no `-fno-pic/-fpie/-fpic` was specified. The force-PIC effect might be a previous limitation, or possibly just something cargo culted from the previous msan limitation.
> > If I use `scudo_flags = ["-fsanitize=scudo", "-fno-pic", "-no-pie"]` in `test/scudo/lit.cfg.py`, `check-scudo_standalone` still passes.
> 
> I see. The change to the tests makes sense with that explanation.
> 
> The test we were seeing fail is compiler-rt/test/dfsan/custom.cpp, and I'm told it fails with the main LLVM project if `CLANG_DEFAULT_PIE_ON_LINUX=OFF` is used. I guess we don't have a buildbot that runs that particular test with that setting? The failure is a segmentation fault in dfsan/X86_64Config/Output/custom.cpp.script. I'm not familiar enough with the dfsan tests to say what this means, but it is triggered by the change in this PR.

Thanks for the report. I noticed this error internally as well. There were two portability issues in `dfsan/custom.cpp`, fixed by #78363 and 8434e5d0a16b11ccdc29fc66a3843a94b0ad19f1 .

https://github.com/llvm/llvm-project/pull/77689


More information about the cfe-commits mailing list