[PATCH] D86539: [Debuginfo][llvm-dwarfutil] llvm-dwarfutil dsymutil-like tool for ELF.
Alexey Lapshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 29 03:27:10 PDT 2022
avl added inline comments.
================
Comment at: llvm/docs/CommandGuide/llvm-dwarfutil.rst:9
+
+:program:`llvm-dwarfutil` [*options*] *input* --out=*output*
+
----------------
jhenderson wrote:
> Assuming you only intend to support a single input file per invocation, I'd follow llvm-objcopy's syntax of `llvm-dwarfutil [options] input [output]`, and if output is missing, modify in-place (this latter behaviour is not necessary, but I think makes some sense).
"modify in-place" behavior has a risk of unintentional rewriting of original file. That is why I disabled it.
If it is preferred behavior then I will change the syntax accordingly.
================
Comment at: llvm/docs/CommandGuide/llvm-dwarfutil.rst:31-38
+ Removes pieces of debug information related to the discarded sections.
+ When the linker does garbage collection the abandoned debug info is left
+ behind. That abandoned debug info references address ranges using
+ tombstone value. Thus, when this option is specified, the tool removes
+ debug info which marked with the tombstone value. That cures
+ "overlapping address ranges" errors reported by :program:`llvm-dwarfdump`.
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > I don't think you need the deleted sentence. Also, if it is enabled by default, how is it disabled, and why does the option actually need to exist?
> There's no need for such massive indentation. A couple of spaces is sufficient. Same applies throughout.
It might be disabled in this way:
```
--do-garbage-collection=0
```
So the option assumed to be used to disable the behavior or to enable it explicitly.
================
Comment at: llvm/docs/CommandGuide/llvm-dwarfutil.rst:111
+
+To report bugs, please visit <https://github.com/llvm/llvm-project/labels/tools:llvm-dwarfutil/>.
----------------
jhenderson wrote:
> Does this label exist yet?
not yet. I think it should be created after review accepted and before integration. right?
================
Comment at: llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp:38
+
+static opt<bool>
+ BuildSeparateDebugFile("separate-debug-file",
----------------
jhenderson wrote:
> I think it would be cleaner to use Tablegen-style options for these options. That also will produce more typical option parsing behaviour (cl::opt has weird syntax for some things).
Will change as it is better way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86539/new/
https://reviews.llvm.org/D86539
More information about the llvm-commits
mailing list