This change fix bug in AMDGPU disassembly. Previously, presence of symbols other than kernel symbols caused objdump to skip begining of those symbols.
Details
Diff Detail
Event Timeline
I find it odd that we need all this machinery just to keep track of the type of a symbol. Why not just make `AllSymbols``` map to a SymbolRef an extract it from there?
Problem here is that SymbolRef::Type isn't same as ELF type. SymbolRef::Type contains only some common types between ELF, COFF and Mach-O file formats. So SymbolRef::Type does not content types that AMDGPU uses (STT_AMDGPU_HSA_KERNEL).
Yes, I think this solution might be better, but it will include rewriting big part of DisassembleObject() that will involve changes in code for other back-ends.
I'm not sure that my change is worth such changes. But If you think it's worth then I can do it.
Hi Michael,
What do you think about this changes? Are they really needed?
Actually, I took another look at DisassembleObject(). Changing AllSymbols map to SymbolRef would require even more changes than I thought earlier. Problem is with COFFObjectFile. It have export table and its contents (class ExportDirectoryEntryRef) are currently stored inside AllSymbols. But if we want to change AllSymbols then we would not be able to store ExportDirectoryEntryRef in AllSymbols. To solve this would requier either some greater machinery or changes in COFFObjectFile architecture.
For what I see "AllSymbols" is a level of indirection to be able to unify symbol name processing with COFF export table, which contains actually no symbols. One of the decision can be to add some generic "type" field to "AllSymbols" and initialize it where applicable removing the need in additional map machinery.
I thought of this but decided that different map whould be better for several reasons:
- It is better if code needed only for AMDGPU would be separeted from general code used by everybody.
- Another thing is that adding another field to AllSymbols would increase it's size for all back-end. Even for those that don't use it.