Re: r350970 - [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via cfe-commits]

r352537 should fix it.

Steven

> On Jan 29, 2019, at 11:36 AM, Matt Arsenault  wrote:
> 
> 
> 
>> On Jan 11, 2019, at 4:16 PM, Steven Wu via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>> Author: steven_wu
>> Date: Fri Jan 11 13:16:04 2019
>> New Revision: 350970
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=350970=rev 
>> 
>> Log:
>> [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
>> 
>> Summary:
>> After r327851, Driver::GetTemporaryPath will create the file rather than
>> just create a potientially unqine filename. If clang driver pass the
>> file as parameter as -object_path_lto, ld64 will pass it back to libLTO
>> as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
>> is not a directory.
>> Now during thinLTO, pass a temp directory path to linker instread.
>> 
>> rdar://problem/47194182 
>> 
>> Reviewers: arphaman, dexonsmith
>> 
>> Reviewed By: arphaman
>> 
>> Subscribers: mehdi_amini, inglorion, jkorous, cfe-commits
>> 
>> Differential Revision: https://reviews.llvm.org/D56608
>> 
>> Modified:
>>cfe/trunk/include/clang/Driver/Driver.h
>>cfe/trunk/lib/Driver/Driver.cpp
>>cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>>cfe/trunk/test/Driver/darwin-ld-lto.c
>> 
>> Modified: cfe/trunk/include/clang/Driver/Driver.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=350970=350969=350970=diff
>> ==
>> --- cfe/trunk/include/clang/Driver/Driver.h (original)
>> +++ cfe/trunk/include/clang/Driver/Driver.h Fri Jan 11 13:16:04 2019
>> @@ -505,6 +505,10 @@ public:
>>   /// GCC goes to extra lengths here to be a bit more robust.
>>   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
>> 
>> +  /// GetTemporaryDirectory - Return the pathname of a temporary directory 
>> to
>> +  /// use as part of compilation; the directory will have the given prefix.
>> +  std::string GetTemporaryDirectory(StringRef Prefix) const;
>> +
>>   /// Return the pathname of the pch file in clang-cl mode.
>>   std::string GetClPchPath(Compilation , StringRef BaseName) const;
>> 
>> 
>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=350970=350969=350970=diff
>> ==
>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Jan 11 13:16:04 2019
>> @@ -4478,6 +4478,17 @@ std::string Driver::GetTemporaryPath(Str
>>   return Path.str();
>> }
>> 
>> +std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
>> +  SmallString<128> Path;
>> +  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
>> +  if (EC) {
>> +Diag(clang::diag::err_unable_to_make_temp) << EC.message();
>> +return "";
>> +  }
>> +
>> +  return Path.str();
>> +}
>> +
>> std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
>>   SmallString<128> Output;
>>   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
>> 
>> Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=350970=350969=350970=diff
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Jan 11 13:16:04 2019
>> @@ -224,13 +224,20 @@ void darwin::Linker::AddLinkArgs(Compila
>>options::OPT_fno_application_extension, false))
>> CmdArgs.push_back("-application_extension");
>> 
>> -  if (D.isUsingLTO()) {
>> -// If we are using LTO, then automatically create a temporary file path 
>> for
>> -// the linker to use, so that it's lifetime will extend past a possible
>> -// dsymutil step.
>> -if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
>> -  const char *TmpPath = C.getArgs().MakeArgString(
>> -  D.GetTemporaryPath("cc", 
>> types::getTypeTempSuffix(types::TY_Object)));
>> +  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
>> +std::string TmpPathName;
>> +if (D.getLTOMode() == LTOK_Full) {
>> +  // If we are using full LTO, then automatically create a temporary 
>> file
>> +  // path for the linker to use, so that it's lifetime will extend past 
>> a
>> +  // possible dsymutil step.
>> +  TmpPathName =
>> +  D.GetTemporaryPath("cc", 
>> types::getTypeTempSuffix(types::TY_Object));
>> +} else if (D.getLTOMode() == LTOK_Thin)
>> +  // If we are using thin LTO, then create a directory instead.
>> +  TmpPathName = D.GetTemporaryDirectory("thinlto");
>> +
>> +if (!TmpPathName.empty()) {
>> +  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
>> 

Re: r350970 - [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Matt Arsenault via cfe-commits]

> On Jan 11, 2019, at 4:16 PM, Steven Wu via cfe-commits 
>  wrote:
> 
> Author: steven_wu
> Date: Fri Jan 11 13:16:04 2019
> New Revision: 350970
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=350970=rev
> Log:
> [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
> 
> Summary:
> After r327851, Driver::GetTemporaryPath will create the file rather than
> just create a potientially unqine filename. If clang driver pass the
> file as parameter as -object_path_lto, ld64 will pass it back to libLTO
> as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
> is not a directory.
> Now during thinLTO, pass a temp directory path to linker instread.
> 
> rdar://problem/47194182
> 
> Reviewers: arphaman, dexonsmith
> 
> Reviewed By: arphaman
> 
> Subscribers: mehdi_amini, inglorion, jkorous, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D56608
> 
> Modified:
>cfe/trunk/include/clang/Driver/Driver.h
>cfe/trunk/lib/Driver/Driver.cpp
>cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
>cfe/trunk/test/Driver/darwin-ld-lto.c
> 
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=350970=350969=350970=diff
> ==
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Fri Jan 11 13:16:04 2019
> @@ -505,6 +505,10 @@ public:
>   /// GCC goes to extra lengths here to be a bit more robust.
>   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
> 
> +  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
> +  /// use as part of compilation; the directory will have the given prefix.
> +  std::string GetTemporaryDirectory(StringRef Prefix) const;
> +
>   /// Return the pathname of the pch file in clang-cl mode.
>   std::string GetClPchPath(Compilation , StringRef BaseName) const;
> 
> 
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=350970=350969=350970=diff
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Jan 11 13:16:04 2019
> @@ -4478,6 +4478,17 @@ std::string Driver::GetTemporaryPath(Str
>   return Path.str();
> }
> 
> +std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
> +  SmallString<128> Path;
> +  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
> +  if (EC) {
> +Diag(clang::diag::err_unable_to_make_temp) << EC.message();
> +return "";
> +  }
> +
> +  return Path.str();
> +}
> +
> std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
>   SmallString<128> Output;
>   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
> 
> Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=350970=350969=350970=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Jan 11 13:16:04 2019
> @@ -224,13 +224,20 @@ void darwin::Linker::AddLinkArgs(Compila
>options::OPT_fno_application_extension, false))
> CmdArgs.push_back("-application_extension");
> 
> -  if (D.isUsingLTO()) {
> -// If we are using LTO, then automatically create a temporary file path 
> for
> -// the linker to use, so that it's lifetime will extend past a possible
> -// dsymutil step.
> -if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
> -  const char *TmpPath = C.getArgs().MakeArgString(
> -  D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object)));
> +  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
> +std::string TmpPathName;
> +if (D.getLTOMode() == LTOK_Full) {
> +  // If we are using full LTO, then automatically create a temporary file
> +  // path for the linker to use, so that it's lifetime will extend past a
> +  // possible dsymutil step.
> +  TmpPathName =
> +  D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object));
> +} else if (D.getLTOMode() == LTOK_Thin)
> +  // If we are using thin LTO, then create a directory instead.
> +  TmpPathName = D.GetTemporaryDirectory("thinlto");
> +
> +if (!TmpPathName.empty()) {
> +  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
>   C.addTempFile(TmpPath);
>   CmdArgs.push_back("-object_path_lto");
>   CmdArgs.push_back(TmpPath);
> 
> Modified: cfe/trunk/test/Driver/darwin-ld-lto.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld-lto.c?rev=350970=350969=350970=diff
> 

r350970 - [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via cfe-commits]

Author: steven_wu
Date: Fri Jan 11 13:16:04 2019
New Revision: 350970

URL: http://llvm.org/viewvc/llvm-project?rev=350970=rev
Log:
[Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

Summary:
After r327851, Driver::GetTemporaryPath will create the file rather than
just create a potientially unqine filename. If clang driver pass the
file as parameter as -object_path_lto, ld64 will pass it back to libLTO
as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
is not a directory.
Now during thinLTO, pass a temp directory path to linker instread.

rdar://problem/47194182

Reviewers: arphaman, dexonsmith

Reviewed By: arphaman

Subscribers: mehdi_amini, inglorion, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D56608

Modified:
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld-lto.c

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=350970=350969=350970=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Fri Jan 11 13:16:04 2019
@@ -505,6 +505,10 @@ public:
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation , StringRef BaseName) const;
 

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=350970=350969=350970=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri Jan 11 13:16:04 2019
@@ -4478,6 +4478,17 @@ std::string Driver::GetTemporaryPath(Str
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=350970=350969=350970=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Jan 11 13:16:04 2019
@@ -224,13 +224,20 @@ void darwin::Linker::AddLinkArgs(Compila
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);

Modified: cfe/trunk/test/Driver/darwin-ld-lto.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld-lto.c?rev=350970=350969=350970=diff
==
--- cfe/trunk/test/Driver/darwin-ld-lto.c (original)
+++ cfe/trunk/test/Driver/darwin-ld-lto.c Fri Jan 11 13:16:04 2019
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir