[llvm] r229129 - llvm-pdbdump: Improve printing of functions and signatures.
David Blaikie
dblaikie at gmail.com
Fri Feb 13 10:43:22 PST 2015
On Fri, Feb 13, 2015 at 10:32 AM, Zachary Turner <zturner at google.com> wrote:
> Good question. I mentioned it in one of the very first commit messages,
> but I guess that's been a while with many more commit messages since then,
> so it's been lost.
>
> Basically, the dwarf tests are tests against llvm-dwarfdump. So without
> llvm-pdbdump, there can't be any tests. I could iterate, as you said, but
> right now llvm-pdbdump doesn't really support many command line options or
> ways to specify what to dump. It just dumps everything. Which is often
> very slow and pollutes the output, making it difficult to check that only
> the thing you care about is the way you expect and also making the test
> suite run slowly. Also, it's not just that I'm iterating right now, it's
> that some stuff just prints wrong, or doesn't print at all.
>
Slow? Usually the inputs (reduced test cases) are so small that we're
dealing with a tiny amount of debug info. Are the APIs/COM stuff so
abysmally slow as to be observable even on such small test cases?
> Most of those kinks are worked out though, and the patch I'm working on
> right now is one to finally add the correct options for limiting the output
> to specific interesting things.
>
We didn't have that in llvm-dwarfdump until a year or two ago. Before that
we'd just dump it all (but, again, in a reduced test case so there wasn't a
/lot/ of output) & CHECK the bits the test was interested in.
> After that, I plan to start adding tests.
>
> One question I have, is that some information about what's in the PDB you
> can only learn by looking in it. How would one test that? It doesn't seem
> very useful to look in the PDB, then write a test that it matches what you
> know it already is. I guess it's useful for verifying someone doesn't
> break the formatting. What are your thoughts there?
>
My basic strategy is "if something got fixed/change, it's something that
can be tested" (& my bar on "can be" to "should be" is pretty low - much
lower than other people's). If there are improvements to the printing of
functions and signatures I'd expect to see a test showing what the current
format looks like (ideally, maybe, even a change to an existing test so I
could look at the CHECK line diffs in the commit and see what the
improvement was - what we did before, and what we do now).
Even if it's not finished, it's a great way to iteratively build up the
test suite, avoid regressing one piece you fixed yesterday when you go to
improve something else tomorrow, and saves going back at the end and trying
to build up a comprehensive test suite - doing it iteratively seems easier
& more likely to be thorough. (doing it at the end/after a lot of
development it's hard to remember all the interesting corner cases you had
to address along the way)
- David
>
> On Fri Feb 13 2015 at 10:27:32 AM David Blaikie <dblaikie at gmail.com>
> wrote:
>
>> On Fri, Feb 13, 2015 at 9:57 AM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> Author: zturner
>>> Date: Fri Feb 13 11:57:09 2015
>>> New Revision: 229129
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=229129&view=rev
>>> Log:
>>> llvm-pdbdump: Improve printing of functions and signatures.
>>>
>>> This correctly prints the function pointers, and also prints
>>> function signatures for symbols as opposed to just types. So
>>> actual functions in your program will now be printed with full
>>> name and signature, as opposed to just name as before.
>>>
>>
>> What's the story on testing this stuff? llvm-dwarfdump is tested with
>> checked in binaries (though I realize pdbdump tests would only be runnable
>> on Windows (where the library is available)) and it'd be nice to see the
>> actual dumping format in tests changing/improving as you iterate here.
>>
>>
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h
>>> llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h
>>> llvm/trunk/lib/DebugInfo/PDB/PDBSymbolExe.cpp
>>> llvm/trunk/lib/DebugInfo/PDB/PDBSymbolFunc.cpp
>>> llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp
>>> llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h (original)
>>> +++ llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h Fri Feb 13
>>> 11:57:09 2015
>>> @@ -24,6 +24,8 @@ public:
>>>
>>> void dump(raw_ostream &OS, int Indent, PDB_DumpLevel Level) const
>>> override;
>>>
>>> + std::unique_ptr<PDBSymbolTypeFunctionSig> getSignature() const;
>>> +
>>> DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::Function)
>>>
>>> FORWARD_SYMBOL_METHOD(getAccess)
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h
>>> (original)
>>> +++ llvm/trunk/include/llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h Fri
>>> Feb 13 11:57:09 2015
>>> @@ -24,7 +24,12 @@ public:
>>>
>>> DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::FunctionSig)
>>>
>>> + std::unique_ptr<PDBSymbol> getReturnType() const;
>>> + std::unique_ptr<IPDBEnumSymbols> getArguments() const;
>>> + std::unique_ptr<PDBSymbol> getClassParent() const;
>>> +
>>> void dump(raw_ostream &OS, int Indent, PDB_DumpLevel Level) const
>>> override;
>>> + void dumpArgList(raw_ostream &OS) const;
>>>
>>> FORWARD_SYMBOL_METHOD(getCallingConvention)
>>> FORWARD_SYMBOL_METHOD(getClassParentId)
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/PDB/PDBSymbolExe.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBSymbolExe.cpp?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/PDB/PDBSymbolExe.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/PDB/PDBSymbolExe.cpp Fri Feb 13 11:57:09
>>> 2015
>>> @@ -47,6 +47,26 @@ void PDBSymbolExe::dump(raw_ostream &OS,
>>> auto ChildrenEnum = getChildStats(Stats);
>>> OS << stream_indent(Indent + 2) << "Children: " << Stats << "\n";
>>> while (auto Child = ChildrenEnum->getNext()) {
>>> + // Skip uninteresting types. These are useful to print as part of
>>> type
>>> + // hierarchies, but as general children of the global scope, they
>>> are
>>> + // not very interesting.
>>> + switch (Child->getSymTag()) {
>>> + case PDB_SymType::ArrayType:
>>> + case PDB_SymType::BaseClass:
>>> + case PDB_SymType::BuiltinType:
>>> + case PDB_SymType::CompilandEnv:
>>> + case PDB_SymType::CustomType:
>>> + case PDB_SymType::Dimension:
>>> + case PDB_SymType::Friend:
>>> + case PDB_SymType::ManagedType:
>>> + case PDB_SymType::VTableShape:
>>> + case PDB_SymType::PointerType:
>>> + case PDB_SymType::FunctionSig:
>>> + case PDB_SymType::FunctionArg:
>>> + continue;
>>> + default:
>>> + break;
>>> + }
>>> Child->dump(OS, Indent + 4, PDB_DumpLevel::Normal);
>>> OS << "\n";
>>> }
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/PDB/PDBSymbolFunc.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBSymbolFunc.cpp?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/PDB/PDBSymbolFunc.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/PDB/PDBSymbolFunc.cpp Fri Feb 13 11:57:09
>>> 2015
>>> @@ -23,6 +23,10 @@ PDBSymbolFunc::PDBSymbolFunc(const IPDBS
>>> std::unique_ptr<IPDBRawSymbol> Symbol)
>>> : PDBSymbol(PDBSession, std::move(Symbol)) {}
>>>
>>> +std::unique_ptr<PDBSymbolTypeFunctionSig> PDBSymbolFunc::getSignature()
>>> const {
>>> + return
>>> Session.getConcreteSymbolById<PDBSymbolTypeFunctionSig>(getTypeId());
>>> +}
>>> +
>>> void PDBSymbolFunc::dump(raw_ostream &OS, int Indent,
>>> PDB_DumpLevel Level) const {
>>> OS << stream_indent(Indent);
>>> @@ -30,7 +34,7 @@ void PDBSymbolFunc::dump(raw_ostream &OS
>>> uint32_t FuncStart = getRelativeVirtualAddress();
>>> uint32_t FuncEnd = FuncStart + getLength();
>>> if (FuncStart == 0 && FuncEnd == 0) {
>>> - OS << "func [???]";
>>> + OS << "func [???] ";
>>> } else {
>>> OS << "func ";
>>> OS << "[" << format_hex(FuncStart, 8);
>>> @@ -52,21 +56,34 @@ void PDBSymbolFunc::dump(raw_ostream &OS
>>>
>>> OS << " ";
>>> uint32_t FuncSigId = getTypeId();
>>> - if (auto FuncSig =
>>> Session.getConcreteSymbolById<PDBSymbolTypeFunctionSig>(
>>> - FuncSigId)) {
>>> - OS << "(" << FuncSig->getCallingConvention() << ") ";
>>> - }
>>> + if (auto FuncSig = getSignature()) {
>>> + // If we have a signature, dump the name with the signature.
>>> + if (auto ReturnType = FuncSig->getReturnType()) {
>>> + ReturnType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << " ";
>>> + }
>>> +
>>> + OS << FuncSig->getCallingConvention() << " ";
>>>
>>> - uint32_t ClassId = getClassParentId();
>>> - if (ClassId != 0) {
>>> - if (auto Class = Session.getSymbolById(ClassId)) {
>>> - if (auto UDT = dyn_cast<PDBSymbolTypeUDT>(Class.get()))
>>> - OS << UDT->getName() << "::";
>>> - else
>>> - OS << "{class " << Class->getSymTag() << "}::";
>>> + if (auto ClassParent = FuncSig->getClassParent()) {
>>> + ClassParent->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << "::";
>>> }
>>> +
>>> + OS << getName();
>>> + FuncSig->dumpArgList(OS);
>>> + } else {
>>> + uint32_t ClassId = getClassParentId();
>>> + if (ClassId != 0) {
>>> + if (auto Class = Session.getSymbolById(ClassId)) {
>>> + if (auto UDT = dyn_cast<PDBSymbolTypeUDT>(Class.get()))
>>> + OS << UDT->getName() << "::";
>>> + else
>>> + OS << "{class " << Class->getSymTag() << "}::";
>>> + }
>>> + }
>>> + OS << getName();
>>> }
>>> - OS << getName();
>>> } else {
>>> OS << getName();
>>> }
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypeFunctionSig.cpp Fri Feb 13
>>> 11:57:09 2015
>>> @@ -17,35 +17,72 @@
>>>
>>> using namespace llvm;
>>>
>>> +namespace {
>>> +class FunctionArgEnumerator : public IPDBEnumSymbols {
>>> +public:
>>> + typedef ConcreteSymbolEnumerator<PDBSymbolTypeFunctionArg>
>>> ArgEnumeratorType;
>>> +
>>> + FunctionArgEnumerator(const IPDBSession &PDBSession,
>>> + const PDBSymbolTypeFunctionSig &Sig)
>>> + : Session(PDBSession),
>>> + Enumerator(Sig.findAllChildren<PDBSymbolTypeFunctionArg>()) {}
>>> +
>>> + FunctionArgEnumerator(const IPDBSession &PDBSession,
>>> + std::unique_ptr<ArgEnumeratorType>
>>> ArgEnumerator)
>>> + : Session(PDBSession), Enumerator(std::move(ArgEnumerator)) {}
>>> +
>>> + uint32_t getChildCount() const { return Enumerator->getChildCount(); }
>>> +
>>> + std::unique_ptr<PDBSymbol> getChildAtIndex(uint32_t Index) const {
>>> + auto FunctionArgSymbol = Enumerator->getChildAtIndex(Index);
>>> + if (!FunctionArgSymbol)
>>> + return nullptr;
>>> + return Session.getSymbolById(FunctionArgSymbol->getTypeId());
>>> + }
>>> +
>>> + std::unique_ptr<PDBSymbol> getNext() {
>>> + auto FunctionArgSymbol = Enumerator->getNext();
>>> + if (!FunctionArgSymbol)
>>> + return nullptr;
>>> + return Session.getSymbolById(FunctionArgSymbol->getTypeId());
>>> + }
>>> +
>>> + void reset() { Enumerator->reset(); }
>>> +
>>> + MyType *clone() const {
>>> + std::unique_ptr<ArgEnumeratorType> Clone(Enumerator->clone());
>>> + return new FunctionArgEnumerator(Session, std::move(Clone));
>>> + }
>>> +
>>> +private:
>>> + const IPDBSession &Session;
>>> + std::unique_ptr<ArgEnumeratorType> Enumerator;
>>> +};
>>> +}
>>> +
>>> PDBSymbolTypeFunctionSig::PDBSymbolTypeFunctionSig(
>>> const IPDBSession &PDBSession, std::unique_ptr<IPDBRawSymbol>
>>> Symbol)
>>> : PDBSymbol(PDBSession, std::move(Symbol)) {}
>>>
>>> -void PDBSymbolTypeFunctionSig::dump(raw_ostream &OS, int Indent,
>>> - PDB_DumpLevel Level) const {
>>> - OS << stream_indent(Indent);
>>> +std::unique_ptr<PDBSymbol> PDBSymbolTypeFunctionSig::getReturnType()
>>> const {
>>> + return Session.getSymbolById(getTypeId());
>>> +}
>>>
>>> - uint32_t ReturnTypeId = getTypeId();
>>> - if (auto ReturnType = Session.getSymbolById(ReturnTypeId)) {
>>> - ReturnType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> - OS << " ";
>>> - }
>>> - // TODO: We need a way to detect if this is a pointer to function so
>>> that we
>>> - // can print the * between the return type and the argument list.
>>> The only
>>> - // way to do this is to pass the parent into this function, but that
>>> will
>>> - // require a larger interface change.
>>> - OS << getCallingConvention() << " ";
>>> +std::unique_ptr<IPDBEnumSymbols>
>>> +PDBSymbolTypeFunctionSig::getArguments() const {
>>> + return llvm::make_unique<FunctionArgEnumerator>(Session, *this);
>>> +}
>>> +
>>> +std::unique_ptr<PDBSymbol> PDBSymbolTypeFunctionSig::getClassParent()
>>> const {
>>> uint32_t ClassId = getClassParentId();
>>> - if (ClassId != 0) {
>>> - if (auto ClassParent = Session.getSymbolById(ClassId)) {
>>> - OS << "(";
>>> - ClassParent->dump(OS, 0, PDB_DumpLevel::Compact);
>>> - OS << "::*)";
>>> - }
>>> - }
>>> - OS.flush();
>>> + if (ClassId == 0)
>>> + return nullptr;
>>> + return Session.getSymbolById(ClassId);
>>> +}
>>> +
>>> +void PDBSymbolTypeFunctionSig::dumpArgList(raw_ostream &OS) const {
>>> OS << "(";
>>> - if (auto ChildEnum = findAllChildren<PDBSymbolTypeFunctionArg>()) {
>>> + if (auto ChildEnum = getArguments()) {
>>> uint32_t Index = 0;
>>> while (auto Arg = ChildEnum->getNext()) {
>>> Arg->dump(OS, 0, PDB_DumpLevel::Compact);
>>> @@ -54,4 +91,27 @@ void PDBSymbolTypeFunctionSig::dump(raw_
>>> }
>>> }
>>> OS << ")";
>>> + if (isConstType())
>>> + OS << " const";
>>> + if (isVolatileType())
>>> + OS << " volatile";
>>> +}
>>> +
>>> +void PDBSymbolTypeFunctionSig::dump(raw_ostream &OS, int Indent,
>>> + PDB_DumpLevel Level) const {
>>> + OS << stream_indent(Indent);
>>> +
>>> + if (auto ReturnType = getReturnType()) {
>>> + ReturnType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << " ";
>>> + }
>>> +
>>> + OS << getCallingConvention() << " ";
>>> + if (auto ClassParent = getClassParent()) {
>>> + OS << "(";
>>> + ClassParent->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << "::*)";
>>> + }
>>> +
>>> + dumpArgList(OS);
>>> }
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp?rev=229129&r1=229128&r2=229129&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/PDB/PDBSymbolTypePointer.cpp Fri Feb 13
>>> 11:57:09 2015
>>> @@ -10,6 +10,7 @@
>>>
>>> #include "llvm/DebugInfo/PDB/IPDBSession.h"
>>> #include "llvm/DebugInfo/PDB/PDBSymbol.h"
>>> +#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
>>> #include "llvm/DebugInfo/PDB/PDBSymbolTypePointer.h"
>>> #include <utility>
>>>
>>> @@ -27,7 +28,18 @@ void PDBSymbolTypePointer::dump(raw_ostr
>>> if (isVolatileType())
>>> OS << "volatile ";
>>> uint32_t PointeeId = getTypeId();
>>> - if (auto PointeeType = Session.getSymbolById(PointeeId))
>>> - PointeeType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> - OS << ((isReference()) ? "&" : "*");
>>> + if (auto PointeeType = Session.getSymbolById(PointeeId)) {
>>> + // Function pointers get special treatment, since we need to print
>>> the * in
>>> + // the middle of the signature.
>>> + if (auto FuncSig =
>>> dyn_cast<PDBSymbolTypeFunctionSig>(PointeeType.get())) {
>>> + if (auto ReturnType = FuncSig->getReturnType())
>>> + ReturnType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << " (" << FuncSig->getCallingConvention() << " ";
>>> + OS << ((isReference()) ? "&" : "*") << ")";
>>> + FuncSig->dumpArgList(OS);
>>> + } else {
>>> + PointeeType->dump(OS, 0, PDB_DumpLevel::Compact);
>>> + OS << ((isReference()) ? "&" : "*");
>>> + }
>>> + }
>>> }
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150213/3ba27ca9/attachment.html>
More information about the llvm-commits
mailing list