[PATCH] D105917: [llvm-objdump][CallGraphSection] Extract call graph information from binary

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 26 00:42:19 PDT 2021


jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

Please don't commit this yet. there's a lot of code, and I'd like to spend quite some time reviewing it - a first glance through raises some initial concerns, but I need to understand the whole thing to be able to properly consider them and possible suggestions.

Some initial high-level comments/questions:

1. Is it possible to break this patch down into smaller parts, perhaps implementing some subset of the functionality per part? That'll make things easier to review.
2. Why is this functionality being added to llvm-objdump rather than llvm-readobj? (There may be a perfectly valid reason to do so, but llvm-readobj tends to be the place we put things to interpret arbitrary sections).
3. Can we avoid the canned binary in the test, please? Use an input generated at runtime using either llvm-mc or yaml2obj (the latter is preferable, but likely requires adding functionality to yaml2obj for the new section type).



================
Comment at: llvm/tools/llvm-objdump/ObjdumpOpts.td:36
+  HelpText<"Dump call graph information including indirect call and target IDs "
+           "from call graph section, if available.">;
+
----------------
Please place this in alphabetical order in relation to other options (i.e. before demangle).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105917/new/

https://reviews.llvm.org/D105917



More information about the llvm-commits mailing list