Hi clayborg, tfiala,

This diff contains some initial work in getting lldb to debug non-8-bit byte 
architectures (currently only CSRs kalimba 5 architecture). I appreciate that 
it won't interest non-CSR people greatly but I thought it polite to place up 
for review prior to commit.

Key points:
* target_byte_size is added to DataExtractor for subsequent use as a formatting 
hint
* target_byte_size is added to Section class, since section sizes are in host 
bytes, and hence a multiplier is required to calculate correct offsets
* ObjectFileELF.cpp slightly modified to create sections with correct bytesize 
(kalimba annoyingly has differing bytesizes in code and data)
* ObjectFile.cpp ReadSection modified regarding bytesizes and offsets
* CommandObjectMemory changed can to allocate larger read buffers for mentioned 
architectures and to advise DataExtractor accordingly
* DataExtractor when printing bytes, consume target_byte_size number of bytes 
from host buffer for each byte printed.

Other changes in the diff:
* removal eCore_kalimba enumerant (specify each kalimba variant separately)
* support better recognition of kalimba3/4/5
* In ArchSpec lie about kalimba 24-bit bytes. Say they are 32-bit since 24-bit 
bytes are much harder to express on regular Intel PC platforms etc.

This diff is very, very minimal regarding getting non-8-bit byte architectures 
supported in lldb. However, for me, the only practical way is to work 
incrementally - lldb is far too big to change in one go, in this area. For 
example, only the count option is currently supported for memory read, I've not 
looked in any depth at -f format options.

http://reviews.llvm.org/D5503

Files:
  include/lldb/Core/ArchSpec.h
  include/lldb/Core/DataExtractor.h
  include/lldb/Core/Section.h
  source/Commands/CommandObjectMemory.cpp
  source/Core/ArchSpec.cpp
  source/Core/DataExtractor.cpp
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
  source/Symbol/ObjectFile.cpp
Index: include/lldb/Core/ArchSpec.h
===================================================================
--- include/lldb/Core/ArchSpec.h
+++ include/lldb/Core/ArchSpec.h
@@ -104,7 +104,6 @@
         eCore_uknownMach32,
         eCore_uknownMach64,
 
-        eCore_kalimba,
         eCore_kalimba3,
         eCore_kalimba4,
         eCore_kalimba5,
@@ -142,7 +141,7 @@
         kCore_hexagon_first  = eCore_hexagon_generic,
         kCore_hexagon_last   = eCore_hexagon_hexagonv5,
 
-        kCore_kalimba_first = eCore_kalimba,
+        kCore_kalimba_first = eCore_kalimba3,
         kCore_kalimba_last = eCore_kalimba5
     };
 
Index: include/lldb/Core/DataExtractor.h
===================================================================
--- include/lldb/Core/DataExtractor.h
+++ include/lldb/Core/DataExtractor.h
@@ -85,8 +85,11 @@
     ///
     /// @param[in] addr_size
     ///     A new address byte size value.
+    ///
+    /// @param[in] target_byte_size
+    ///     A size of a target byte in 8-bit host bytes
     //------------------------------------------------------------------
-    DataExtractor (const void* data, lldb::offset_t data_length, lldb::ByteOrder byte_order, uint32_t addr_size);
+    DataExtractor (const void* data, lldb::offset_t data_length, lldb::ByteOrder byte_order, uint32_t addr_size, uint32_t target_byte_size = 1);
 
     //------------------------------------------------------------------
     /// Construct with shared data.
@@ -104,8 +107,11 @@
     ///
     /// @param[in] addr_size
     ///     A new address byte size value.
+    ///
+    /// @param[in] target_byte_size
+    ///     A size of a target byte in 8-bit host bytes
     //------------------------------------------------------------------
-    DataExtractor (const lldb::DataBufferSP& data_sp, lldb::ByteOrder byte_order, uint32_t addr_size);
+    DataExtractor (const lldb::DataBufferSP& data_sp, lldb::ByteOrder byte_order, uint32_t addr_size, uint32_t target_byte_size = 1);
 
     //------------------------------------------------------------------
     /// Construct with a subset of \a data.
@@ -129,8 +135,11 @@
     ///
     /// @param[in] length
     ///     The length in bytes of the subset of data.
+    ///
+    /// @param[in] target_byte_size
+    ///     A size of a target byte in 8-bit host bytes
     //------------------------------------------------------------------
-    DataExtractor (const DataExtractor& data, lldb::offset_t offset, lldb::offset_t length);
+    DataExtractor (const DataExtractor& data, lldb::offset_t offset, lldb::offset_t length, uint32_t target_byte_size = 1);
 
     DataExtractor (const DataExtractor& rhs);
     //------------------------------------------------------------------
@@ -863,7 +872,7 @@
         *offset_ptr += 1;
         return val;
     }
-    
+
     uint16_t
     GetU16_unchecked (lldb::offset_t *offset_ptr) const;
 
@@ -1311,6 +1320,7 @@
     lldb::ByteOrder m_byte_order;   ///< The byte order of the data we are extracting from.
     uint32_t m_addr_size;           ///< The address size to use when extracting pointers or addresses
     mutable lldb::DataBufferSP m_data_sp; ///< The shared pointer to data that can be shared among multilple instances
+    const uint32_t m_target_byte_size;
 };
 
 } // namespace lldb_private
Index: include/lldb/Core/Section.h
===================================================================
--- include/lldb/Core/Section.h
+++ include/lldb/Core/Section.h
@@ -120,7 +120,8 @@
              lldb::offset_t file_offset,
              lldb::offset_t file_size,
              uint32_t log2align,
-             uint32_t flags);
+             uint32_t flags,
+             uint32_t target_byte_size = 1);
 
     // Create a section that is a child of parent_section_sp
     Section (const lldb::SectionSP &parent_section_sp,    // NULL for top level sections, non-NULL for child sections
@@ -134,7 +135,8 @@
              lldb::offset_t file_offset,
              lldb::offset_t file_size,
              uint32_t log2align,
-             uint32_t flags);
+             uint32_t flags,
+             uint32_t target_byte_size = 1);
 
     ~Section ();
 
@@ -297,6 +299,12 @@
         m_log2align = align;
     }
 
+    // Get the number of host bytes required to hold a target byte
+    uint32_t
+    GetTargetByteSize() const
+    {
+        return m_target_byte_size; 
+    }     
 
 protected:
 
@@ -317,6 +325,8 @@
                                         // hits unless the children contain the address.
                     m_encrypted:1,      // Set to true if the contents are encrypted
                     m_thread_specific:1;// This section is thread specific
+    uint32_t        m_target_byte_size; // Some architectures have non-8-bit byte size. This is specified as
+                                        // as a multiple number of a host bytes   
 private:
     DISALLOW_COPY_AND_ASSIGN (Section);
 };
Index: source/Commands/CommandObjectMemory.cpp
===================================================================
--- source/Commands/CommandObjectMemory.cpp
+++ source/Commands/CommandObjectMemory.cpp
@@ -614,7 +614,16 @@
         }
 
         size_t item_count = m_format_options.GetCountValue().GetCurrentValue();
-        size_t item_byte_size = m_format_options.GetByteSizeValue().GetCurrentValue();
+
+        // TODO For non-8-bit byte addressable architectures this needs to be
+        // revisited to fully support all lldb's range of formatting options.
+        // Furthermore code memory reads (for those architectures) will not
+        // be correctly formatted even w/o formatting options.
+        size_t item_byte_size =
+            target->GetArchitecture().GetDataByteSize() > 1 ?
+            target->GetArchitecture().GetDataByteSize() :
+            m_format_options.GetByteSizeValue().GetCurrentValue();
+
         const size_t num_per_line = m_memory_options.m_num_per_line.GetCurrentValue();
 
         if (total_byte_size == 0)
@@ -661,7 +670,7 @@
             total_byte_size = end_addr - addr;
             item_count = total_byte_size / item_byte_size;
         }
-        
+
         uint32_t max_unforced_size = target->GetMaximumMemReadSize();
         
         if (total_byte_size > max_unforced_size && !m_memory_options.m_force)
@@ -858,7 +867,8 @@
         result.SetStatus(eReturnStatusSuccessFinishResult);
         DataExtractor data (data_sp, 
                             target->GetArchitecture().GetByteOrder(), 
-                            target->GetArchitecture().GetAddressByteSize());
+                            target->GetArchitecture().GetAddressByteSize(),
+                            target->GetArchitecture().GetDataByteSize());
         
         Format format = m_format_options.GetFormat();
         if ( ( (format == eFormatChar) || (format == eFormatCharPrintable) )
@@ -892,7 +902,7 @@
                                          format,
                                          item_byte_size,
                                          item_count,
-                                         num_per_line,
+                                         num_per_line / target->GetArchitecture().GetDataByteSize(),
                                          addr,
                                          0,
                                          0,
Index: source/Core/ArchSpec.cpp
===================================================================
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -118,7 +118,6 @@
     { eByteOrderLittle, 4, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach32  , "unknown-mach-32" },
     { eByteOrderLittle, 8, 4, 4 , llvm::Triple::UnknownArch , ArchSpec::eCore_uknownMach64  , "unknown-mach-64" },
 
-    { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba  , "kalimba" },
     { eByteOrderBig   , 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba3  , "kalimba3" },
     { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba4  , "kalimba4" },
     { eByteOrderLittle, 4, 1, 1 , llvm::Triple::kalimba , ArchSpec::eCore_kalimba5  , "kalimba5" }
@@ -264,11 +263,9 @@
     { ArchSpec::eCore_x86_64_x86_64   , llvm::ELF::EM_X86_64 , LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // AMD64
     { ArchSpec::eCore_mips64          , llvm::ELF::EM_MIPS   , LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // MIPS
     { ArchSpec::eCore_hexagon_generic , llvm::ELF::EM_HEXAGON, LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu }, // HEXAGON
-    { ArchSpec::eCore_kalimba ,         llvm::ELF::EM_CSR_KALIMBA, LLDB_INVALID_CPUTYPE, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
-    { ArchSpec::eCore_kalimba3 ,        llvm::ELF::EM_CSR_KALIMBA, 3, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
-    { ArchSpec::eCore_kalimba4 ,        llvm::ELF::EM_CSR_KALIMBA, 4, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
-    { ArchSpec::eCore_kalimba5 ,        llvm::ELF::EM_CSR_KALIMBA, 5, 0xFFFFFFFFu, 0xFFFFFFFFu }  // KALIMBA
-
+    { ArchSpec::eCore_kalimba3 ,        llvm::ELF::EM_CSR_KALIMBA, llvm::Triple::KalimbaSubArch_v3, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
+    { ArchSpec::eCore_kalimba4 ,        llvm::ELF::EM_CSR_KALIMBA, llvm::Triple::KalimbaSubArch_v4, 0xFFFFFFFFu, 0xFFFFFFFFu },  // KALIMBA
+    { ArchSpec::eCore_kalimba5 ,        llvm::ELF::EM_CSR_KALIMBA, llvm::Triple::KalimbaSubArch_v5, 0xFFFFFFFFu, 0xFFFFFFFFu }  // KALIMBA
 };
 
 static const ArchDefinition g_elf_arch_def = {
@@ -503,11 +500,11 @@
     switch (m_core)
     {
     case eCore_kalimba3:
-        return 3;        
+        return 4;        
     case eCore_kalimba4:
         return 1;        
     case eCore_kalimba5:
-        return 3;
+        return 4;
     default:        
         return 1;        
     }
@@ -1036,16 +1033,6 @@
         }
         break;
 
-    case ArchSpec::eCore_kalimba:
-    case ArchSpec::eCore_kalimba3:
-    case ArchSpec::eCore_kalimba4:
-    case ArchSpec::eCore_kalimba5:
-        if (core2 >= ArchSpec::kCore_kalimba_first && core2 <= ArchSpec::kCore_kalimba_last)
-        {
-            return true;
-        }
-        break;
-
     case ArchSpec::eCore_arm_armv8:
         if (!enforce_exact_match)
         {
Index: source/Core/DataExtractor.cpp
===================================================================
--- source/Core/DataExtractor.cpp
+++ source/Core/DataExtractor.cpp
@@ -132,20 +132,22 @@
     m_end       (NULL),
     m_byte_order(lldb::endian::InlHostByteOrder()),
     m_addr_size (4),
-    m_data_sp   ()
+    m_data_sp   (),
+    m_target_byte_size(1)
 {
 }
 
 //----------------------------------------------------------------------
 // This constructor allows us to use data that is owned by someone else.
 // The data must stay around as long as this object is valid.
 //----------------------------------------------------------------------
-DataExtractor::DataExtractor (const void* data, offset_t length, ByteOrder endian, uint32_t addr_size) :
+DataExtractor::DataExtractor (const void* data, offset_t length, ByteOrder endian, uint32_t addr_size, uint32_t target_byte_size/*=1*/) :
     m_start     ((uint8_t*)data),
     m_end       ((uint8_t*)data + length),
     m_byte_order(endian),
     m_addr_size (addr_size),
-    m_data_sp   ()
+    m_data_sp   (),
+    m_target_byte_size(target_byte_size)
 {
 }
 
@@ -156,12 +158,13 @@
 // as long as any DataExtractor objects exist that have a reference to
 // this data.
 //----------------------------------------------------------------------
-DataExtractor::DataExtractor (const DataBufferSP& data_sp, ByteOrder endian, uint32_t addr_size) :
+DataExtractor::DataExtractor (const DataBufferSP& data_sp, ByteOrder endian, uint32_t addr_size, uint32_t target_byte_size/*=1*/) :
     m_start     (NULL),
     m_end       (NULL),
     m_byte_order(endian),
     m_addr_size (addr_size),
-    m_data_sp   ()
+    m_data_sp   (),
+    m_target_byte_size(target_byte_size)
 {
     SetData (data_sp);
 }
@@ -173,12 +176,13 @@
 // as any object contains a reference to that data. The endian
 // swap and address size settings are copied from "data".
 //----------------------------------------------------------------------
-DataExtractor::DataExtractor (const DataExtractor& data, offset_t offset, offset_t length) :
+DataExtractor::DataExtractor (const DataExtractor& data, offset_t offset, offset_t length, uint32_t target_byte_size/*=1*/) :
     m_start(NULL),
     m_end(NULL),
     m_byte_order(data.m_byte_order),
     m_addr_size(data.m_addr_size),
-    m_data_sp()
+    m_data_sp(),
+    m_target_byte_size(target_byte_size)
 {
     if (data.ValidOffset(offset))
     {
@@ -194,7 +198,8 @@
     m_end (rhs.m_end),
     m_byte_order (rhs.m_byte_order),
     m_addr_size (rhs.m_addr_size),
-    m_data_sp (rhs.m_data_sp)
+    m_data_sp (rhs.m_data_sp),
+    m_target_byte_size(rhs.m_target_byte_size)
 {
 }
 
@@ -1480,7 +1485,9 @@
                 s->EOL();
             }
             if (base_addr != LLDB_INVALID_ADDRESS)
-                s->Printf ("0x%8.8" PRIx64 ": ", (uint64_t)(base_addr + (offset - start_offset)));
+                s->Printf ("0x%8.8" PRIx64 ": ",
+                    (uint64_t)(base_addr + (offset - start_offset)/m_target_byte_size  ));
+
             line_start_offset = offset;
         }
         else
@@ -1535,6 +1542,7 @@
             {
                 s->Printf ("%2.2x", GetU8(&offset));
             }
+
             // Put an extra space between the groups of bytes if more than one
             // is being dumped in a group (item_byte_size is more than 1).
             if (item_byte_size > 1)
Index: source/Core/Section.cpp
===================================================================
--- source/Core/Section.cpp
+++ source/Core/Section.cpp
@@ -28,7 +28,8 @@
                   lldb::offset_t file_offset,
                   lldb::offset_t file_size,
                   uint32_t log2align,
-                  uint32_t flags) :
+                  uint32_t flags,
+                  uint32_t target_byte_size/*=1*/) :
     ModuleChild     (module_sp),
     UserID          (sect_id),
     Flags           (flags),
@@ -44,7 +45,8 @@
     m_children      (),
     m_fake          (false),
     m_encrypted     (false),
-    m_thread_specific (false)
+    m_thread_specific (false),
+    m_target_byte_size(target_byte_size)
 {
 //    printf ("Section::Section(%p): module=%p, sect_id = 0x%16.16" PRIx64 ", addr=[0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), file [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), flags = 0x%8.8x, name = %s\n",
 //            this, module_sp.get(), sect_id, file_addr, file_addr + byte_size, file_offset, file_offset + file_size, flags, name.GetCString());
@@ -61,7 +63,8 @@
                   lldb::offset_t file_offset,
                   lldb::offset_t file_size,
                   uint32_t log2align,
-                  uint32_t flags) :
+                  uint32_t flags,
+                  uint32_t target_byte_size/*=1*/) :
     ModuleChild     (module_sp),
     UserID          (sect_id),
     Flags           (flags),
@@ -77,7 +80,8 @@
     m_children      (),
     m_fake          (false),
     m_encrypted     (false),
-    m_thread_specific (false)
+    m_thread_specific (false),
+    m_target_byte_size(target_byte_size)
 {
 //    printf ("Section::Section(%p): module=%p, sect_id = 0x%16.16" PRIx64 ", addr=[0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), file [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 "), flags = 0x%8.8x, name = %s.%s\n",
 //            this, module_sp.get(), sect_id, file_addr, file_addr + byte_size, file_offset, file_offset + file_size, flags, parent_section_sp->GetName().GetCString(), name.GetCString());
@@ -186,7 +190,7 @@
     {
         if (file_addr <= vm_addr)
         {
-            const addr_t offset = vm_addr - file_addr;
+            const addr_t offset = (vm_addr - file_addr) *  m_target_byte_size;
             return offset < GetByteSize();
         }
     }
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -267,10 +267,14 @@
     {
         // TODO(mg11) Support more variants
         case 10:
-            kal_arch_variant = 3;
+            kal_arch_variant = llvm::Triple::KalimbaSubArch_v3;
             break;
         case 14:
-            kal_arch_variant = 4;
+            kal_arch_variant = llvm::Triple::KalimbaSubArch_v4;
+            break;
+        case 17:
+        case 20:
+            kal_arch_variant = llvm::Triple::KalimbaSubArch_v5;
             break;
         default:
             break;           
@@ -287,6 +291,27 @@
         LLDB_INVALID_CPUTYPE;
 }
 
+//! brief kalimbaSectionType
+//! The kalimba toolchain identifies a code section as being
+//! one with the SHT_PROGBITS set in the section sh_type and the top
+//! bit in the 32-bit address field set.
+static lldb::SectionType
+kalimbaSectionType(
+    const elf::ELFHeader& header,
+    const elf::ELFSectionHeader& sect_hdr)
+{
+    if (
+        llvm::ELF::EM_CSR_KALIMBA != header.e_machine ||
+        llvm::ELF::SHT_PROGBITS != sect_hdr.sh_type)
+    {
+        return eSectionTypeOther;
+    }
+
+    const lldb::addr_t KAL_CODE_BIT = 1 << 31;
+    return KAL_CODE_BIT & sect_hdr.sh_addr ?
+         eSectionTypeCode  : eSectionTypeData;
+}
+
 // Arbitrary constant used as UUID prefix for core files.
 const uint32_t
 ObjectFileELF::g_core_uuid_magic(0xE210C);
@@ -1592,6 +1617,20 @@
                     break;
             }
 
+            if (eSectionTypeOther == sect_type)
+            {
+                // the kalimba toolchain assumes that ELF section names are free-form. It does
+                // supports linkscripts which (can) give rise to various arbitarily named
+                // sections being "Code" or "Data". 
+                sect_type = kalimbaSectionType(m_header, header);
+            }
+
+            const uint32_t target_bytes_size =
+                eSectionTypeData == sect_type ? 
+                m_arch_spec.GetDataByteSize() :
+                    eSectionTypeCode == sect_type ?
+                    m_arch_spec.GetCodeByteSize() : 1;
+
             elf::elf_xword log2align = (header.sh_addralign==0)
                                         ? 0
                                         : llvm::Log2_64(header.sh_addralign);
@@ -1605,7 +1644,8 @@
                                               header.sh_offset,   // Offset of this section in the file.
                                               file_size,          // Size of the section as found in the file.
                                               log2align,          // Alignment of the section
-                                              header.sh_flags));  // Flags for this section.
+                                              header.sh_flags,    // Flags for this section.
+                                              target_bytes_size));// Number of host bytes per target byte
 
             if (is_thread_specific)
                 section_sp->SetIsThreadSpecific (is_thread_specific);
Index: source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
===================================================================
--- source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
+++ source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
@@ -250,7 +250,17 @@
 {
     if (idx == 0)
     {
-        arch = ArchSpec("kalimba-csr-unknown");
+        arch = ArchSpec("kalimba3-csr-unknown");
+        return true;
+    }
+    if (idx == 1)
+    {
+        arch = ArchSpec("kalimba4-csr-unknown");
+        return true;
+    }
+    if (idx == 2)
+    {
+        arch = ArchSpec("kalimba5-csr-unknown");
         return true;
     }
     return false;
Index: source/Symbol/ObjectFile.cpp
===================================================================
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -458,6 +458,9 @@
 size_t
 ObjectFile::ReadSectionData (const Section *section, lldb::offset_t section_offset, void *dst, size_t dst_len) const
 {
+    assert(section);
+    section_offset *= section->GetTargetByteSize();
+
     // If some other objectfile owns this data, pass this to them.
     if (section->GetObjectFile() != this)
         return section->GetObjectFile()->ReadSectionData (section, section_offset, dst, dst_len);
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Reply via email to