This is an archive of the discontinued LLVM Phabricator instance.

Differential D47480

clang-cl: Expose -no-canonical-prefixes
ClosedPublic

Authored by thakis on May 29 2018, 8:02 AM.

Details

Reviewers
rnk
takuto.ikuta
Summary

-no-canonical-prefixes is a weird flag: In gcc, it controls whether realpath() is called on the path of the driver binary. It's needed to support some usecases where gcc is symlinked to, see https://gcc.gnu.org/ml/gcc/2011-01/msg00429.html for some background.

In clang, the resource dir is found relative to the compiler binary, and without -no-canonical-prefixes that's an absolute path. For clang, the main use case for -no-canonical-prefixes is to make the -resource-dir path added by the driver relative instead of absolute. Making it relative seems like the better default, but since neither clang not gcc have -canonical-prefixes without no- which makes changing the default tricky, and since some symlink behaviors do depend on the realpath() call at least for gcc, just expose -no-canonical-prefixes in clang-cl mode.

Alternatively we could default to no-canonical-prefix-mode for clang-cl since it's less likely to be used in symlinked scenarios, but since you already need to about -no-canonical-prefixes for the non-clang-cl bits of your build, not hooking this of driver mode seems better to me.

Diff Detail

Event Timeline

thakis created this revision.May 29 2018, 8:02 AM
takuto.ikuta added a subscriber: takuto.ikuta.May 29 2018, 8:13 AM

Thank you for quick supporting, but this is not sufficient for clang-cl on windows.

llvm::InitLLVM calls windows::GetCommandLineArguments, and argv0 is changed to absolute path.
https://github.com/llvm-project/llvm-project-20170507/blob/7c81daae49eaf7f9681457b46c569c1bc6b80283/llvm/lib/Support/Windows/Process.inc#L229

Then clang's builtin include files are shown in absolute path even we specify -no-canonical-prefixes.
Can we keep using original argv0 when argv0 is not 8.3 filename? Or is there better solution?

thakis added a comment.May 29 2018, 8:38 AM

I did test this locally before sending it out, and the -resource-dir arg passed to cc1 is relative with this patch:

$ bin/clang-cl -no-canonical-prefixes -c test.cc -###
clang version 7.0.0 
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: bin
 "bin/clang-cl" "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0" "-emit-obj" "-mrelax-all" "-mincremental-linker-compatible" "-disable-free" "-main-file-name" "test.cc" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model" "posix" "-relaxed-aliasing" "-fmath-errno" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "x86-64" "-mllvm" "-x86-asm-syntax=intel" "-D_MT" "-flto-visibility-public-std" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-stack-protector" "2" "-fms-volatile" "-fdiagnostics-format" "msvc" "-dwarf-column-info" "-debugger-tuning=gdb" "-target-linker-version" "305" "-momit-leaf-frame-pointer" "-coverage-notes-file" "/Users/thakis/src/llvm-mono/test.gcno" "-resource-dir" "lib/clang/7.0.0" "-internal-isystem" "lib/clang/7.0.0/include" "-fdeprecated-macro" "-fdebug-compilation-dir" "/Users/thakis/src/llvm-mono" "-ferror-limit" "19" "-fmessage-length" "254" "-fno-use-cxa-atexit" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=19.11" "-std=c++14" "-fdelayed-template-parsing" "-fobjc-runtime=gcc" "-fseh-exceptions" "-fdiagnostics-show-option" "-fcolor-diagnostics" "-o" "test.obj" "-x" "c++" "test.cc"

driver.cpp handles this directly here http://llvm-cs.pcc.me.uk/tools/clang/tools/driver/driver.cpp#54 and here http://llvm-cs.pcc.me.uk/tools/clang/tools/driver/driver.cpp#390 -- what's the problem with the InitLLVM call you point at?

takuto.ikuta added a comment.May 29 2018, 2:28 PM

On windows, argv0 is changed to absolute path before we call GetExecutablePath.
That makes resource-dir absolute.
I need clang-cl on windows has relative --resource-dir.

See around heres.
https://github.com/llvm-project/llvm-project-20170507/blob/83b39c10b1d7a9189b7ee646212eccf6e61dcfbf/clang/tools/driver/driver.cpp#L323
https://github.com/llvm-project/llvm-project-20170507/blob/ca50c5d758541decdb59330fb15fe6b73ba6467d/llvm/lib/Support/InitLLVM.cpp#L28

thakis added a subscriber: rnk.May 29 2018, 3:51 PM

But GetExecutablePath isn't called if -no-canonical-prefixes is passed, is
it? (See the first link I pasted above)

takuto.ikuta added a comment.May 29 2018, 5:48 PM

GetExecutablePath is called here.
http://llvm-cs.pcc.me.uk/tools/clang/tools/driver/driver.cpp#427

You'll understand what I said if you see the behavior of clang-cl on windows.

test/Driver/cl-options.c
595

Is this related to this change?

rnk accepted this revision.May 30 2018, 11:32 AM

lgtm

test/Driver/cl-options.c
595

It's a test for a previous change that I made.

This revision is now accepted and ready to land.May 30 2018, 11:32 AM
takuto.ikuta accepted this revision.Jun 1 2018, 7:02 AM

I confirmed this CL and https://reviews.llvm.org/D47578 remove absolute path from /showIncludes when include paths are given in relative.

thakis closed this revision.Jun 1 2018, 8:04 AM

r333761, thanks!

thakis marked 2 inline comments as done.Jun 1 2018, 8:07 AM
thakis added inline comments.
test/Driver/cl-options.c
595

Sorry, I missed this. It was a mistake, I removed this line again in r333762.

thakis mentioned this in D108818: [clang] Add a -canonical-prefixes option.Aug 27 2021, 7:23 AM
thakis mentioned this in rG3d157cfcc4ef: [clang] Add a -canonical-prefixes option.Sep 1 2021, 11:51 AM

Revision Contents

PathSize
include/
clang/
Driver/
2 lines
test/
Driver/
2 lines

Diff 148912

include/clang/Driver/Options.td

Show First 20 Lines • Show All 2,245 Lines • ▼ Show 20 Lines
def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group<Action_Group>, def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group<Action_Group>,
HelpText<"Provide information about a particular module file">; HelpText<"Provide information about a particular module file">;
def mthumb : Flag<["-"], "mthumb">, Group<m_Group>; def mthumb : Flag<["-"], "mthumb">, Group<m_Group>;
def mtune_EQ : Joined<["-"], "mtune=">, Group<m_Group>; def mtune_EQ : Joined<["-"], "mtune=">, Group<m_Group>;
def multi__module : Flag<["-"], "multi_module">; def multi__module : Flag<["-"], "multi_module">;
def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">; def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
def multiply__defined : Separate<["-"], "multiply_defined">; def multiply__defined : Separate<["-"], "multiply_defined">;
def mwarn_nonportable_cfstrings : Flag<["-"], "mwarn-nonportable-cfstrings">, Group<m_Group>; def mwarn_nonportable_cfstrings : Flag<["-"], "mwarn-nonportable-cfstrings">, Group<m_Group>;
def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, Flags<[HelpHidden]>, def no_canonical_prefixes : Flag<["-"], "no-canonical-prefixes">, Flags<[HelpHidden, CoreOption]>,
HelpText<"Use relative instead of canonical paths">; HelpText<"Use relative instead of canonical paths">;
def no_cpp_precomp : Flag<["-"], "no-cpp-precomp">, Group<clang_ignored_f_Group>; def no_cpp_precomp : Flag<["-"], "no-cpp-precomp">, Group<clang_ignored_f_Group>;
def no_integrated_cpp : Flag<["-", "--"], "no-integrated-cpp">, Flags<[DriverOption]>; def no_integrated_cpp : Flag<["-", "--"], "no-integrated-cpp">, Flags<[DriverOption]>;
def no_pedantic : Flag<["-", "--"], "no-pedantic">, Group<pedantic_Group>; def no_pedantic : Flag<["-", "--"], "no-pedantic">, Group<pedantic_Group>;
def no__dead__strip__inits__and__terms : Flag<["-"], "no_dead_strip_inits_and_terms">; def no__dead__strip__inits__and__terms : Flag<["-"], "no_dead_strip_inits_and_terms">;
def nobuiltininc : Flag<["-"], "nobuiltininc">, Flags<[CC1Option, CoreOption]>, def nobuiltininc : Flag<["-"], "nobuiltininc">, Flags<[CC1Option, CoreOption]>,
HelpText<"Disable builtin #include directories">; HelpText<"Disable builtin #include directories">;
def nocudainc : Flag<["-"], "nocudainc">; def nocudainc : Flag<["-"], "nocudainc">;
▲ Show 20 Lines • Show All 706 Lines • Show Last 20 Lines

test/Driver/cl-options.c

Show First 20 Lines • Show All 585 Lines • ▼ Show 20 Lines
// RUN: -resource-dir asdf \ // RUN: -resource-dir asdf \
// RUN: -resource-dir=asdf \ // RUN: -resource-dir=asdf \
// RUN: -Wunused-variable \ // RUN: -Wunused-variable \
// RUN: -fmacro-backtrace-limit=0 \ // RUN: -fmacro-backtrace-limit=0 \
// RUN: -fstandalone-debug \ // RUN: -fstandalone-debug \
// RUN: -flimit-debug-info \ // RUN: -flimit-debug-info \
// RUN: -flto \ // RUN: -flto \
// RUN: -fmerge-all-constants \ // RUN: -fmerge-all-constants \
// RUN: -no-canonical-prefixes \
// RUN: -fno-coverage-mapping \
takuto.ikutaUnsubmitted
Done
ReplyInline Actions

Is this related to this change?

takuto.ikuta: Is this related to this change?
rnkUnsubmitted
Done
ReplyInline Actions

It's a test for a previous change that I made.

rnk: It's a test for a previous change that I made.
thakisAuthorUnsubmitted
Not Done
ReplyInline Actions

Sorry, I missed this. It was a mistake, I removed this line again in r333762.

thakis: Sorry, I missed this. It was a mistake, I removed this line again in r333762.
// RUN: --version \ // RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1 // RUN: -Werror /Zs -- %s 2>&1
void f() { } void f() { }