[lldb][AIX] Added XCOFF Object File Header for AIX #111814
Conversation
|
@llvm/pr-subscribers-lldb Author: Dhruv Srivastava (DhruvSrivastavaX) ChangesThis PR is in reference to porting LLDB on AIX. Link to discussions on llvm discourse and github:
Added XCOFF Object File Header for AIX. Commit 1: Taken Linux version for reference. Review Request: @DavidSpickett Full diff: https://github.com/llvm/llvm-project/pull/111814.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.h b/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.h
new file mode 100644
index 00000000000000..2cb73394a0306d
--- /dev/null
+++ b/lldb/source/Plugins/ObjectFile/XCOFF/ObjectFileXCOFF.h
@@ -0,0 +1,244 @@
+//===-- ObjectFileXCOFF.h --------------------------------------- -*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_XCOFF_OBJECTFILEXCOFF_H
+#define LLDB_SOURCE_PLUGINS_OBJECTFILE_XCOFF_OBJECTFILEXCOFF_H
+
+#include <cstdint>
+
+#include <vector>
+
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/lldb-private.h"
+#include "llvm/Object/XCOFFObjectFile.h"
+
+/// \class ObjectFileXCOFF
+/// Generic XCOFF object file reader.
+///
+/// This class provides a generic XCOFF (32/64 bit) reader plugin implementing
+/// the ObjectFile protocol.
+class ObjectFileXCOFF : public lldb_private::ObjectFile {
+public:
+ // Static Functions
+ static void Initialize();
+
+ static void Terminate();
+
+ static llvm::StringRef GetPluginNameStatic() { return "xcoff"; }
+
+ static llvm::StringRef GetPluginDescriptionStatic() {
+ return "XCOFF object file reader.";
+ }
+
+ static lldb_private::ObjectFile *
+ CreateInstance(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
+ lldb::offset_t data_offset, const lldb_private::FileSpec *file,
+ lldb::offset_t file_offset, lldb::offset_t length);
+
+ static lldb_private::ObjectFile *CreateMemoryInstance(
+ const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp,
+ const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
+
+ static size_t GetModuleSpecifications(const lldb_private::FileSpec &file,
+ lldb::DataBufferSP &data_sp,
+ lldb::offset_t data_offset,
+ lldb::offset_t file_offset,
+ lldb::offset_t length,
+ lldb_private::ModuleSpecList &specs);
+
+ static bool MagicBytesMatch(lldb::DataBufferSP &data_sp, lldb::addr_t offset,
+ lldb::addr_t length);
+
+ static lldb::SymbolType MapSymbolType(llvm::object::SymbolRef::Type sym_type);
+
+ // PluginInterface protocol
+ llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
+
+ // LLVM RTTI support
+ static char ID;
+ bool isA(const void *ClassID) const override {
+ return ClassID == &ID || ObjectFile::isA(ClassID);
+ }
+ static bool classof(const ObjectFile *obj) { return obj->isA(&ID); }
+
+ // ObjectFile Protocol.
+ bool ParseHeader() override;
+
+ bool SetLoadAddress(lldb_private::Target &target, lldb::addr_t value,
+ bool value_is_offset) override;
+
+ bool SetLoadAddressByType(lldb_private::Target &target, lldb::addr_t value,
+ bool value_is_offset, int type_id) override;
+
+ lldb::ByteOrder GetByteOrder() const override;
+
+ bool IsExecutable() const override;
+
+ uint32_t GetAddressByteSize() const override;
+
+ lldb_private::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
+
+ void ParseSymtab(lldb_private::Symtab &symtab) override;
+
+ bool IsStripped() override;
+
+ void CreateSections(lldb_private::SectionList &unified_section_list) override;
+
+ void Dump(lldb_private::Stream *s) override;
+
+ lldb_private::ArchSpec GetArchitecture() override;
+
+ lldb_private::UUID GetUUID() override;
+
+ /// Return the contents of the .gnu_debuglink section, if the object file
+ /// contains it.
+ std::optional<lldb_private::FileSpec> GetDebugLink();
+
+ uint32_t GetDependentModules(lldb_private::FileSpecList &files) override;
+
+ lldb_private::Address
+ GetImageInfoAddress(lldb_private::Target *target) override;
+
+ lldb_private::Address GetEntryPointAddress() override;
+
+ lldb_private::Address GetBaseAddress() override;
+
+ ObjectFile::Type CalculateType() override;
+
+ ObjectFile::Strata CalculateStrata() override;
+
+ llvm::StringRef
+ StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const override;
+
+ void RelocateSection(lldb_private::Section *section) override;
+
+ lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
+ ObjectFileXCOFF(const lldb::ModuleSP &module_sp, lldb::DataBufferSP data_sp,
+ lldb::offset_t data_offset,
+ const lldb_private::FileSpec *file, lldb::offset_t offset,
+ lldb::offset_t length);
+
+ ObjectFileXCOFF(const lldb::ModuleSP &module_sp,
+ lldb::DataBufferSP header_data_sp,
+ const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
+
+protected:
+ typedef struct xcoff_header {
+ uint16_t magic;
+ uint16_t nsects;
+ uint32_t modtime;
+ uint64_t symoff;
+ uint32_t nsyms;
+ uint16_t auxhdrsize;
+ uint16_t flags;
+ } xcoff_header_t;
+
+ typedef struct xcoff_aux_header {
+ uint16_t AuxMagic;
+ uint16_t Version;
+ uint32_t ReservedForDebugger;
+ uint64_t TextStartAddr;
+ uint64_t DataStartAddr;
+ uint64_t TOCAnchorAddr;
+ uint16_t SecNumOfEntryPoint;
+ uint16_t SecNumOfText;
+ uint16_t SecNumOfData;
+ uint16_t SecNumOfTOC;
+ uint16_t SecNumOfLoader;
+ uint16_t SecNumOfBSS;
+ uint16_t MaxAlignOfText;
+ uint16_t MaxAlignOfData;
+ uint16_t ModuleType;
+ uint8_t CpuFlag;
+ uint8_t CpuType;
+ uint8_t TextPageSize;
+ uint8_t DataPageSize;
+ uint8_t StackPageSize;
+ uint8_t FlagAndTDataAlignment;
+ uint64_t TextSize;
+ uint64_t InitDataSize;
+ uint64_t BssDataSize;
+ uint64_t EntryPointAddr;
+ uint64_t MaxStackSize;
+ uint64_t MaxDataSize;
+ uint16_t SecNumOfTData;
+ uint16_t SecNumOfTBSS;
+ uint16_t XCOFF64Flag;
+ } xcoff_aux_header_t;
+
+ typedef struct section_header {
+ char name[8];
+ uint64_t phyaddr; // Physical Addr
+ uint64_t vmaddr; // Virtual Addr
+ uint64_t size; // Section size
+ uint64_t offset; // File offset to raw data
+ uint64_t reloff; // Offset to relocations
+ uint64_t lineoff; // Offset to line table entries
+ uint32_t nreloc; // Number of relocation entries
+ uint32_t nline; // Number of line table entries
+ uint32_t flags;
+ } section_header_t;
+
+ typedef struct xcoff_symbol {
+ uint64_t value;
+ uint32_t offset;
+ uint16_t sect;
+ uint16_t type;
+ uint8_t storage;
+ uint8_t naux;
+ } xcoff_symbol_t;
+
+ typedef struct xcoff_sym_csect_aux_entry {
+ uint32_t section_or_len_low_byte;
+ uint32_t parameter_hash_index;
+ uint16_t type_check_sect_num;
+ uint8_t symbol_alignment_and_type;
+ uint8_t storage_mapping_class;
+ uint32_t section_or_len_high_byte;
+ uint8_t pad;
+ uint8_t aux_type;
+ } xcoff_sym_csect_aux_entry_t;
+
+ static bool ParseXCOFFHeader(lldb_private::DataExtractor &data,
+ lldb::offset_t *offset_ptr,
+ xcoff_header_t &xcoff_header);
+ bool ParseXCOFFOptionalHeader(lldb_private::DataExtractor &data,
+ lldb::offset_t *offset_ptr);
+ bool ParseSectionHeaders(uint32_t offset);
+
+ std::vector<LoadableData>
+ GetLoadableData(lldb_private::Target &target) override;
+
+ static lldb::WritableDataBufferSP
+ MapFileDataWritable(const lldb_private::FileSpec &file, uint64_t Size,
+ uint64_t Offset);
+ llvm::StringRef GetSectionName(const section_header_t §);
+ static lldb::SectionType GetSectionType(llvm::StringRef sect_name,
+ const section_header_t §);
+
+ uint32_t ParseDependentModules();
+ typedef std::vector<section_header_t> SectionHeaderColl;
+
+private:
+ bool CreateBinary();
+
+ xcoff_header_t m_xcoff_header;
+ xcoff_aux_header_t m_xcoff_aux_header;
+ SectionHeaderColl m_sect_headers;
+ std::unique_ptr<llvm::object::XCOFFObjectFile> m_binary;
+ lldb_private::Address m_entry_point_address;
+ std::optional<lldb_private::FileSpecList> m_deps_filespec;
+ std::map<std::string, std::vector<std::string>> m_deps_base_members;
+};
+
+#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_ELF_OBJECTFILEELF_H
|
|
A couple of quick notes:
|
|
Hi Community Members, Apologies for being away for a while as I was occupied with some other activities.
Going forward @Lakshmi-Surekha will also be assisting me in some of the upstreaming activities. Thanks & Regards |
Hi @labath ,
|
That's okay, though I fear you're taking this a bit too literally. I definitely wanted to split that PR up, but this is a bit too far. It would be better to have the header and the matching cpp file in the same PR, since you usually need to look at them in tandem. That said, the main reason we (or I, at least) wanted to see this kind of a PR is to determine whether it makes sense to merge this with some of the existing code. For ObjectFileELF, I'm pretty confident the answer to that is going to be "no" (I'm slightly less sure about ObjectFilePECOFF, but I think I'd be willing to take your word for it -- I think a good litmus test would be whether the PECOFF and XCOFF parsing code in llvm shares anything). Given that, the comparison to a different object file plugin is not really interesting to me, as I believe that means this part of code should be treated as any new code that's being added to llvm. In particular, that it should be subject to the incremental development policy. This is where I was coming from when I wrote the previous message (I did not include the context, as I was in a bit of a
I know this is going to be more work, and it's going to be a bit awkward to re-engineer a finished file into an incremental set of changes, but what you have is essentially a long-term development branch -- exactly the thing which the policy discourages. I'm not asking this because I want to be difficult, but large changes really are hard to review. |
|
Okay sure. There are a couple of incoming code with such huge changes, it will be good to adhere to this systematic approach. So, I will go ahead and modify this PR to drop a smaller piece of ObjectFileXCOFF.h and ObjectFileXCOFF.cpp instead, Thanks! |
|
Thank you for your understanding. Before trying to split up the other large files (a fairly large undertaking), I'd still recommend doing the initial thing of dropping the file in a PR (which should be relatively fast) and getting some feedback. ObjectFile plugins are one of the (few) things which I know that can be reasonably split and tested in multiple PRs. Unfortunately, most of the other plugins are much trickier, so we may need to get more creative. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Hi @labath , |
There was a problem hiding this comment.
This is exactly what I had in mind -- thank you.
The patch looks mostly good, apart from the inline comments. My main question is about the "dependant modules" parsing code. AFAICT, its not actually functional (and not tested). Is yaml2objs xcoff backend sufficiently developed to create a test for the dependant module functionality? Given that we rely on llvm for parsing that, we don't have to test it extensively, but it would be nice to have at least one test. We could either do that in this patch, or rip out the dependant module functionality, and add it back in another patch -- up to you.
| PluginManager::UnregisterPlugin(CreateInstance); | ||
| } | ||
|
|
||
| bool UGLY_FLAG_FOR_AIX __attribute__((weak)) = false; |
There was a problem hiding this comment.
We're going to have to find a different way to achieve whatever this is trying to achieve. Let's remove that, as it should not be necessary for this patch.
There was a problem hiding this comment.
Yes, sure. It is actually used in other files as a flag. But we can check on it later.
There was a problem hiding this comment.
I see its used in the dwarf parser. We can deal with that later. My suggestion would be to look at how the llvm dwarf parser (llvm-dwarfdump) does this.
| return 0; | ||
|
|
||
| std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex()); | ||
| if (m_deps_filespec) |
There was a problem hiding this comment.
I'm confused. Who fills in this variable?
There was a problem hiding this comment.
Filtered out for now.
Great. Thanks for your inputs too. On another note, I have a doubt or maybe I am missing something: I am able to run lldb-test successfully on my aix system setup for this new basic xcoff file (where I have the entire aix changes along with a reduced xcoff same as in this PR). |
It should work. It's likely that you missed some critical (small) piece of functionality that causes lldb to think the input is not valid. It should be fairly easy to step through the relevant code in |
|
Ok right. |
|
Hi @labath, |
Are XCOFF files always big endian? If so, then all you need is to add a |
Yes, AIX XCOFF does not support little endian, so I went ahead with the SetByteOrder Fix. |
Ok, great. Yes, please merge it, I don't have the permission to do that yet. |
Added XCOFF Object File Header for AIX. Added base functionality for XCOFF support. Will enhance the files in incremental PRs Details about XCOFF file format on AIX: [XCOFF](https://www.ibm.com/docs/en/aix/7.3?topic=formats-xcoff-object-file-format)
This PR is in reference to porting LLDB on AIX.
Link to discussions on llvm discourse and github:
The complete changes for porting are present in this draft PR:
Extending LLDB to work on AIX #102601
Added XCOFF Object File Header for AIX.
Added base functionality for XCOFF support. Will enhance the files in incremental PRs
Details about XCOFF file format on AIX: XCOFF
Review Request: @DavidSpickett