[Lldb-commits] [PATCH] D131437: Don't index the skeleton CU when we have a fission compile unit.

Tue Aug 9 09:34:25 PDT 2022

clayborg added a comment.

In D131437#3709921 <https://reviews.llvm.org/D131437#3709921>, @labath wrote:

> Seems reasonable, but could use a test case, though I'm not sure what would be the best way to approach that. I suppose one could dump the index of one of these dwo-less files, and then make sure it's contents are right (empty?).

That is what I was struggling with. I might be able to use lldb-test to dump a type lookup on a name that used to appear in both the DWO file and in the skeleton compile unit and make sure no error string from the parsing gets emitted? I haven't used lldb-test much, but is this possible to expect output and make sure the error that detected this issue is not emitted once it is fixed? The test would create a binary with -fsplit-dwarf-inlining enabled and make sure that the skeleton compile unit ends up having a type from a type template that _could_ be found if this fix wasn't there, then make sure when we do a type lookup we don't see the error message. Let me know if you have any other ideas on how to do this.

> The m_dwo_id change also looks like its fixing a bug where we could end up mistakenly associating a regular unit (from the main exe file) with a split unit from a dwp file if that split unit happens to have a dwo id of zero. That might be another test vector.

yeah, I guess we would test this by making a DWO ID of zero and making sure it works. Doesn't matter if it is in a .dwo file in a .dwp file right? Just a test with a "a.out" binary that contains a skeleton compile unit that has a DWO ID of zero and a corresponding .dwo file that has this same ID inside of it right?

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h:339
   /// Value of DW_AT_GNU_dwo_id (v4) or dwo_id from CU header (v5).
-  uint64_t m_dwo_id;
+  llvm::Optional<uint64_t> m_dwo_id;

----------------
labath wrote:
> What's the relationship of this field and the m_is_dwo flag? Do we need both?
I think the "m_is_dwo" is set to true if this is a DWO file, where m_dwo_id is for the skeleton compile unit. So they are different and can't be derived from each other if I understand the code correctly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131437