[PATCH] D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 23 14:17:08 PDT 2022


avl added a comment.

In D126059#3532264 <https://reviews.llvm.org/D126059#3532264>, @clayborg wrote:

> A 1 to 2 percent improvement is probably going to go away if the asserts are enabled when it has to do the check each time. If would still hesitate to add this kind of API as it will encourage others to use the APIs. I would be ok with it if the methods are protected or private and if we can add the DWARFLinker as a friend class to allow access to these performant but dangerous APIs only in certain situations that really require it. I really want to avoid most people from being able to use these APIs if possible to make sure we don't end up with bugs. We had many many bugs that stemmed from mismatching DWARFUnit + DWARFDebugIntoEntry before the DWARFDie class was created, mostly when doing cross CU references or in Fission cases where you have the skeleton CU vs the actual DWO CU + a DWARFDebugInfoEntry that comes from the actual DWO file. So there is a real danger to the safety and correctness of the code here.

if all of these bugs could be caught by assertions then the problem probably does not look too severe?
Running debug version of the application will show the problems.

Anyway, if we do not want these methods to be used by others - I am OK to make these methods protected and make DWARFLinker the only one using them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126059



More information about the llvm-commits mailing list