[PATCH] D86539: [Debuginfo][llvm-dwarfutil] llvm-dwarfutil dsymutil-like tool for ELF.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/test/tools/llvm-dwarfutil/ELF/Inputs/common.yaml:8-13
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1000
+    AddressAlign:    0x0000000000000010
+    Content:        "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"
----------------
jhenderson wrote:
> Assuming you need a .text section, do you actually need it to have anything other than something like the below?
> ```
> Name:    .text
> Type:    SHT_PROGBITS
> Address: 0x1000
> Size:    0x10
> ```
we need Flags and probably AddressAlign:, but Content might be replaced with Size.


================
Comment at: llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp:452-453
+
+// TODO: This implementation is copied from llvm-objcopy.
+//       It is nessessary to refactor it out and remove this duplication.
+static Error restoreStatOnFile(StringRef Filename,
----------------
jhenderson wrote:
> Could we refactor it out first please? Alternatively, does the behaviour need to be in the initial landing of this patch, or can it be added later?
I think it is better to land it in this form. It is useful functionality and it is better to have it from start. As to the refactoring restoreStatOnFile - I am going to create a patch for soon.


================
Comment at: llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp:528
+  if (Error Err = validateAndSetOptions(Args, Opts)) {
+    error(std::move(Err), dwarfutil::ToolName);
+    return EXIT_FAILURE;
----------------
jhenderson wrote:
> Many tools have the `error()` function call `std::exit(1)` to end the program without needing an explicit `return EXIT_FAILURE`. I'm okay either way with it, but wanted to raise common practice with you here.
The reason to not calling std::exit(1) is that error() function  is used as a error reporting function for DWARFLinker. DWARFLinker in it`s current state does not stop if any error encountered. As an alternative I can create separate error reporting function for DWARFLinker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86539