Skip to content

[dsymutil] Remove paper trail warnings (#67041) #7511

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

Merged
merged 1 commit into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions llvm/docs/CommandGuide/dsymutil.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@ OPTIONS
Specifies an alternate ``path`` to place the dSYM bundle. The default dSYM
bundle path is created by appending ``.dSYM`` to the executable name.

.. option:: --papertrail

When running dsymutil as part of your build system, it can be desirable for
warnings to be part of the end product, rather than just being emitted to the
output stream. When enabled warnings are embedded in the linked DWARF debug
information.

.. option:: --remarks-drop-without-debug

Drop remarks without valid debug locations. Without this flags, all remarks are kept.
Expand Down
15 changes: 2 additions & 13 deletions llvm/include/llvm/DWARFLinker/DWARFLinker.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ class DwarfEmitter {
public:
virtual ~DwarfEmitter();

/// Emit DIE containing warnings.
virtual void emitPaperTrailWarningsDie(DIE &Die) = 0;

/// Emit section named SecName with data SecData.
virtual void emitSectionContents(StringRef SecData, StringRef SecName) = 0;

Expand Down Expand Up @@ -267,10 +264,9 @@ using UnitListTy = std::vector<std::unique_ptr<CompileUnit>>;
class DWARFFile {
public:
DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
std::unique_ptr<AddressesMap> Addresses,
const std::vector<std::string> &Warnings)
std::unique_ptr<AddressesMap> Addresses)
: FileName(Name), Dwarf(std::move(Dwarf)),
Addresses(std::move(Addresses)), Warnings(Warnings) {}
Addresses(std::move(Addresses)) {}

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

/// Helpful address information(list of valid address ranges, relocations).
std::unique_ptr<AddressesMap> Addresses;

/// Warnings for this object file.
const std::vector<std::string> &Warnings;
};

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

/// Emit warnings as Dwarf compile units to leave a trail after linking.
bool emitPaperTrailWarnings(const DWARFFile &File,
OffsetsStringPool &StringPool);

void copyInvariantDebugSection(DWARFContext &Dwarf);

/// Keep information for referenced clang module: already loaded DWARF info
Expand Down
3 changes: 0 additions & 3 deletions llvm/include/llvm/DWARFLinker/DWARFStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ class DwarfStreamer : public DwarfEmitter {
void emitAbbrevs(const std::vector<std::unique_ptr<DIEAbbrev>> &Abbrevs,
unsigned DwarfVersion) override;

/// Emit DIE containing warnings.
void emitPaperTrailWarningsDie(DIE &Die) override;

/// Emit contents of section SecName From Obj.
void emitSectionContents(StringRef SecData, StringRef SecName) override;

Expand Down
7 changes: 1 addition & 6 deletions llvm/include/llvm/DWARFLinkerParallel/DWARFFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@ class DWARFFile {

DWARFFile(StringRef Name, std::unique_ptr<DWARFContext> Dwarf,
std::unique_ptr<AddressesMap> Addresses,
const std::vector<std::string> &Warnings,
UnloadCallbackTy UnloadFunc = nullptr)
: FileName(Name), Dwarf(std::move(Dwarf)),
Addresses(std::move(Addresses)), Warnings(Warnings),
UnloadFunc(UnloadFunc) {
Addresses(std::move(Addresses)), UnloadFunc(UnloadFunc) {
if (this->Dwarf)
Endianess = this->Dwarf->isLittleEndian() ? support::endianness::little
: support::endianness::big;
Expand All @@ -48,9 +46,6 @@ class DWARFFile {
/// Helpful address information(list of valid address ranges, relocations).
std::unique_ptr<AddressesMap> Addresses;

/// Warnings for object file.
const std::vector<std::string> &Warnings;

/// Endiannes of source DWARF information.
support::endianness Endianess = support::endianness::little;

Expand Down
66 changes: 0 additions & 66 deletions llvm/lib/DWARFLinker/DWARFLinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2561,69 +2561,6 @@ uint64_t DWARFLinker::DIECloner::cloneAllCompileUnits(
return OutputDebugInfoSize - StartOutputDebugInfoSize;
}

bool DWARFLinker::emitPaperTrailWarnings(const DWARFFile &File,
OffsetsStringPool &StringPool) {

if (File.Warnings.empty())
return false;

DIE *CUDie = DIE::get(DIEAlloc, dwarf::DW_TAG_compile_unit);
CUDie->setOffset(11);
StringRef Producer;
StringRef WarningHeader;

switch (DwarfLinkerClientID) {
case DwarfLinkerClient::Dsymutil:
Producer = StringPool.internString("dsymutil");
WarningHeader = "dsymutil_warning";
break;

default:
Producer = StringPool.internString("dwarfopt");
WarningHeader = "dwarfopt_warning";
break;
}

StringRef FileName = StringPool.internString(File.FileName);
CUDie->addValue(DIEAlloc, dwarf::DW_AT_producer, dwarf::DW_FORM_strp,
DIEInteger(StringPool.getStringOffset(Producer)));
DIEBlock *String = new (DIEAlloc) DIEBlock();
DIEBlocks.push_back(String);
for (auto &C : FileName)
String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
DIEInteger(C));
String->addValue(DIEAlloc, dwarf::Attribute(0), dwarf::DW_FORM_data1,
DIEInteger(0));

CUDie->addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_string, String);
for (const auto &Warning : File.Warnings) {
DIE &ConstDie = CUDie->addChild(DIE::get(DIEAlloc, dwarf::DW_TAG_constant));
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_name, dwarf::DW_FORM_strp,
DIEInteger(StringPool.getStringOffset(WarningHeader)));
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_artificial, dwarf::DW_FORM_flag,
DIEInteger(1));
ConstDie.addValue(DIEAlloc, dwarf::DW_AT_const_value, dwarf::DW_FORM_strp,
DIEInteger(StringPool.getStringOffset(Warning)));
}
unsigned Size = 4 /* FORM_strp */ + FileName.size() + 1 +
File.Warnings.size() * (4 + 1 + 4) + 1 /* End of children */;
DIEAbbrev Abbrev = CUDie->generateAbbrev();
assignAbbrev(Abbrev);
CUDie->setAbbrevNumber(Abbrev.getNumber());
Size += getULEB128Size(Abbrev.getNumber());
// Abbreviation ordering needed for classic compatibility.
for (auto &Child : CUDie->children()) {
Abbrev = Child.generateAbbrev();
assignAbbrev(Abbrev);
Child.setAbbrevNumber(Abbrev.getNumber());
Size += getULEB128Size(Abbrev.getNumber());
}
CUDie->setSize(Size);
TheDwarfEmitter->emitPaperTrailWarningsDie(*CUDie);

return true;
}

void DWARFLinker::copyInvariantDebugSection(DWARFContext &Dwarf) {
TheDwarfEmitter->emitSectionContents(Dwarf.getDWARFObj().getLocSection().Data,
"debug_loc");
Expand Down Expand Up @@ -2687,9 +2624,6 @@ Error DWARFLinker::link() {
outs() << "OBJECT FILE: " << OptContext.File.FileName << "\n";
}

if (emitPaperTrailWarnings(OptContext.File, DebugStrPool))
continue;

if (!OptContext.File.Dwarf)
continue;

Expand Down
12 changes: 0 additions & 12 deletions llvm/lib/DWARFLinker/DWARFStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,6 @@ void DwarfStreamer::emitSectionContents(StringRef SecData, StringRef SecName) {
}
}

/// Emit DIE containing warnings.
void DwarfStreamer::emitPaperTrailWarningsDie(DIE &Die) {
switchToDebugInfoSection(/* Version */ 2);
auto &Asm = getAsmPrinter();
Asm.emitInt32(11 + Die.getSize() - 4);
Asm.emitInt16(2);
Asm.emitInt32(0);
Asm.emitInt8(MC->getTargetTriple().isArch64Bit() ? 8 : 4);
DebugInfoSectionSize += 11;
emitDIE(Die);
}

/// Emit the debug_str section stored in \p Pool.
void DwarfStreamer::emitStrings(const NonRelocatableStringpool &Pool) {
Asm->OutStreamer->switchSection(MOFI->getDwarfStrSection());
Expand Down
30 changes: 0 additions & 30 deletions llvm/test/tools/dsymutil/X86/papertrail-warnings.test

This file was deleted.

1 change: 0 additions & 1 deletion llvm/test/tools/dsymutil/cmdline.test
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ CHECK: -object-prefix-map <prefix=remapped>
CHECK: -oso-prepend-path <path>
CHECK: -out <filename>
CHECK: {{-o <filename>}}
CHECK: -papertrail
CHECK: -remarks-drop-without-debug
CHECK: -remarks-output-format <format>
CHECK: -remarks-prepend-path <path>
Expand Down
6 changes: 2 additions & 4 deletions llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ DwarfLinkerForBinary::loadObject(const DebugMapObject &Obj,
if (ErrorOrObj) {
Res = std::make_unique<OutDWARFFile>(
Obj.getObjectFilename(), DWARFContext::create(*ErrorOrObj),
std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj),
Obj.empty() ? Obj.getWarnings() : EmptyWarnings);
std::make_unique<AddressesMap>(*this, *ErrorOrObj, Obj));

Error E = RL.link(*ErrorOrObj);
if (Error NewE = handleErrors(
Expand Down Expand Up @@ -768,8 +767,7 @@ bool DwarfLinkerForBinary::linkImpl(
OnCUDieLoaded);
} else {
ObjectsForLinking.push_back(std::make_unique<OutDwarfFile>(
Obj->getObjectFilename(), nullptr, nullptr,
Obj->empty() ? Obj->getWarnings() : EmptyWarnings));
Obj->getObjectFilename(), nullptr, nullptr));
GeneralLinker->addObjectFile(*ObjectsForLinking.back());
}
}
Expand Down
20 changes: 4 additions & 16 deletions llvm/tools/dsymutil/MachODebugMapParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ class MachODebugMapParser {
public:
MachODebugMapParser(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
StringRef BinaryPath, ArrayRef<std::string> Archs,
StringRef PathPrefix = "",
bool PaperTrailWarnings = false, bool Verbose = false)
StringRef PathPrefix = "", bool Verbose = false)
: BinaryPath(std::string(BinaryPath)), Archs(Archs.begin(), Archs.end()),
PathPrefix(std::string(PathPrefix)),
PaperTrailWarnings(PaperTrailWarnings), BinHolder(VFS, Verbose),
PathPrefix(std::string(PathPrefix)), BinHolder(VFS, Verbose),
CurrentDebugMapObject(nullptr), SkipDebugMapObject(false) {}

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

/// Owns the MemoryBuffer for the main binary.
BinaryHolder BinHolder;
Expand Down Expand Up @@ -137,13 +134,6 @@ class MachODebugMapParser {
<< MachOUtils::getArchName(
Result->getTriple().getArchName())
<< ") " << File << " " << Msg << "\n";

if (PaperTrailWarnings) {
if (!File.empty())
Result->addDebugMapObject(File, sys::TimePoint<std::chrono::seconds>());
if (Result->end() != Result->begin())
(*--Result->end())->addWarning(Msg.str());
}
}
};

Expand Down Expand Up @@ -704,13 +694,11 @@ namespace dsymutil {
llvm::ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
StringRef InputFile, ArrayRef<std::string> Archs,
StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
bool InputIsYAML) {
StringRef PrependPath, bool Verbose, bool InputIsYAML) {
if (InputIsYAML)
return DebugMap::parseYAMLDebugMap(InputFile, PrependPath, Verbose);

MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath,
PaperTrailWarnings, Verbose);
MachODebugMapParser Parser(VFS, InputFile, Archs, PrependPath, Verbose);
return Parser.parse();
}

Expand Down
4 changes: 0 additions & 4 deletions llvm/tools/dsymutil/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ def yaml_input: Flag<["-", "--"], "y">,
HelpText<"Treat the input file is a YAML debug map rather than a binary.">,
Group<grp_general>;

def papertrail: F<"papertrail">,
HelpText<"Embed warnings in the linked DWARF debug info.">,
Group<grp_general>;

def assembly: Flag<["-", "--"], "S">,
HelpText<"Output textual assembly instead of a binary dSYM companion file.">,
Group<grp_general>;
Expand Down
14 changes: 2 additions & 12 deletions llvm/tools/dsymutil/dsymutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ struct DsymutilOptions {
bool DumpStab = false;
bool Flat = false;
bool InputIsYAMLDebugMap = false;
bool PaperTrailWarnings = false;
bool ForceKeepFunctionForStatic = false;
std::string SymbolMap;
std::string OutputFile;
Expand Down Expand Up @@ -197,11 +196,6 @@ static Error verifyOptions(const DsymutilOptions &Options) {
"cannot use -o with multiple inputs in flat mode.",
errc::invalid_argument);

if (Options.PaperTrailWarnings && Options.InputIsYAMLDebugMap)
return make_error<StringError>(
"paper trail warnings are not supported for YAML input.",
errc::invalid_argument);

if (!Options.ReproducerPath.empty() &&
Options.ReproMode != ReproducerMode::Use)
return make_error<StringError>(
Expand Down Expand Up @@ -303,7 +297,6 @@ static Expected<DsymutilOptions> getOptions(opt::InputArgList &Args) {
Options.DumpStab = Args.hasArg(OPT_symtab);
Options.Flat = Args.hasArg(OPT_flat);
Options.InputIsYAMLDebugMap = Args.hasArg(OPT_yaml_input);
Options.PaperTrailWarnings = Args.hasArg(OPT_papertrail);

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

if (getenv("RC_DEBUG_OPTIONS"))
Options.PaperTrailWarnings = true;

if (opt::Arg *RemarksPrependPath = Args.getLastArg(OPT_remarks_prepend_path))
Options.LinkOpts.RemarksPrependPath = RemarksPrependPath->getValue();

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

auto DebugMapPtrsOrErr =
parseDebugMap(Options.LinkOpts.VFS, InputFile, Options.Archs,
Options.LinkOpts.PrependPath, Options.PaperTrailWarnings,
Options.LinkOpts.Verbose, Options.InputIsYAMLDebugMap);
Options.LinkOpts.PrependPath, Options.LinkOpts.Verbose,
Options.InputIsYAMLDebugMap);

if (auto EC = DebugMapPtrsOrErr.getError()) {
WithColor::error() << "cannot parse the debug map for '" << InputFile
Expand Down
3 changes: 1 addition & 2 deletions llvm/tools/dsymutil/dsymutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class BinaryHolder;
ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
parseDebugMap(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
StringRef InputFile, ArrayRef<std::string> Archs,
StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
bool InputIsYAML);
StringRef PrependPath, bool Verbose, bool InputIsYAML);

/// Dump the symbol table.
bool dumpStab(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
Expand Down
6 changes: 2 additions & 4 deletions llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,17 +322,15 @@ Error linkDebugInfoImpl(object::ObjectFile &File, const Options &Options,
DebugInfoLinker->setUpdateIndexTablesOnly(!Options.DoGarbageCollection);

std::vector<std::unique_ptr<OutDwarfFile>> ObjectsForLinking(1);
std::vector<std::string> EmptyWarnings;

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

ObjectsForLinking[0] =
std::make_unique<OutDwarfFile>(File.getFileName(), std::move(Context),
std::move(AddressesMap), EmptyWarnings);
ObjectsForLinking[0] = std::make_unique<OutDwarfFile>(
File.getFileName(), std::move(Context), std::move(AddressesMap));

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