[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/lib/DWARFLinker/DWARFLinker.cpp:503
 
+  if (Unit.overlapsWithFunctionsRanges(*LowPc, *HighPc)) {
+    reportWarning(formatv("Overlapped address range [{0:X}, {1:X}]. Range will "
----------------
clayborg wrote:
> clayborg wrote:
> > avl wrote:
> > > clayborg wrote:
> > > > Can we improve this check to see if the range already exists and not warn if we try to add the same range that is already in there or if the range is a subrange of an existing range?
> > > @clayborg 
> > > 
> > > >Can we improve this check to see if the range already exists and not warn if we try to add the same range that is already in there or if the range is a subrange of an existing range?
> > > 
> > > I did not get the idea. Every overlapping range is such that "it is already in there or if the range is a subrange of an existing range". Probably you mean: not to report the same range twice ?
> > > 
> > > 
> > The idea here is that the "Ranges" ivar in the CompileUnit class I am assuming is used to create the DW_AT_ranges for a compile unit. If you have two functions that LTO combined into a single address range, you currently will get an "Overlapping address range [0x1000-0x2000), [0x1000, 0x2000). Range will be discarded." warning, which is not true at all in this case and is a bad warning. What is actually happening is that the range is already in the "Ranges" and it won't be discarded and everything is fine.
> > 
> > We also get errors in DWARF a lot of the time due to no smart DWARF linkers out there, and you might have a range that is already inside the "Ranges" like if you have a DW_TAG_subprogram with a range of [0x1000-0x2000) and another with [0x1100-0x1200). In this case "Ranges" will already contain the second range and nothing needs to be added to "Ranges". This is bad DWARF of course, but it happens quite a bit as I have taken part in the "llvm-dwarfdump --verify" patches and we would get a error for overlapping ranges here.
> > 
> > So there are 3 cases I can think of:
> > - The same range tries to get added to "Ranges" and that is ok since it is already there
> > - A range that tries to get added is already fully contained within "Ranges", which should also be ok since "Ranges" already contains that range, though it is an indication of bad DWARF.
> > - You have ranges that intersect with "Ranges" but are not fully contained. like if "Ranges" contained "[0x1000-0x02000) and you tried to add [0xf00-0x1100] or [0x1100-0x2100)
> > 
> > If our goal is to do the best job possible with "CompileUnit::Ranges" so that we can produce a valid DW_AT_ranges attribute, then we should try and do our best to keep it up to date. Right now with the current code, we will happily link the DWARF for the 3rd case above, but we will create in incomplete DW_AT_ranges by ejecting either of the overlapping ranges like [0xf00-0x1100] or [0x1100-0x2100).
> Also, I believe "Ranges" will combine sequential ranges from reading the header file, so if "Ranges" already fully contains the range, we do nothing and don't warn, and if a range isn't fully contained but intersects, we should extend the range it overlaps with, and if it doesn't overlap of course we just add it.
@clayborg The patch which implements that behavior is D123469.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86539