[PATCH] D85094: [obj2yaml] Add support for dumping the .debug_aranges section.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 3 01:56:19 PDT 2020


grimar added inline comments.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:411
+      if (Err)
+        consumeError(std::move(Err));
+      else
----------------
Higuoxing wrote:
> I want to hear suggestions on handling the parsing error here. I think it would be good if we can 
> emit a warning or add a comment about the details of parsing failure in the generated YAML description. e.g.,
> 
> 1) Emit a warning
> ```
> $ obj2yaml a.out
> ...
> obj2yaml warning: obj2yaml doesn't support parsing the address range table whose segment selector size is not 0.
> ```
> 
> 2) Add a comment field.
> ```
> Sections:
>   - Name:    '.debug_aranges'
>     Type:    SHT_PROGBITS
>     Content: '01234567890abcdef'
>     Comment: 'obj2yaml doesn't support parsing the address range table whose segment selector size is not 0.'
> ```
> 1. Emit a warning

I do not think we are emiting warnings when unable to parse a section, but this
sounds reasonable to me. Perhaps I`d mention in the warning message that we falled
back to dumping section data as raw bytes. Also I'd mention the section name.

> 2. Add a comment field.

I am not sure this is a useful thing to have. At least if it is going to be a comment, it can probably be a real
comment, e.g:

```
## 'obj2yaml doesn't support parsing the address range table whose segment selector size is not 
    Content: '01234567890abcdef'
```

but I think it is probably too much and emiting a warning is enough.






Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85094



More information about the llvm-commits mailing list