[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

Mon Jan 23 06:10:25 PST 2023

labath added a comment.

In D138618#4060565 <https://reviews.llvm.org/D138618#4060565>, @clayborg wrote:

> ...
> Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else, we can make them what we need them to be so they work for us. I would suggest to remove the use of DIERef from calculating the IDs of symbol files and have .o files for mac and .dwo files for fission use a 1 based index as their ID to make it easy to encode into a DIERef when needed for lldb::user_id_t values that _are_ included in objects that we hand out. Is there anything else that would need to be done to keep everyone happy here?

I think that the 1-based index thingy helps a lot here, but I haven't seen anything (in your reponse, or in the new patch) that would address my concernt DIERef<->user_id conversion ambiguity. I.e. how is one supposed to know what is the "right" way to convert a DIERef to a user_id:

- `die_ref.get_id()`
- or `symbol_file.GetUID(die_ref)` (which, funnily enough, will construct another DIERef, and *then* call get_id? (`return DIERef(GetID(), ref.section(), ref.die_offset()).get_id();`)

What's your position on that? That we should live with the ambiguity?

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:53
+
+  void set_die_offset(dw_offset_t offset) {
+    lldbassert(offset <= DW_INVALID_OFFSET);
----------------
Can we remove this function now?

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:11

+#include "DIERef.h"
 #include "lldb/Core/Section.h"
----------------
nor this

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h:12

+#include "DIERef.h"
 #include "SymbolFileDWARF.h"
----------------
I guess this isn't necessary anymore.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618