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

clang-cl doesn't properly handle the -o flag #21268

Closed
ehsan opened this issue Sep 10, 2014 · 21 comments
Closed

clang-cl doesn't properly handle the -o flag #21268

ehsan opened this issue Sep 10, 2014 · 21 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'

Comments

@ehsan
Copy link
Contributor

ehsan commented Sep 10, 2014

Bugzilla Link 20894
Resolution FIXED
Resolved on Sep 11, 2014 13:16
Version trunk
OS All
CC @zmodem,@jrmuizel,@nico,@rnk,@rinon,@timurrrr

Extended Description

I know that clang-cl ignores -o completely, but now I'm running into a weird situation that I can't fig myself out of. Here, we're passing the ASAN libraries on the command line to clang-cl without -c. It gets the output filename wrong by thinking that it should be derived from the input library, and it passes the wrong -out: flag to the linker which completely confuses it:

$ clang-cl -o conftest -Z7 -fallback -fsanitize=address clang_rt.asan_dynamic-i386.lib clang_rt.asan_uar_thunk-i386.lib conftest.c
clang-cl.exe: warning: argument unused during compilation: '-o conftest'
LINK : fatal error LNK1149: output filename matches input filename 'c:\moz\fallback\clang_rt.asan_dynamic-i386.lib'
clang-cl.exe: error: linker command failed with exit code 1149 (use -v to see invocation)

$ clang-cl -o conftest -Z7 -fallback -fsanitize=address clang_rt.asan_dynamic-i386.lib clang_rt.asan_uar_thunk-i386.lib conftest.c -###
clang version 3.6.0
Target: i686-pc-windows-msvc
Thread model: posix
clang-cl.exe: warning: argument unused during compilation: '-o conftest'
"c:\moz\llvm-objdir\bin\clang-cl.exe" "-cc1" "-triple" "i686-pc-windows-msvc" "-emit-obj" "-mrelax-all" "-disable-free" "-main-file-name" "conftest.c" "-mrelocation-model" "static" "-mdisable-fp-elim" "-relaxed-aliasing" "-fmath-errno" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "pentium4" "-D_MT" "--dependent-lib=libcmt" "--dependent-lib=oldnames" "-fdiagnostics-format" "msvc-fallback" "-gline-tables-only" "-dwarf-column-info" "-resource-dir" "c:\moz\llvm-objdir\bin\..\lib\clang\3.6.0" "-internal-isystem" "c:\moz\llvm-objdir\bin\..\lib\clang\3.6.0\include" "-internal-isystem" "C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\INCLUDE" "-internal-isystem" "C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\ATLMFC\INCLUDE" "-internal-isystem" "C:\Program Files (x86)\Windows Kits\8.0\include\shared" "-internal-isystem" "C:\Program Files (x86)\Windows Kits\8.0\include\um" "-internal-isystem" "C:\Program Files (x86)\Windows Kits\8.0\include\winrt" "-fdebug-compilation-dir" "c:\moz\fallback" "-ferror-limit" "19" "-fmessage-length" "152" "-fsanitize=address" "-fsanitize-blacklist=c:\moz\llvm-objdir\bin\..\lib\clang\3.6.0\asan_blacklist.txt" "-mstackrealign" "-fms-extensions" "-fms-compatibility" "-fms-compatibility-version=17.00" "-fdelayed-template-parsing" "-fobjc-runtime=gcc" "-fdiagnostics-show-option" "-fcolor-diagnostics" "-o" "C:/Users/EHSANA1/AppData/Local/Temp\conftest-0ff154.obj" "-x" "c" "conftest.c" || "c:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\BIN\cl.exe" "/nologo" "/c" "/W0" "/Z7" "/Tc" "conftest.c" "/FoC:/Users/EHSANA1/AppData/Local/Temp\conftest-0ff154.obj"
"c:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\BIN\link.exe" "-out:clang_rt.asan_dynamic-i386.exe" "-nologo" "-debug" "-debug" "-incremental:no" "c:\moz\llvm-objdir\bin\..\lib\clang\3.6.0\lib\windows\clang_rt.asan-i386.lib" "c:\moz\llvm-objdir\bin\..\lib\clang\3.6.0\lib\windows\clang_rt.asan_cxx-i386.lib" "clang_rt.asan_dynamic-i386.lib" "clang_rt.asan_uar_thunk-i386.lib" "C:/Users/EHSANA~1/AppData/Local/Temp\conftest-0ff154.obj"

Should we add proper support for -o? Or somehow differentiate between .lib and .c files, and don't construct -out: arguments to the linker from the former?

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

assigned to @ehsan

@rnk
Copy link
Collaborator

rnk commented Sep 10, 2014

I wanted to add support for -o, but at the time Alp Toker objected to it strongly. He wanted clang-cl to focus on MSVC compatibility and not try to be more than that. He felt that if users wanted Unix-style options, they should use the gcc-compatible driver. I haven't heard from him in months, so we could probably go ahead and land this change if we decide we don't agree. :)

We should also fix clang-cl to ignore linker inputs for the purposes of finding an output filename, especially because I don't think you need -o to have problems, clang-cl foo.lib bar.c will probably try to output to foo.lib. =/

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

I wanted to add support for -o, but at the time Alp Toker objected to it
strongly. He wanted clang-cl to focus on MSVC compatibility and not try to
be more than that. He felt that if users wanted Unix-style options, they
should use the gcc-compatible driver. I haven't heard from him in months, so
we could probably go ahead and land this change if we decide we don't agree.
:)

Hmm, cl.exe does support -o, so this in fact a cl compat issue. (Note that -o is deprecated and undocumented, but it's definitely supported.

We should also fix clang-cl to ignore linker inputs for the purposes of
finding an output filename, especially because I don't think you need -o to
have problems, clang-cl foo.lib bar.c will probably try to output to
foo.lib. =/

Yeah. The reason why that is that here we claim that cl.exe always uses the "base name" to construct the output https://github.com/llvm-mirror/clang/blame/master/lib/Driver/Driver.cpp#L1714 (which is false!). The base name comes from the first input https://github.com/llvm-mirror/clang/blame/master/lib/Driver/Driver.cpp#L1572. That is clearly broken, it should come from the first compiler input. But as far as I can tell, we treat all inputs equal here, and I don't know what will changing this break.

So, shall I go and write a patch for handling -o? I'm planning to make it kick in when -Fo or -Fe are not specified, depending on the type of output.

@timurrrr
Copy link
Contributor

Hmm, cl.exe does support -o

What makes you believe so?
I think they've actually dropped support for -o a few versions ago...

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

Hmm, cl.exe does support -o

What makes you believe so?

Testing cl.exe's behavior. :-)

$ cat test.cpp
int main() { return 1; }

$ cl -o myexe test.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 17.00.61030 for x86
Copyright (C) Microsoft Corporation. All rights reserved.

cl : Command line warning D9035 : option 'o' has been deprecated and will be removed in a future release
test.cpp
Microsoft (R) Incremental Linker Version 11.00.61030.0
Copyright (C) Microsoft Corporation. All rights reserved.

/out:test.exe
/out:myexe.exe
test.obj

$ ./myexe.exe

$ echo $?
1

I think they've actually dropped support for -o a few versions ago...

I think there is some confusion here because this flag is undocumented.

@rnk
Copy link
Collaborator

rnk commented Sep 10, 2014

I wanted to add support for -o, but at the time Alp Toker objected to it
strongly. He wanted clang-cl to focus on MSVC compatibility and not try to
be more than that. He felt that if users wanted Unix-style options, they
should use the gcc-compatible driver. I haven't heard from him in months, so
we could probably go ahead and land this change if we decide we don't agree.
:)

Hmm, cl.exe does support -o, so this in fact a cl compat issue. (Note
that -o is deprecated and undocumented, but it's definitely supported.

Last time I checked, -o is recognized, but it has no effect on where object files get written. Object files always follow -Fo or basename of the first input.

I haven't checked if -o affects the location of executables. If -o affects executables, then we should just add it and support it for object files.

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

I wanted to add support for -o, but at the time Alp Toker objected to it
strongly. He wanted clang-cl to focus on MSVC compatibility and not try to
be more than that. He felt that if users wanted Unix-style options, they
should use the gcc-compatible driver. I haven't heard from him in months, so
we could probably go ahead and land this change if we decide we don't agree.
:)

Hmm, cl.exe does support -o, so this in fact a cl compat issue. (Note
that -o is deprecated and undocumented, but it's definitely supported.

Last time I checked, -o is recognized, but it has no effect on where object
files get written. Object files always follow -Fo or basename of the first
input.

You're right!

I haven't checked if -o affects the location of executables. If -o affects
executables, then we should just add it and support it for object files.

It seems like it only affects the location of executables. See comment 4. I'll submit a patch for this soon.

@timurrrr
Copy link
Contributor

I think we should support -o for both object files and executables or for neither of them. I don't see any reason to follow the CL behaviour given that -o is explicitly deprecated there.

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

I think we should support -o for both object files and executables or for
neither of them. I don't see any reason to follow the CL behaviour given
that -o is explicitly deprecated there.

I could be convinced either way, since the thing that I really need is specifying executable locations. Reid, what do you think?

@rnk
Copy link
Collaborator

rnk commented Sep 10, 2014

I think we should support -o for both object files and executables or for
neither of them. I don't see any reason to follow the CL behaviour given
that -o is explicitly deprecated there.

I agree. I think we should take the option of supporting it for all outputs: preprocessed source, object files, and executables.

Unfortunately I seem to recall that we asserted that there could be no conflicting -o option when implementing -Fo. I think the last flag should win if both are supplied.

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

I think we should support -o for both object files and executables or for
neither of them. I don't see any reason to follow the CL behaviour given
that -o is explicitly deprecated there.

I agree. I think we should take the option of supporting it for all outputs:
preprocessed source, object files, and executables.

OK, will do.

Unfortunately I seem to recall that we asserted that there could be no
conflicting -o option when implementing -Fo. I think the last flag should
win if both are supplied.

Hah! I just tested this and indeed MSVC favors the last argument here. Can you please show me how I can get the last argument from a set of two? All I know how to do is to get the last argument of a specific type...

@zmodem
Copy link
Collaborator

zmodem commented Sep 10, 2014

Hah! I just tested this and indeed MSVC favors the last argument here. Can
you please show me how I can get the last argument from a set of two? All I
know how to do is to get the last argument of a specific type...

ArgList::getLastArg has overloads that take multiple flag types.

@ehsan
Copy link
Contributor Author

ehsan commented Sep 10, 2014

Hah! I just tested this and indeed MSVC favors the last argument here. Can
you please show me how I can get the last argument from a set of two? All I
know how to do is to get the last argument of a specific type...

ArgList::getLastArg has overloads that take multiple flag types.

Perfect!

@ehsan
Copy link
Contributor Author

ehsan commented Sep 11, 2014

Submitted a patch here: http://reviews.llvm.org/D5308

@nico
Copy link
Contributor

nico commented Sep 11, 2014

Hmm, I kind of agree with Alp here. Why do you want to use clang-cl with autoconf instead of regular clang?

@ehsan
Copy link
Contributor Author

ehsan commented Sep 11, 2014

Hmm, I kind of agree with Alp here. Why do you want to use clang-cl with
autoconf instead of regular clang?

Because it is not practical to port the entire Windows build system for large projects that use autoconf to configure to clang. Mozilla's build system for example has been tailored to use cl.exe. clang-cl strives to be a drop-in replacement for cl.exe, what's wrong with adding support for the options that cl.exe supports?

@timurrrr
Copy link
Contributor

I'd argue your autoconf needs to be changed anyways:

cl : Command line warning D9035 : option 'o' has been deprecated and will be removed in a future release

@ehsan
Copy link
Contributor Author

ehsan commented Sep 11, 2014

I'd argue your autoconf needs to be changed anyways:

cl : Command line warning D9035 : option 'o' has been deprecated and will be removed in a future release

Sure, if Microsoft drops this in the future, we'll need to have another solution. If and when that happens, we can also modify clang-cl to not accept /o if -fmsc-version indicates compat with the said possible future version.

@zmodem
Copy link
Collaborator

zmodem commented Sep 11, 2014

I too think clang-cl should support this.

The fact that cl.exe supports it (including in the 14 CTP despite the deprecation warning) and that real-world projects rely on it seem like enough motivation.

@nico
Copy link
Contributor

nico commented Sep 11, 2014

I'd argue your autoconf needs to be changed anyways:

cl : Command line warning D9035 : option 'o' has been deprecated and will be removed in a future release

Sure, if Microsoft drops this in the future, we'll need to have another
solution. If and when that happens, we can also modify clang-cl to not
accept /o if -fmsc-version indicates compat with the said possible future
version.

Ok, that sounds fair.

@ehsan
Copy link
Contributor Author

ehsan commented Sep 11, 2014

Fixed in r217615.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Development

No branches or pull requests

5 participants