-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[llvm-objdump] Handle .callgraph section #151009
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
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
uint64_t IgnoredICallIdCount = 0; | ||
// Number of valid indirect calls with type ids. | ||
uint64_t ICallWithTypeIdCount = 0; | ||
if (CallGraphSection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we handle direct callsites and print them out even if the CallgraphSection is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's reasonable if they pass the flag and ask for the call graph. AIUI, that section will only ever contain info on indirect call targets, so it should be fine.
if (CallGraphSection) { | ||
StringRef CGSecContents = unwrapOrError( | ||
CallGraphSection.value().getContents(), Obj->getFileName()); | ||
// TODO: some entries are written in pointer size. are they always 64-bit? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to change this to target specific pointer size so that we can support both 32 bit and 64 bit targets.
outs() << "\n\nINDIRECT CALL SITES (CALLER_ADDR [CALL_SITE_ADDR,])"; | ||
for (const auto &El : FuncInfo) { | ||
auto CallerPc = El.first; | ||
auto FuncIndirCallSites = El.second.IndirectCallSites; | ||
if (!FuncIndirCallSites.empty()) { | ||
outs() << "\n" << format("%lx", CallerPc); | ||
for (auto IndirCallSitePc : FuncIndirCallSites) | ||
outs() << " " << format("%lx", IndirCallSitePc); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all these printing routines should be factored out, so you can change them more easily, or support additional formats.
if (CGSecContents.size() % sizeof(uint64_t)) | ||
reportError(Obj->getFileName(), "Malformed .callgraph section."); | ||
|
||
size_t Size = CGSecContents.size() / sizeof(uint64_t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size_t Size = CGSecContents.size() / sizeof(uint64_t); | |
const size_t Size = CGSecContents.size() / sizeof(uint64_t); |
else | ||
consumeError(NameOrErr.takeError()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this return the error? or maybe at least continue to the next iteration?
reportWarning("there is no .callgraph section", Obj->getFileName()); | ||
|
||
// Map type id to indirect call sites. | ||
MapVector<uint64_t, SmallVector<uint64_t>> TypeIdToIndirCallSites; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider a using
statement to make this type easier to reason about.
using TypIdCallSiteMap = MapVector<uint64_t, SmallVector<uint64_t>>
Introduce --call-graph-info flag to read and print .callgraph section's contents to console in a human readable format