[PATCH] D125783: [llvm-debuginfo-analyzer] 08 - ELF Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp:289
+  Iter = SectionAddresses.lower_bound(Address);
+  if (Iter != SectionAddresses.begin())
+    --Iter;
----------------
psamolysov wrote:
> CarlosAlbertoEnciso wrote:
> > CarlosAlbertoEnciso wrote:
> > > probinson wrote:
> > > > psamolysov wrote:
> > > > > Should the `Iter` be checked that it is not `SectionAddresses.end()`? If the `Iter` is the `end()` iterator, `--Iter` just returns it to a "normal" value and all the checks for `end` in the callers (if these checks are there) will positive (sometimes false positive).
> > > > This comment hasn't been answered.
> > > @psamolysov I am sorry for missing this comment. I will answer it once I finish with the 09-codeview-reader patch.
> > The best way to explain this code is by an example:
> > 
> > Using the LLVM 'count' testing tool executable
> > `llvm-debuginfo-analyzer --print=lines,instructions count.exe`
> > 
> > We traverse the collected public names to generate assembler instructions by calling `createInstructions`.
> > 
> > Collected public names: CompileUnit `PublicNames`
> > ```
> > Range: [0x01400014c0:0x0140001760] Name: 'main' / ''
> > Range: [0x0140001820:0x0140001865] Name: '_vfprintf_l' / ''
> > Range: [0x0140001810:0x014000181a] Name: '__local_stdio_printf_options' / ''
> > Range: [0x0140001880:0x01400018f9] Name: 'fprintf' / ''
> > ```
> > Code section information: `SectionAddresses`
> > ```
> > [0x0140001000:0x0140008cdf] Size: 0x0000007cdf
> > ```
> > While generating instructions for the public name `main`
> > ```
> > 'main' / 'main' Range: [0x01400014c0:0x0140001760]
> > ```
> > Find an entry in `SectionAddresses` that contains its lower `Address=0x01400014c0` 
> > 
> > ```
> >   Iter = SectionAddresses.lower_bound(Address);
> >   if (Iter != SectionAddresses.begin())
> >     --Iter;
> > ```  
> > `Iter` is the `end()` iterator.
> > `--Iter` points to the entry: `0x0140001000` which is the object section that contains the code for `main`.
> Thank you for the great explanation. I've checked from some sources (answers on SO but not the standard, unfortunately) that doing -- for end() is ok for bidirectional iterators. If the container is empty, end() will be equal to begin() and your code won't do -- for the iterator. So, I have no concerns. Thank you very much for the application.
Thanks for your very valuable feedback.


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

https://reviews.llvm.org/D125783