Skip to content

Commit c86ef59

Browse files
JDevlieghereJDevlieghere
committed
[dsymutil] Remove paper trail warnings (llvm#67041)
Remove the paper trail warning from dsymutil and the DWARF linker. The original purpose of this functionality was to leave a paper trail in the output artifact about missing object files. The current implementation however has diverged and is the source of a pretty serious bug: - In the debug map parser, missing object files are not the only warnings we emit. When paper trail warnings are enabled, all of them end up in the dSYM which wasn't the goal. - When warnings are associated with a object file in the debug map, it is skipped by the DWARF linker. This only makes sense if the object file is missing and is obviously incorrect for any other type of warning (such as a missing symbol). The combination of the two means that we can generate broken DWARF when the feature is enabled. AFAIK it was only used by Apple and nobody I spoke to has relied on it, so rather than fixing the broken behavior I propose we remove it. (cherry picked from commit 697b34f)
1 parent 0db3f71 commit c86ef59

File tree

14 files changed

+14
-180
lines changed

14 files changed

+14
-180
lines changed

‎llvm/docs/CommandGuide/dsymutil.rst

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,6 @@ OPTIONS
100100
Specifies an alternate ``path`` to place the dSYM bundle. The default dSYM
101101
bundle path is created by appending ``.dSYM`` to the executable name.
102102

103-
.. option:: --papertrail
104-
105-
When running dsymutil as part of your build system, it can be desirable for
106-
warnings to be part of the end product, rather than just being emitted to the
107-
output stream. When enabled warnings are embedded in the linked DWARF debug
108-
information.
109-
110103
.. option:: --remarks-drop-without-debug
111104

112105
Drop remarks without valid debug locations. Without this flags, all remarks are kept.

‎llvm/include/llvm/DWARFLinker/DWARFLinker.h

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ class DwarfEmitter {
9999
public:
100100
virtual ~DwarfEmitter();
101101

102-
/// Emit DIE containing warnings.
103-
virtual void emitPaperTrailWarningsDie(DIE &Die) = 0;
104-
105102
/// Emit section named SecName with data SecData.
106103
virtual void emitSectionContents(StringRef SecData, StringRef SecName) = 0;
107104

@@ -267,10 +264,9 @@ using UnitListTy = std::vector<std::unique_ptr<CompileUnit>>;
267264
class DWARFFile {
268265
public:
269266
DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
270-
std::unique_ptr<AddressesMap> Addresses,
271-
const std::vector<std::string> &Warnings)
267+
std::unique_ptr<AddressesMap> Addresses)
272268
: FileName(Name), Dwarf(std::move(Dwarf)),
273-
Addresses(std::move(Addresses)), Warnings(Warnings) {}
269+
Addresses(std::move(Addresses)) {}
274270

275271
/// The object file name.
276272
StringRef FileName;
@@ -280,9 +276,6 @@ class DWARFFile {
280276

281277
/// Helpful address information(list of valid address ranges, relocations).
282278
std::unique_ptr<AddressesMap> Addresses;
283-
284-
/// Warnings for this object file.
285-
const std::vector<std::string> &Warnings;
286279
};
287280

288281
typedef std::map<std::string, std::string> swiftInterfacesMap;
@@ -502,10 +495,6 @@ class DWARFLinker {
502495
ErrorHandler(Warning, File.FileName, DIE);
503496
}
504497

505-
/// Emit warnings as Dwarf compile units to leave a trail after linking.
506-
bool emitPaperTrailWarnings(const DWARFFile &File,
507-
OffsetsStringPool &StringPool);
508-
509498
void copyInvariantDebugSection(DWARFContext &Dwarf);
510499

511500
/// Keep information for referenced clang module: already loaded DWARF info

‎llvm/include/llvm/DWARFLinker/DWARFStreamer.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ class DwarfStreamer : public DwarfEmitter {
7272
void emitAbbrevs(const std::vector<std::unique_ptr<DIEAbbrev>> &Abbrevs,
7373
unsigned DwarfVersion) override;
7474

75-
/// Emit DIE containing warnings.
76-
void emitPaperTrailWarningsDie(DIE &Die) override;
77-
7875
/// Emit contents of section SecName From Obj.
7976
void emitSectionContents(StringRef SecData, StringRef SecName) override;
8077

‎llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ class DWARFFile {
2929

3030
DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
3131
std::unique_ptr<AddressesMap> Addresses,
32-
const std::vector<std::string> &Warnings,
3332
UnloadCallbackTy UnloadFunc = nullptr)
3433
: FileName(Name), Dwarf(std::move(Dwarf)),
35-
Addresses(std::move(Addresses)), Warnings(Warnings),
36-
UnloadFunc(UnloadFunc) {
34+
Addresses(std::move(Addresses)), UnloadFunc(UnloadFunc) {
3735
if (this->Dwarf)
3836
Endianess = this->Dwarf->isLittleEndian() ? support::endianness::little
3937
: support::endianness::big;
@@ -48,9 +46,6 @@ class DWARFFile {
4846
/// Helpful address information(list of valid address ranges, relocations).
4947
std::unique_ptr<AddressesMap> Addresses;
5048

51-
/// Warnings for object file.
52-
const std::vector<std::string> &Warnings;
53-
5449
/// Endiannes of source DWARF information.
5550
support::endianness Endianess = support::endianness::little;
5651

‎llvm/lib/DWARFLinker/DWARFLinker.cpp

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2561,69 +2561,6 @@ uint64_t DWARFLinker::DIECloner::cloneAllCompileUnits(
25612561
return OutputDebugInfoSize - StartOutputDebugInfoSize;
25622562
}
25632563

2564-
bool DWARFLinker::emitPaperTrailWarnings(const DWARFFile &File,
2565-
OffsetsStringPool &StringPool) {
2566-
2567-
if (File.Warnings.empty())
2568-
return false;
2569-
2570-
DIE *CUDie = DIE::get(DIEAlloc, dwarf::DW_TAG_compile_unit);
2571-
CUDie->setOffset(11);
2572-
StringRef Producer;
2573-
StringRef WarningHeader;
2574-
2575-
switch (DwarfLinkerClientID) {
2576-
case DwarfLinkerClient::Dsymutil:
2577-
Producer = StringPool.internString("dsymutil");
2578-
WarningHeader = "dsymutil_warning";
2579-
break;
2580-
2581-
default:
2582-
Producer = StringPool.internString("dwarfopt");
2583-
WarningHeader = "dwarfopt_warning";
2584-
break;
2585-
}
2586-
2587-
StringRef FileName = StringPool.internString(File.FileName);
2588-
CUDie->addValue(DIEAlloc, dwarf::DW_AT_producer, dwarf::DW_FORM_strp,
2589-
DIEInteger(StringPool.getStringOffset(Producer)));
2590-
DIEBlock *String = new (DIEAlloc) DIEBlock();
2591-
DIEBlocks.push_back(String);
2592-
for (auto &C : FileName)
2593-
String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
2594-
DIEInteger(C));
2595-
String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
2596-
DIEInteger(0));
2597-
2598-
CUDie->addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_string, String);
2599-
for (const auto &Warning : File.Warnings) {
2600-
DIE &ConstDie = CUDie->addChild(DIE::get(DIEAlloc, dwarf::DW_TAG_constant));
2601-
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_strp,
2602-
DIEInteger(StringPool.getStringOffset(WarningHeader)));
2603-
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_artificial, dwarf::DW_FORM_flag,
2604-
DIEInteger(1));
2605-
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_const_value, dwarf::DW_FORM_strp,
2606-
DIEInteger(StringPool.getStringOffset(Warning)));
2607-
}
2608-
unsigned Size = 4 /* FORM_strp */ + FileName.size() + 1 +
2609-
File.Warnings.size() * (4 + 1 + 4) + 1 /* End of children */;
2610-
DIEAbbrev Abbrev = CUDie->generateAbbrev();
2611-
assignAbbrev(Abbrev);
2612-
CUDie->setAbbrevNumber(Abbrev.getNumber());
2613-
Size += getULEB128Size(Abbrev.getNumber());
2614-
// Abbreviation ordering needed for classic compatibility.
2615-
for (auto &Child : CUDie->children()) {
2616-
Abbrev = Child.generateAbbrev();
2617-
assignAbbrev(Abbrev);
2618-
Child.setAbbrevNumber(Abbrev.getNumber());
2619-
Size += getULEB128Size(Abbrev.getNumber());
2620-
}
2621-
CUDie->setSize(Size);
2622-
TheDwarfEmitter->emitPaperTrailWarningsDie(*CUDie);
2623-
2624-
return true;
2625-
}
2626-
26272564
void DWARFLinker::copyInvariantDebugSection(DWARFContext &Dwarf) {
26282565
TheDwarfEmitter->emitSectionContents(Dwarf.getDWARFObj().getLocSection().Data,
26292566
"debug_loc");
@@ -2687,9 +2624,6 @@ Error DWARFLinker::link() {
26872624
outs() << "OBJECT FILE: " << OptContext.File.FileName << "\n";
26882625
}
26892626

2690-
if (emitPaperTrailWarnings(OptContext.File, DebugStrPool))
2691-
continue;
2692-
26932627
if (!OptContext.File.Dwarf)
26942628
continue;
26952629

‎llvm/lib/DWARFLinker/DWARFStreamer.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -234,18 +234,6 @@ void DwarfStreamer::emitSectionContents(StringRef SecData, StringRef SecName) {
234234
}
235235
}
236236

237-
/// Emit DIE containing warnings.
238-
void DwarfStreamer::emitPaperTrailWarningsDie(DIE &Die) {
239-
switchToDebugInfoSection(/* Version */ 2);
240-
auto &Asm = getAsmPrinter();
241-
Asm.emitInt32(11 + Die.getSize() - 4);
242-
Asm.emitInt16(2);
243-
Asm.emitInt32(0);
244-
Asm.emitInt8(MC->getTargetTriple().isArch64Bit() ? 8 : 4);
245-
DebugInfoSectionSize += 11;
246-
emitDIE(Die);
247-
}
248-
249237
/// Emit the debug_str section stored in \p Pool.
250238
void DwarfStreamer::emitStrings(const NonRelocatableStringpool &Pool) {
251239
Asm->OutStreamer->switchSection(MOFI->getDwarfStrSection());

‎llvm/test/tools/dsymutil/X86/papertrail-warnings.test

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎llvm/test/tools/dsymutil/cmdline.test

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ CHECK: -object-prefix-map <prefix=remapped>
2121
CHECK: -oso-prepend-path <path>
2222
CHECK: -out <filename>
2323
CHECK: {{-o <filename>}}
24-
CHECK: -papertrail
2524
CHECK: -remarks-drop-without-debug
2625
CHECK: -remarks-output-format <format>
2726
CHECK: -remarks-prepend-path <path>

‎llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,7 @@ DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
234234
if (ErrorOrObj) {
235235
Res = std::make_unique<OutDWARFFile>(
236236
Obj.getObjectFilename(), DWARFContext::create(*ErrorOrObj),
237-
std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj),
238-
Obj.empty() ? Obj.getWarnings() : EmptyWarnings);
237+
std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj));
239238

240239
Error E = RL.link(*ErrorOrObj);
241240
if (Error NewE = handleErrors(
@@ -768,8 +767,7 @@ bool DwarfLinkerForBinary::linkImpl(
768767
OnCUDieLoaded);
769768
} else {
770769
ObjectsForLinking.push_back(std::make_unique<OutDwarfFile>(
771-
Obj->getObjectFilename(), nullptr, nullptr,
772-
Obj->empty() ? Obj->getWarnings() : EmptyWarnings));
770+
Obj->getObjectFilename(), nullptr, nullptr));
773771
GeneralLinker->addObjectFile(*ObjectsForLinking.back());
774772
}
775773
}

‎llvm/tools/dsymutil/MachODebugMapParser.cpp

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,9 @@ class MachODebugMapParser {
2828
public:
2929
MachODebugMapParser(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
3030
StringRef BinaryPath, ArrayRef<std::string> Archs,
31-
StringRef PathPrefix = "",
32-
bool PaperTrailWarnings = false, bool Verbose = false)
31+
StringRef PathPrefix = "", bool Verbose = false)
3332
: BinaryPath(std::string(BinaryPath)), Archs(Archs.begin(), Archs.end()),
34-
PathPrefix(std::string(PathPrefix)),
35-
PaperTrailWarnings(PaperTrailWarnings), BinHolder(VFS, Verbose),
33+
PathPrefix(std::string(PathPrefix)), BinHolder(VFS, Verbose),
3634
CurrentDebugMapObject(nullptr), SkipDebugMapObject(false) {}
3735

3836
/// Parses and returns the DebugMaps of the input binary. The binary contains
@@ -50,7 +48,6 @@ class MachODebugMapParser {
5048
std::string BinaryPath;
5149
SmallVector<StringRef, 1> Archs;
5250
std::string PathPrefix;
53-
bool PaperTrailWarnings;
5451

5552
/// Owns the MemoryBuffer for the main binary.
5653
BinaryHolder BinHolder;
@@ -137,13 +134,6 @@ class MachODebugMapParser {
137134
<< MachOUtils::getArchName(
138135
Result->getTriple().getArchName())
139136
<< ") " << File << " " << Msg << "\n";
140-
141-
if (PaperTrailWarnings) {
142-
if (!File.empty())
143-
Result->addDebugMapObject(File, sys::TimePoint<std::chrono::seconds>());
144-
if (Result->end() != Result->begin())
145-
(*--Result->end())->addWarning(Msg.str());
146-
}
147137
}
148138
};
149139

@@ -704,13 +694,11 @@ namespace dsymutil {
704694
llvm::ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
705695
parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
706696
StringRef InputFile, ArrayRef<std::string> Archs,
707-
StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
708-
bool InputIsYAML) {
697+
StringRef PrependPath, bool Verbose, bool InputIsYAML) {
709698
if (InputIsYAML)
710699
return DebugMap::parseYAMLDebugMap(InputFile, PrependPath, Verbose);
711700

712-
MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath,
713-
PaperTrailWarnings, Verbose);
701+
MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath, Verbose);
714702
return Parser.parse();
715703
}
716704

‎llvm/tools/dsymutil/Options.td

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,6 @@ def yaml_input: Flag<["-", "--"], "y">,
6969
HelpText<"Treat the input file is a YAML debug map rather than a binary.">,
7070
Group<grp_general>;
7171

72-
def papertrail: F<"papertrail">,
73-
HelpText<"Embed warnings in the linked DWARF debug info.">,
74-
Group<grp_general>;
75-
7672
def assembly: Flag<["-", "--"], "S">,
7773
HelpText<"Output textual assembly instead of a binary dSYM companion file.">,
7874
Group<grp_general>;

‎llvm/tools/dsymutil/dsymutil.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ struct DsymutilOptions {
106106
bool DumpStab = false;
107107
bool Flat = false;
108108
bool InputIsYAMLDebugMap = false;
109-
bool PaperTrailWarnings = false;
110109
bool ForceKeepFunctionForStatic = false;
111110
std::string SymbolMap;
112111
std::string OutputFile;
@@ -197,11 +196,6 @@ static Error verifyOptions(const DsymutilOptions &Options) {
197196
"cannot use -o with multiple inputs in flat mode.",
198197
errc::invalid_argument);
199198

200-
if (Options.PaperTrailWarnings && Options.InputIsYAMLDebugMap)
201-
return make_error<StringError>(
202-
"paper trail warnings are not supported for YAML input.",
203-
errc::invalid_argument);
204-
205199
if (!Options.ReproducerPath.empty() &&
206200
Options.ReproMode != ReproducerMode::Use)
207201
return make_error<StringError>(
@@ -303,7 +297,6 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
303297
Options.DumpStab = Args.hasArg(OPT_symtab);
304298
Options.Flat = Args.hasArg(OPT_flat);
305299
Options.InputIsYAMLDebugMap = Args.hasArg(OPT_yaml_input);
306-
Options.PaperTrailWarnings = Args.hasArg(OPT_papertrail);
307300

308301
if (Expected<DWARFVerify> Verify = getVerifyKind(Args)) {
309302
Options.Verify = *Verify;
@@ -389,9 +382,6 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
389382
if (Options.DumpDebugMap || Options.LinkOpts.Verbose)
390383
Options.LinkOpts.Threads = 1;
391384

392-
if (getenv("RC_DEBUG_OPTIONS"))
393-
Options.PaperTrailWarnings = true;
394-
395385
if (opt::Arg *RemarksPrependPath = Args.getLastArg(OPT_remarks_prepend_path))
396386
Options.LinkOpts.RemarksPrependPath = RemarksPrependPath->getValue();
397387

@@ -671,8 +661,8 @@ int main(int argc, char **argv) {
671661

672662
auto DebugMapPtrsOrErr =
673663
parseDebugMap(Options.LinkOpts.VFS, InputFile, Options.Archs,
674-
Options.LinkOpts.PrependPath, Options.PaperTrailWarnings,
675-
Options.LinkOpts.Verbose, Options.InputIsYAMLDebugMap);
664+
Options.LinkOpts.PrependPath, Options.LinkOpts.Verbose,
665+
Options.InputIsYAMLDebugMap);
676666

677667
if (auto EC = DebugMapPtrsOrErr.getError()) {
678668
WithColor::error() << "cannot parse the debug map for '" << InputFile

‎llvm/tools/dsymutil/dsymutil.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ class BinaryHolder;
3737
ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
3838
parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
3939
StringRef InputFile, ArrayRef<std::string> Archs,
40-
StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
41-
bool InputIsYAML);
40+
StringRef PrependPath, bool Verbose, bool InputIsYAML);
4241

4342
/// Dump the symbol table.
4443
bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,

‎llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,17 +322,15 @@ Error linkDebugInfoImpl(object::ObjectFile &File, const Options &Options,
322322
DebugInfoLinker->setUpdateIndexTablesOnly(!Options.DoGarbageCollection);
323323

324324
std::vector<std::unique_ptr<OutDwarfFile>> ObjectsForLinking(1);
325-
std::vector<std::string> EmptyWarnings;
326325

327326
// Add object files to the DWARFLinker.
328327
std::unique_ptr<DWARFContext> Context = DWARFContext::create(File);
329328
std::unique_ptr<ObjFileAddressMap<AddressMapBase>> AddressesMap(
330329
std::make_unique<ObjFileAddressMap<AddressMapBase>>(*Context, Options,
331330
File));
332331

333-
ObjectsForLinking[0] =
334-
std::make_unique<OutDwarfFile>(File.getFileName(), std::move(Context),
335-
std::move(AddressesMap), EmptyWarnings);
332+
ObjectsForLinking[0] = std::make_unique<OutDwarfFile>(
333+
File.getFileName(), std::move(Context), std::move(AddressesMap));
336334

337335
uint16_t MaxDWARFVersion = 0;
338336
std::function<void(const DWARFUnit &Unit)> OnCUDieLoaded =

0 commit comments

Comments
 (0)