[lldb] add SBSection.alignment to python bindings

[dmlary]

Summary

This commit adds SBSection.GetAlignment(), and SBSection.alignment as a python property to lldb.

[JDevlieghere]

Is this something we could add a test for?

[dmlary]

I went through the existing tests for SBSection, and there is only one test case for all the getters & props, and that is for target_byte_size. Based on that lack, and the simplicity of the getter, I didn't think a test case was warranted here.

If the reviewers feel a test is necessary here, I can definitely add one.

[JDevlieghere]

In D128069#3592733, @dmlary wrote:

I went through the existing tests for SBSection, and there is only one test case for all the getters & props, and that is for target_byte_size. Based on that lack, and the simplicity of the getter, I didn't think a test case was warranted here.

I'm less concerned about the getter and the property and more about the underlying functionality. It doesn't look like it's being tested, and adding it to the SB API allows us to change that.

If the reviewers feel a test is necessary here, I can definitely add one.

Yes, unless there's a good reason not too, every change should be accompanied by a test.

[labath]

For the underlying functionality, I think it'd be best to extend lldb-test to print section alignment (dumpSectionList in lldb/tools/lldb-test/lldb-test.cpp) and then write (or extend) some test in test/Shell/ObjectFile/ELF (or your favourite object format).

That won't test the code added here though, as lldb-test does not use the SB API.

[dmlary]

Sent this via email, but doesn't look like it showed up here:

I'm less concerned about the getter and the property and more about the underlying functionality. It doesn't look like it's being tested, and adding it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, Section.GetLog2Align()? Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?

[clayborg]

In D128069#3599456, @dmlary wrote:

Sent this via email, but doesn't look like it showed up here:

I'm less concerned about the getter and the property and more about the underlying functionality. It doesn't look like it's being tested, and adding it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, Section.GetLog2Align()? Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?

We should test any APIs we add in a python test IMHO. Also testing that the ".alignment" property works

[dmlary]

We should test any APIs we add in a python test IMHO. Also testing that the ".alignment" property works

Ok, I'll add tests for the added python function and property.

Now for the hard part, what's the recommended way to access elf section details within python without using the function added in this commit? I do not think it is safe to hard code the expected alignment in the test case as the default alignment may vary across architectures. Shelling out to readelf and parsing is possible, but feels clunky; is there an existing function to get this data?

[labath]

In D128069#3602256, @dmlary wrote:

We should test any APIs we add in a python test IMHO. Also testing that the ".alignment" property works

Ok, I'll add tests for the added python function and property.

Now for the hard part, what's the recommended way to access elf section details within python without using the function added in this commit? I do not think it is safe to hard code the expected alignment in the test case as the default alignment may vary across architectures. Shelling out to readelf and parsing is possible, but feels clunky; is there an existing function to get this data?

You can use yaml2obj to create sections with known alignments. You should be able to find examples of that in the existing tests (just grep for yaml2obj).

[dmlary]

updated to include test cases for SBSection.GetAlignment() and SBSection.alignment property.

@labath Thanks for the yaml2obj pointer.

[labath]

In D128069#3619736, @dmlary wrote:

updated to include test cases for SBSection.GetAlignment() and SBSection.alignment property.

Cool. Looks good, just a couple of small comments inline.

lldb/test/API/python_api/section/TestSectionAPI.py

43

we normally use the .yaml extension.

59

I don't think this is worth a separate test, I'd just move it next to the other check.

[dmlary]

merge tests, rename yml to yaml

[clayborg]

Looks good, thanks for the revisions

[dmlary]

@JDevlieghere if I'm reading this right, yours is the last approval this review needs. Is there anything else you would like to be updated in this diff?

[JDevlieghere]

LGTM!

[dmlary]

I do not have commit access; could someone merge this patch as time allows?

lldb/bindings/interface/SBSection.i

public:
     varying minimum addressable unit (i.e. byte) size for their
     CODE or DATA buses.
 
     @return
         The number of host (8-bit) bytes needed to hold a target byte") GetTargetByteSize;
     uint32_t
     GetTargetByteSize ();
 
+    uint32_t
+    GetAlignment ();
+
     bool
     GetDescription (lldb::SBStream &description);
 
     bool
     operator == (const lldb::SBSection &rhs);
 
     bool
     operator != (const lldb::SBSection &rhs);
%pythoncode %{
         addr = property(get_addr, None, doc='''A read only property that returns an lldb object that represents the start address (lldb.SBAddress) for this section.''')
         file_addr = property(GetFileAddress, None, doc='''A read only property that returns an integer that represents the starting "file" address for this section, or the address of the section in the object file in which it is defined.''')
         size = property(GetByteSize, None, doc='''A read only property that returns the size in bytes of this section as an integer.''')
         file_offset = property(GetFileOffset, None, doc='''A read only property that returns the file offset in bytes of this section as an integer.''')
         file_size = property(GetFileByteSize, None, doc='''A read only property that returns the file size in bytes of this section as an integer.''')
         data = property(GetSectionData, None, doc='''A read only property that returns an lldb object that represents the bytes for this section (lldb.SBData) for this section.''')
         type = property(GetSectionType, None, doc='''A read only property that returns an lldb enumeration value (see enumerations that start with "lldb.eSectionType") that represents the type of this section (code, data, etc.).''')
         target_byte_size = property(GetTargetByteSize, None, doc='''A read only property that returns the size of a target byte represented by this section as a number of host bytes.''')
+        alignment = property(GetAlignment, None, doc='''A read only property that returns the alignment of this section as a number of host bytes.''')
     %}
 #endif
 
 private:
 
     std::unique_ptr<lldb_private::SectionImpl> m_opaque_ap;
 };
 
 } // namespace lldb

lldb/include/lldb/API/SBSection.h

public:
   /// in numbers of host bytes. Note that certain architectures have
   /// varying minimum addressable unit (i.e. byte) size for their
   /// CODE or DATA buses.
   ///
   /// \return
   ///     The number of host (8-bit) bytes needed to hold a target byte
   uint32_t GetTargetByteSize();
 
+  /// Return the alignment of the section in bytes
+  ///
+  /// \return
+  ///     The alignment of the section in bytes
+  uint32_t GetAlignment();
+
   bool operator==(const lldb::SBSection &rhs);
 
   bool operator!=(const lldb::SBSection &rhs);
 
   bool GetDescription(lldb::SBStream &description);
 
 private:
   friend class SBAddress;

lldb/source/API/SBSection.cpp

uint32_t SBSection::GetTargetByteSize() {
   LLDB_INSTRUMENT_VA(this);
 
   SectionSP section_sp(GetSP());
   if (section_sp.get())
     return section_sp->GetTargetByteSize();
   return 0;
 }
 
+uint32_t SBSection::GetAlignment() {
+  LLDB_INSTRUMENT_VA(this);
+
+  SectionSP section_sp(GetSP());
+  if (section_sp.get())
+    return (1 << section_sp->GetLog2Align());
+  return 0;
+}
+
 bool SBSection::operator==(const SBSection &rhs) {
   LLDB_INSTRUMENT_VA(this, rhs);
 
   SectionSP lhs_section_sp(GetSP());
   SectionSP rhs_section_sp(rhs.GetSP());
   if (lhs_section_sp && rhs_section_sp)
     return lhs_section_sp == rhs_section_sp;
   return false;

lldb/test/API/python_api/section/TestSectionAPI.py

def test_get_target_byte_size(self):
                     ss = s.GetSubSectionAtIndex(i)
                     sect_type = ss.GetSectionType()
                     if sect_type == lldb.eSectionTypeData:
                         data_section = ss
                         break
 
         self.assertIsNotNone(data_section)
         self.assertEqual(data_section.target_byte_size, 1)
+
+    def test_get_alignment(self):
+        exe = self.getBuildArtifact("aligned.out")
+        self.yaml2obj("aligned.yaml", exe)
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # exe contains a single section aligned to 0x1000
+        section = target.modules[0].sections[0]
+        self.assertEqual(section.GetAlignment(), 0x1000)
+        self.assertEqual(section.alignment, 0x1000)

lldb/test/API/python_api/section/aligned.yaml

+--- !ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+  Entry:           0x0000000000400000
+Sections:        
+  - Name:            .text1
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x0000000000400000
+    AddressAlign:    0x0000000000001000
+    Size:            0xb0