labath created this revision. labath added reviewers: clayborg, zturner. Herald added subscribers: JDevlieghere, arichardson, emaste. Herald added a reviewer: espindola.
instead of returning the architecture through by-ref argument and a boolean value indicating success, we can just return the ArchSpec directly. Since the ArchSpec already has an invalid state, it can be used to denote the failure without the additional bool. https://reviews.llvm.org/D56129 Files: include/lldb/Core/Module.h include/lldb/Expression/IRExecutionUnit.h include/lldb/Symbol/ObjectFile.h include/lldb/Symbol/UnwindTable.h include/lldb/Utility/ArchSpec.h source/Core/Module.cpp source/Expression/IRExecutionUnit.cpp source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp source/Plugins/ObjectFile/ELF/ObjectFileELF.h source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp source/Plugins/ObjectFile/JIT/ObjectFileJIT.h source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h source/Plugins/Process/elf-core/ProcessElfCore.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Symbol/CompactUnwindInfo.cpp source/Symbol/DWARFCallFrameInfo.cpp source/Symbol/FuncUnwinders.cpp source/Symbol/Type.cpp source/Symbol/UnwindTable.cpp
Index: source/Symbol/UnwindTable.cpp =================================================================== --- source/Symbol/UnwindTable.cpp +++ source/Symbol/UnwindTable.cpp @@ -180,8 +180,8 @@ return m_arm_unwind_up.get(); } -bool UnwindTable::GetArchitecture(lldb_private::ArchSpec &arch) { - return m_object_file.GetArchitecture(arch); +ArchSpec UnwindTable::GetArchitecture() { + return m_object_file.GetArchitecture(); } bool UnwindTable::GetAllowAssemblyEmulationUnwindPlans() { Index: source/Symbol/Type.cpp =================================================================== --- source/Symbol/Type.cpp +++ source/Symbol/Type.cpp @@ -328,8 +328,7 @@ case eEncodingIsPointerUID: case eEncodingIsLValueReferenceUID: case eEncodingIsRValueReferenceUID: { - ArchSpec arch; - if (m_symbol_file->GetObjectFile()->GetArchitecture(arch)) + if (ArchSpec arch = m_symbol_file->GetObjectFile()->GetArchitecture()) m_byte_size = arch.GetAddressByteSize(); } break; } Index: source/Symbol/FuncUnwinders.cpp =================================================================== --- source/Symbol/FuncUnwinders.cpp +++ source/Symbol/FuncUnwinders.cpp @@ -443,8 +443,7 @@ lldb::UnwindAssemblySP FuncUnwinders::GetUnwindAssemblyProfiler(Target &target) { UnwindAssemblySP assembly_profiler_sp; - ArchSpec arch; - if (m_unwind_table.GetArchitecture(arch)) { + if (ArchSpec arch = m_unwind_table.GetArchitecture()) { arch.MergeFrom(target.GetArchitecture()); assembly_profiler_sp = UnwindAssembly::FindPlugin(arch); } Index: source/Symbol/DWARFCallFrameInfo.cpp =================================================================== --- source/Symbol/DWARFCallFrameInfo.cpp +++ source/Symbol/DWARFCallFrameInfo.cpp @@ -423,8 +423,7 @@ m_objfile.GetFileSpec().GetFilename().AsCString("")); bool clear_address_zeroth_bit = false; - ArchSpec arch; - if (m_objfile.GetArchitecture(arch)) { + if (ArchSpec arch = m_objfile.GetArchitecture()) { if (arch.GetTriple().getArch() == llvm::Triple::arm || arch.GetTriple().getArch() == llvm::Triple::thumb) clear_address_zeroth_bit = true; Index: source/Symbol/CompactUnwindInfo.cpp =================================================================== --- source/Symbol/CompactUnwindInfo.cpp +++ source/Symbol/CompactUnwindInfo.cpp @@ -183,8 +183,7 @@ if (function_info.encoding == 0) return false; - ArchSpec arch; - if (m_objfile.GetArchitecture(arch)) { + if (ArchSpec arch = m_objfile.GetArchitecture()) { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND)); if (log && log->GetVerbose()) { @@ -338,8 +337,7 @@ // }; bool clear_address_zeroth_bit = false; - ArchSpec arch; - if (m_objfile.GetArchitecture(arch)) { + if (ArchSpec arch = m_objfile.GetArchitecture()) { if (arch.GetTriple().getArch() == llvm::Triple::arm || arch.GetTriple().getArch() == llvm::Triple::thumb) clear_address_zeroth_bit = true; Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1084,9 +1084,7 @@ * #0 * for MIPS. Use ArchSpec to clear the bit #0. */ - ArchSpec arch; - GetObjectFile()->GetArchitecture(arch); - switch (arch.GetMachine()) { + switch (GetObjectFile()->GetArchitecture().GetMachine()) { case llvm::Triple::mips: case llvm::Triple::mipsel: case llvm::Triple::mips64: Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp =================================================================== --- source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -736,8 +736,7 @@ } ArchSpec ProcessElfCore::GetArchitecture() { - ArchSpec arch; - m_core_module_sp->GetObjectFile()->GetArchitecture(arch); + ArchSpec arch = m_core_module_sp->GetObjectFile()->GetArchitecture(); ArchSpec target_arch = GetTarget().GetArchitecture(); arch.MergeFrom(target_arch); Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h =================================================================== --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -110,7 +110,7 @@ void Dump(lldb_private::Stream *s) override; - bool GetArchitecture(lldb_private::ArchSpec &arch) override; + lldb_private::ArchSpec GetArchitecture() override; bool GetUUID(lldb_private::UUID *uuid) override; Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp =================================================================== --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -944,8 +944,7 @@ s->Indent(); s->PutCString("ObjectFilePECOFF"); - ArchSpec header_arch; - GetArchitecture(header_arch); + ArchSpec header_arch = GetArchitecture(); *s << ", file = '" << m_file << "', arch = " << header_arch.GetArchitectureName() << "\n"; @@ -1148,9 +1147,11 @@ } } -bool ObjectFilePECOFF::GetArchitecture(ArchSpec &arch) { +ArchSpec ObjectFilePECOFF::GetArchitecture() { uint16_t machine = m_coff_header.machine; switch (machine) { + default: + break; case llvm::COFF::IMAGE_FILE_MACHINE_AMD64: case llvm::COFF::IMAGE_FILE_MACHINE_I386: case llvm::COFF::IMAGE_FILE_MACHINE_POWERPC: @@ -1158,14 +1159,13 @@ case llvm::COFF::IMAGE_FILE_MACHINE_ARM: case llvm::COFF::IMAGE_FILE_MACHINE_ARMNT: case llvm::COFF::IMAGE_FILE_MACHINE_THUMB: + ArchSpec arch; arch.SetArchitecture(eArchTypeCOFF, machine, LLDB_INVALID_CPUTYPE, IsWindowsSubsystem() ? llvm::Triple::Win32 : llvm::Triple::UnknownOS); - return true; - default: - break; + return arch; } - return false; + return ArchSpec(); } ObjectFile::Type ObjectFilePECOFF::CalculateType() { Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h =================================================================== --- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -92,7 +92,7 @@ void Dump(lldb_private::Stream *s) override; - bool GetArchitecture(lldb_private::ArchSpec &arch) override; + lldb_private::ArchSpec GetArchitecture() override; bool GetUUID(lldb_private::UUID *uuid) override; @@ -147,10 +147,10 @@ lldb::offset_t lc_offset, // Offset to the first load command lldb_private::UUID &uuid); - static bool GetArchitecture(const llvm::MachO::mach_header &header, - const lldb_private::DataExtractor &data, - lldb::offset_t lc_offset, - lldb_private::ArchSpec &arch); + static lldb_private::ArchSpec + GetArchitecture(const llvm::MachO::mach_header &header, + const lldb_private::DataExtractor &data, + lldb::offset_t lc_offset); // Intended for same-host arm device debugging where lldb needs to // detect libraries in the shared cache and augment the nlist entries Index: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp =================================================================== --- source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -915,12 +915,10 @@ spec.SetObjectOffset(file_offset); spec.SetObjectSize(length); - if (GetArchitecture(header, data, data_offset, - spec.GetArchitecture())) { - if (spec.GetArchitecture().IsValid()) { - GetUUID(header, data, data_offset, spec.GetUUID()); - specs.Append(spec); - } + spec.GetArchitecture() = GetArchitecture(header, data, data_offset); + if (spec.GetArchitecture().IsValid()) { + GetUUID(header, data, data_offset, spec.GetUUID()); + specs.Append(spec); } } } @@ -1103,9 +1101,7 @@ if (can_parse) { m_data.GetU32(&offset, &m_header.cputype, 6); - ArchSpec mach_arch; - - if (GetArchitecture(mach_arch)) { + if (ArchSpec mach_arch = GetArchitecture()) { // Check if the module has a required architecture const ArchSpec &module_arch = module_sp->GetArchitecture(); if (module_arch.IsValid() && !module_arch.IsCompatibleMatch(mach_arch)) @@ -4838,8 +4834,7 @@ else s->PutCString("ObjectFileMachO32"); - ArchSpec header_arch; - GetArchitecture(header_arch); + ArchSpec header_arch = GetArchitecture(); *s << ", file = '" << m_file << "', triple = " << header_arch.GetTriple().getTriple() << "\n"; @@ -4962,10 +4957,11 @@ }; } // namespace -bool ObjectFileMachO::GetArchitecture(const llvm::MachO::mach_header &header, - const lldb_private::DataExtractor &data, - lldb::offset_t lc_offset, - ArchSpec &arch) { +ArchSpec +ObjectFileMachO::GetArchitecture(const llvm::MachO::mach_header &header, + const lldb_private::DataExtractor &data, + lldb::offset_t lc_offset) { + ArchSpec arch; arch.SetArchitecture(eArchTypeMachO, header.cputype, header.cpusubtype); if (arch.IsValid()) { @@ -4990,7 +4986,7 @@ triple.setVendor(llvm::Triple::UnknownVendor); triple.setVendorName(llvm::StringRef()); } - return true; + return arch; } else { struct load_command load_cmd; llvm::SmallString<16> os_name; @@ -5019,7 +5015,7 @@ os << GetOSName(load_cmd.cmd) << min_os.major_version << '.' << min_os.minor_version << '.' << min_os.patch_version; triple.setOSName(os.str()); - return true; + return arch; } default: break; @@ -5055,7 +5051,7 @@ triple.setOSName(os.str()); if (!os_env.environment.empty()) triple.setEnvironmentName(os_env.environment); - return true; + return arch; } } while (0); offset = cmd_offset + load_cmd.cmdsize; @@ -5070,7 +5066,7 @@ } } } - return arch.IsValid(); + return arch; } bool ObjectFileMachO::GetUUID(lldb_private::UUID *uuid) { @@ -5692,14 +5688,16 @@ return llvm::VersionTuple(); } -bool ObjectFileMachO::GetArchitecture(ArchSpec &arch) { +ArchSpec ObjectFileMachO::GetArchitecture() { ModuleSP module_sp(GetModule()); + ArchSpec arch; if (module_sp) { std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex()); + return GetArchitecture(m_header, m_data, - MachHeaderSizeFromMagic(m_header.magic), arch); + MachHeaderSizeFromMagic(m_header.magic)); } - return false; + return arch; } void ObjectFileMachO::GetProcessSharedCacheUUID(Process *process, addr_t &base_addr, UUID &uuid) { Index: source/Plugins/ObjectFile/JIT/ObjectFileJIT.h =================================================================== --- source/Plugins/ObjectFile/JIT/ObjectFileJIT.h +++ source/Plugins/ObjectFile/JIT/ObjectFileJIT.h @@ -73,7 +73,7 @@ void Dump(lldb_private::Stream *s) override; - bool GetArchitecture(lldb_private::ArchSpec &arch) override; + lldb_private::ArchSpec GetArchitecture() override; bool GetUUID(lldb_private::UUID *uuid) override; Index: source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp =================================================================== --- source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp +++ source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp @@ -153,8 +153,7 @@ s->Indent(); s->PutCString("ObjectFileJIT"); - ArchSpec arch; - if (GetArchitecture(arch)) + if (ArchSpec arch = GetArchitecture()) *s << ", arch = " << arch.GetArchitectureName(); s->EOL(); @@ -190,11 +189,10 @@ ObjectFile::Strata ObjectFileJIT::CalculateStrata() { return eStrataJIT; } -bool ObjectFileJIT::GetArchitecture(ArchSpec &arch) { - ObjectFileJITDelegateSP delegate_sp(m_delegate_wp.lock()); - if (delegate_sp) - return delegate_sp->GetArchitecture(arch); - return false; +ArchSpec ObjectFileJIT::GetArchitecture() { + if (ObjectFileJITDelegateSP delegate_sp = m_delegate_wp.lock()) + return delegate_sp->GetArchitecture(); + return ArchSpec(); } //------------------------------------------------------------------ Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.h =================================================================== --- source/Plugins/ObjectFile/ELF/ObjectFileELF.h +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.h @@ -121,7 +121,7 @@ void Dump(lldb_private::Stream *s) override; - bool GetArchitecture(lldb_private::ArchSpec &arch) override; + lldb_private::ArchSpec GetArchitecture() override; bool GetUUID(lldb_private::UUID *uuid) override; Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp =================================================================== --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -434,9 +434,8 @@ if (address_size == 4 || address_size == 8) { std::unique_ptr<ObjectFileELF> objfile_ap(new ObjectFileELF( module_sp, data_sp, data_offset, file, file_offset, length)); - ArchSpec spec; - if (objfile_ap->GetArchitecture(spec) && - objfile_ap->SetModulesArchitecture(spec)) + ArchSpec spec = objfile_ap->GetArchitecture(); + if (spec && objfile_ap->SetModulesArchitecture(spec)) return objfile_ap.release(); } @@ -453,9 +452,8 @@ if (address_size == 4 || address_size == 8) { std::unique_ptr<ObjectFileELF> objfile_ap( new ObjectFileELF(module_sp, data_sp, process_sp, header_addr)); - ArchSpec spec; - if (objfile_ap->GetArchitecture(spec) && - objfile_ap->SetModulesArchitecture(spec)) + ArchSpec spec = objfile_ap->GetArchitecture(); + if (spec && objfile_ap->SetModulesArchitecture(spec)) return objfile_ap.release(); } } @@ -1957,8 +1955,7 @@ bool skip_oatdata_oatexec = file_extension == ConstString(".oat") || file_extension == ConstString(".odex"); - ArchSpec arch; - GetArchitecture(arch); + ArchSpec arch = GetArchitecture(); ModuleSP module_sp(GetModule()); SectionList *module_section_list = module_sp ? module_sp->GetSectionList() : nullptr; @@ -2885,8 +2882,7 @@ s->Indent(); s->PutCString("ObjectFileELF"); - ArchSpec header_arch; - GetArchitecture(header_arch); + ArchSpec header_arch = GetArchitecture(); *s << ", file = '" << m_file << "', arch = " << header_arch.GetArchitectureName() << "\n"; @@ -3172,9 +3168,9 @@ } } -bool ObjectFileELF::GetArchitecture(ArchSpec &arch) { +ArchSpec ObjectFileELF::GetArchitecture() { if (!ParseHeader()) - return false; + return ArchSpec(); if (m_section_headers.empty()) { // Allow elf notes to be parsed which may affect the detected architecture. @@ -3195,8 +3191,7 @@ } } } - arch = m_arch_spec; - return true; + return m_arch_spec; } ObjectFile::Type ObjectFileELF::CalculateType() { Index: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h =================================================================== --- source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h +++ source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h @@ -82,7 +82,7 @@ void Dump(Stream *s) override {} - bool GetArchitecture(ArchSpec &arch) override; + ArchSpec GetArchitecture() override { return m_arch; } bool GetUUID(UUID *uuid) override; Index: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp =================================================================== --- source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp +++ source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp @@ -230,11 +230,6 @@ return nullptr; } -bool ObjectFileBreakpad::GetArchitecture(ArchSpec &arch) { - arch = m_arch; - return true; -} - bool ObjectFileBreakpad::GetUUID(UUID *uuid) { *uuid = m_uuid; return true; Index: source/Expression/IRExecutionUnit.cpp =================================================================== --- source/Expression/IRExecutionUnit.cpp +++ source/Expression/IRExecutionUnit.cpp @@ -1217,14 +1217,11 @@ } } -bool IRExecutionUnit::GetArchitecture(lldb_private::ArchSpec &arch) { +ArchSpec IRExecutionUnit::GetArchitecture() { ExecutionContext exe_ctx(GetBestExecutionContextScope()); - Target *target = exe_ctx.GetTargetPtr(); - if (target) - arch = target->GetArchitecture(); - else - arch.Clear(); - return arch.IsValid(); + if(Target *target = exe_ctx.GetTargetPtr()) + return target->GetArchitecture(); + return ArchSpec(); } lldb::ModuleSP IRExecutionUnit::GetJITModule() { Index: source/Core/Module.cpp =================================================================== --- source/Core/Module.cpp +++ source/Core/Module.cpp @@ -309,7 +309,7 @@ // Once we get the object file, update our module with the object // file's architecture since it might differ in vendor/os if some // parts were unknown. - m_objfile_sp->GetArchitecture(m_arch); + m_arch = m_objfile_sp->GetArchitecture(); } else { error.SetErrorString("unable to find suitable object file plug-in"); } @@ -1265,9 +1265,7 @@ // parts were unknown. But since the matching arch might already be // more specific than the generic COFF architecture, only merge in // those values that overwrite unspecified unknown values. - ArchSpec new_arch; - m_objfile_sp->GetArchitecture(new_arch); - m_arch.MergeFrom(new_arch); + m_arch.MergeFrom(m_objfile_sp->GetArchitecture()); } else { ReportError("failed to load objfile for %s", GetFileSpec().GetPath().c_str()); Index: include/lldb/Utility/ArchSpec.h =================================================================== --- include/lldb/Utility/ArchSpec.h +++ include/lldb/Utility/ArchSpec.h @@ -371,6 +371,7 @@ bool IsValid() const { return m_core >= eCore_arm_generic && m_core < kNumCores; } + explicit operator bool() const { return IsValid(); } bool TripleVendorWasSpecified() const { return !m_triple.getVendorName().empty(); Index: include/lldb/Symbol/UnwindTable.h =================================================================== --- include/lldb/Symbol/UnwindTable.h +++ include/lldb/Symbol/UnwindTable.h @@ -50,7 +50,7 @@ GetUncachedFuncUnwindersContainingAddress(const Address &addr, SymbolContext &sc); - bool GetArchitecture(lldb_private::ArchSpec &arch); + ArchSpec GetArchitecture(); private: void Dump(Stream &s); Index: include/lldb/Symbol/ObjectFile.h =================================================================== --- include/lldb/Symbol/ObjectFile.h +++ include/lldb/Symbol/ObjectFile.h @@ -40,7 +40,7 @@ virtual void PopulateSectionList(lldb_private::ObjectFile *obj_file, lldb_private::SectionList §ion_list) = 0; - virtual bool GetArchitecture(lldb_private::ArchSpec &arch) = 0; + virtual ArchSpec GetArchitecture() = 0; }; //---------------------------------------------------------------------- @@ -305,19 +305,13 @@ virtual const FileSpec &GetFileSpec() const { return m_file; } //------------------------------------------------------------------ - /// Get the name of the cpu, vendor and OS for this object file. - /// - /// This value is a string that represents the target triple where the cpu - /// type, the vendor and the OS are encoded into a string. - /// - /// @param[out] target_triple - /// The string value of the target triple. + /// Get the ArchSpec for this object file. /// /// @return - /// \b True if the target triple was able to be computed, \b - /// false otherwise. + /// The ArchSpec of this object file. In case of error, an invalid + /// ArchSpec object is returned. //------------------------------------------------------------------ - virtual bool GetArchitecture(ArchSpec &arch) = 0; + virtual ArchSpec GetArchitecture() = 0; //------------------------------------------------------------------ /// Gets the section list for the currently selected architecture (and Index: include/lldb/Expression/IRExecutionUnit.h =================================================================== --- include/lldb/Expression/IRExecutionUnit.h +++ include/lldb/Expression/IRExecutionUnit.h @@ -108,7 +108,7 @@ void PopulateSectionList(lldb_private::ObjectFile *obj_file, lldb_private::SectionList §ion_list) override; - bool GetArchitecture(lldb_private::ArchSpec &arch) override; + ArchSpec GetArchitecture() override; lldb::ModuleSP GetJITModule(); Index: include/lldb/Core/Module.h =================================================================== --- include/lldb/Core/Module.h +++ include/lldb/Core/Module.h @@ -168,10 +168,11 @@ // Once we get the object file, update our module with the object file's // architecture since it might differ in vendor/os if some parts were // unknown. - if (!module_sp->m_objfile_sp->GetArchitecture(module_sp->m_arch)) - return nullptr; - - return module_sp; + if (ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture()) { + module_sp->m_arch = arch; + return module_sp; + } + return nullptr; } //------------------------------------------------------------------
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits