Skip to content

Commit a04879c

Browse files
committed
[dsymutil] Use createTemporaryFile instead of TempFile
Use createTemporaryFile in favor of the TempFile abstraction to ensure we have an on-disk file. This fixes an issue on Windows where the DWARF verifier would fail when trying to open the temporary file from disk.
1 parent 1f929aa commit a04879c

File tree

3 files changed

+39
-23
lines changed

3 files changed

+39
-23
lines changed

llvm/tools/dsymutil/MachOUtils.cpp

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "BinaryHolder.h"
1111
#include "DebugMap.h"
1212
#include "LinkUtils.h"
13+
#include "llvm/ADT/SmallString.h"
1314
#include "llvm/CodeGen/NonRelocatableStringpool.h"
1415
#include "llvm/MC/MCAsmLayout.h"
1516
#include "llvm/MC/MCAssembler.h"
@@ -29,24 +30,30 @@ namespace dsymutil {
2930
namespace MachOUtils {
3031

3132
llvm::Error ArchAndFile::createTempFile() {
32-
llvm::SmallString<128> TmpModel;
33-
llvm::sys::path::system_temp_directory(true, TmpModel);
34-
llvm::sys::path::append(TmpModel, "dsym.tmp%%%%%.dwarf");
35-
Expected<sys::fs::TempFile> T = sys::fs::TempFile::create(TmpModel);
33+
SmallString<256> SS;
34+
std::error_code EC = sys::fs::createTemporaryFile("dsym", "dwarf", FD, SS);
3635

37-
if (!T)
38-
return T.takeError();
36+
if (EC)
37+
return errorCodeToError(EC);
38+
39+
Path = SS.str();
3940

40-
File = std::make_unique<sys::fs::TempFile>(std::move(*T));
4141
return Error::success();
4242
}
4343

44-
llvm::StringRef ArchAndFile::path() const { return File->TmpName; }
44+
llvm::StringRef ArchAndFile::getPath() const {
45+
assert(!Path.empty() && "path called before createTempFile");
46+
return Path;
47+
}
48+
49+
int ArchAndFile::getFD() const {
50+
assert((FD != -1) && "path called before createTempFile");
51+
return FD;
52+
}
4553

4654
ArchAndFile::~ArchAndFile() {
47-
if (File)
48-
if (auto E = File->discard())
49-
llvm::consumeError(std::move(E));
55+
if (!Path.empty())
56+
sys::fs::remove(Path);
5057
}
5158

5259
std::string getArchName(StringRef Arch) {
@@ -82,11 +89,17 @@ bool generateUniversalBinary(SmallVectorImpl<ArchAndFile> &ArchFiles,
8289
bool Fat64) {
8390
// No need to merge one file into a universal fat binary.
8491
if (ArchFiles.size() == 1) {
85-
if (auto E = ArchFiles.front().File->keep(OutputFileName)) {
86-
WithColor::error() << "while keeping " << ArchFiles.front().path()
87-
<< " as " << OutputFileName << ": "
88-
<< toString(std::move(E)) << "\n";
89-
return false;
92+
llvm::StringRef TmpPath = ArchFiles.front().getPath();
93+
if (auto EC = sys::fs::rename(TmpPath, OutputFileName)) {
94+
// If we can't rename, try to copy to work around cross-device link
95+
// issues.
96+
EC = sys::fs::copy_file(TmpPath, OutputFileName);
97+
if (EC) {
98+
WithColor::error() << "while keeping " << TmpPath << " as "
99+
<< OutputFileName << ": " << EC.message() << "\n";
100+
return false;
101+
}
102+
sys::fs::remove(TmpPath);
90103
}
91104
return true;
92105
}
@@ -96,7 +109,7 @@ bool generateUniversalBinary(SmallVectorImpl<ArchAndFile> &ArchFiles,
96109
Args.push_back("-create");
97110

98111
for (auto &Thin : ArchFiles)
99-
Args.push_back(Thin.path());
112+
Args.push_back(Thin.getPath());
100113

101114
// Align segments to match dsymutil-classic alignment.
102115
for (auto &Thin : ArchFiles) {

llvm/tools/dsymutil/MachOUtils.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ namespace MachOUtils {
2626

2727
struct ArchAndFile {
2828
std::string Arch;
29-
std::unique_ptr<llvm::sys::fs::TempFile> File;
29+
std::string Path;
30+
int FD = -1;
3031

3132
llvm::Error createTempFile();
32-
llvm::StringRef path() const;
33+
llvm::StringRef getPath() const;
34+
int getFD() const;
3335

3436
ArchAndFile(StringRef Arch) : Arch(std::string(Arch)) {}
3537
ArchAndFile(ArchAndFile &&A) = default;

llvm/tools/dsymutil/dsymutil.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -772,10 +772,10 @@ int main(int argc, char **argv) {
772772
return;
773773
}
774774

775-
auto &TempFile = *(TempFiles.back().File);
776-
OS = std::make_shared<raw_fd_ostream>(TempFile.FD,
775+
MachOUtils::ArchAndFile &AF = TempFiles.back();
776+
OS = std::make_shared<raw_fd_ostream>(AF.getFD(),
777777
/*shouldClose*/ false);
778-
OutputFile = TempFile.TmpName;
778+
OutputFile = AF.getPath();
779779
} else {
780780
std::error_code EC;
781781
OS = std::make_shared<raw_fd_ostream>(
@@ -843,7 +843,8 @@ int main(int argc, char **argv) {
843843
uint64_t FileOffset =
844844
MagicAndCountSize + UniversalArchInfoSize * TempFiles.size();
845845
for (const auto &File : TempFiles) {
846-
ErrorOr<vfs::Status> stat = Options.LinkOpts.VFS->status(File.path());
846+
ErrorOr<vfs::Status> stat =
847+
Options.LinkOpts.VFS->status(File.getPath());
847848
if (!stat)
848849
break;
849850
if (FileOffset > UINT32_MAX) {

0 commit comments

Comments
 (0)