[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