[PATCH] D124121: [DebugInfo] Give warning instead of error for premature terminator in .debug_aranges section.

[Junfeng Dong via Phabricator via llvm-commits]

junfd added a comment.

In D124121#3466853 <https://reviews.llvm.org/D124121#3466853>, @jhenderson wrote:

> It's been a while since I looked at this code, so let me just confirm my understanding is correct:
>
> 1. You have a debug aranges table entry with address and size 0, but which corresponds to a real piece of data of size 0.
> 2. Such entries are treated as "warning" level problems, i.e. don't prevent continued parsing of the data within `DWARFDebugArangesSet`.
> 3. Prior to your change, "warning" level issues were reported via the same callback as "recoverable errors" within the `DWARFDebugAranges` code.
> 4. Your change changes the warning to be handled via a different callback, so that llvm-profdata can handle them differently.
>
> All seems reasonable to me.

I don't think llvm-profdata handle these error message at all. it just calls "Symbolizer->symbolizeInlinedCode(SymbolizerPath.str(), Addr)" . After the different handler is hooked in the code where I changed, the extract function (DWARFDebugArangeSet::extract) directly print error/warning message out by the following line:

  WarningHandler(createStringError(
            errc::invalid_argument,
            "address range table at offset 0x%" PRIx64
            " has a premature terminator entry at offset 0x%" PRIx64,
            Offset, EntryOffset));

> Re. testing, are there no existing ones that break with this change? If we need new tests, a lot of the DWARF code has gtest unit tests, including DWARFDebugArangesSet. I wonder if you could use those existing ones as a basis for testing these code chanegs?

It doesn't break any existing tests. I will check in a test case under llvm-profgen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124121