ObjectFileELF: Add support for arbitrarily named code sections

[kbaladurin]

Summary

ObjectFileELF assumes that code section has ".text" name. There is an exception for kalimba toolchain that can use arbitrary names, but other toolchains also could use arbitrary names for code sections. For example, corert uses separate section for compiled managed code. As lldb doesn't recognize such section it leads to problem with breakpoints on arm, because debugger cannot determine instruction set (arm/thumb) and uses incorrect breakpoint opcode that breaks program execution.

This change allows debugger to correctly handle such code sections. We assume that section is a code section if it has SHF_EXECINSTR flag set and has SHT_PROGBITS type.

[davide]

This needs a testcase.

[clayborg]

Quick test case where we have an ELF file with a .text section and with another executable section with something in it that we can set breakpoints in. Easiest to make an ELF file that fits the bill, and then run obj2yaml on it. You check that into a test case directory and have the test case run yaml2obj on it and run the test.

[labath]

We already have section dumping code in the lldb-test utility. Extending the output to dump the detected section type seems like a good (and simple) thing to do.

[kbaladurin]

Thank you! What is more preferable add python or lit test?

[davide]

lldb-test for this purpose will be great. there should be examples in lit/.

[kbaladurin]

Thank you! I've extended lldb-test utility to dump section type:

Showing 6 sections
  Index: 0
  Type: regular
  Name: 
  VM size: 0
  File size: 0
  
  Index: 1
  Type: code
  Name: .text
  VM size: 8
  File size: 8
  
  Index: 2
  Type: code
  Name: __codesection
  VM size: 8
  File size: 8
  
  Index: 3
  Type: elf-symbol-table
  Name: .symtab
  VM size: 0
  File size: 16
  ...

and added lit test to check section type and arm test in python test suite to check breakpoints in arbitrary named thumb code sections.

[davide]

I think this is almost ready to go in modulo minors. I'll let also @labath comment on it. Thanks for your contribution!
Do you need somebody to commit this on your behalf?

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py

2 ↗

(On Diff #140609)

nit: arbitrary.

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1950–1954 ↗

(On Diff #140609)

can you add a comment explaining why this is needed?

[kbaladurin]

@davide thank you! I've updated diff. Yes, I need somebody to commit this, as I haven't commit access.

[clayborg]

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c

1 ↗

(On Diff #140628)

Will this work with all compilers we currently run the test suite with? I would assume with will work with GCC and Clang at least. IF not, we might need to make a lldbtest.h file that any test case can use and use a macro here?

source/Core/Section.cpp

30 ↗

(On Diff #140628)

Why did you take static off of this function? Please remove this change, or change this function to get the section type from the section itself and not require the argument.

[kbaladurin]

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c

1 ↗

(On Diff #140628)

What compilers do we use with test suite? This construction works fine with gcc and clang. Is it enough for us?

source/Core/Section.cpp

30 ↗

(On Diff #140628)

I change it to static method to use it in lldb-test. There is similar static methods in Value and Scalar classes: Value::GetValueTypeAsCString and Scalar::GetValueTypeAsCString. Is non static method more preferable for us in this case?

[davide]

LGTM. I'll commit for you once Greg reviews it again.

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c

1 ↗

(On Diff #140628)

I think this is fine.
BTW, if we really want to abstract this, the place is not lldb but llvm.
We already have an header for the purpose.

llvm/Support/Compiler.h

But again, I don't think this is needed.

source/Core/Section.cpp

30 ↗

(On Diff #140628)

I think what you did was correct. Greg?

[labath]

The patch looks fine to me. I think I'd make the arm test (I guess that is interesting because of the bit 0 twiddling ?) a non-execution test, but this is fine as well.

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py

29 ↗

(On Diff #140628)

Do you think there's any added value in making sure that the breakpoint is *hit* (instead of just making sure that it's resolved to the right file address). If it's the latter we could make this a non-execution test and have it run everywhere (e.g., via lldb-test breakpoint) instead of just on arm hardware.

[clayborg]

source/Core/Section.cpp

30 ↗

(On Diff #140628)

Woops, I missed the extra "Section::" that was added. You indeed did do this right... I thought in my mind that the "Section::" was there prior to this change and we were making the function a member function. My bad.

[kbaladurin]

packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py

29 ↗

(On Diff #140628)

I think we need to make sure that breakpoint is *hit* and program can successfully continue execution after it, because it is resolved to right address in both cases:

lldb without this patch:

$ llvm-arm/bin/lldb thumb-bp 
(lldb) target create "thumb-bp"
Current executable set to 'thumb-bp' (arm).
(lldb) b bp.c:3
Breakpoint 1: where = thumb-bp`f + 6 at bp.c:3, address = 0x000103f6
(lldb) r
Process 22192 launched: '/home/kbaladurin/thumb-bp' (arm)
Process 22192 stopped
* thread #1: tid = 22192, 0x000103d0 thumb-bp`__libc_csu_init + 32, name = 'thumb-bp', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: 0x000103d0 thumb-bp`__libc_csu_init + 32
thumb-bp`__libc_csu_init:
->  0x103d0 <+32>: ldr    r3, [r5], #4
    0x103d4 <+36>: mov    r2, r9
    0x103d6 <+38>: mov    r1, r8
    0x103d8 <+40>: mov    r0, r7
(lldb) disas -n f
thumb-bp`f:
    0x103f0 <+0>:  strmi  r11, [r1], -r2, lsl #1
    0x103f4 <+4>:  stmdals r1, {r0, r12, pc}
    0x103f8 <+8>:  .long  0x91003001                ; unknown opcode
    0x103fc <+12>: ldrbmi r11, [r0, -r2]!

lldb with this patch:

$ llvm-arm-patch/bin/lldb thumb-bp 
(lldb) target create "thumb-bp"
Current executable set to 'thumb-bp' (arm).
(lldb) b bp.c:3
Breakpoint 1: where = thumb-bp`f + 6 at bp.c:3, address = 0x000103f6
(lldb) r
Process 22282 launched: '/home/kbaladurin/thumb-bp' (arm)
Process 22282 stopped
* thread #1: tid = 22282, 0x000103f6 thumb-bp`f(a=10) + 6 at bp.c:3, name = 'thumb-bp', stop reason = breakpoint 1.1
    frame #0: 0x000103f6 thumb-bp`f(a=10) + 6 at bp.c:3
   1   	__attribute__((section("__codesection")))
   2   	int f(int a) {
-> 3   	  return a + 1; // Set break point at this line.
   4   	}
   5   	
   6   	int main() {
   7   	  return f(10);
(lldb) disas
thumb-bp`f:
    0x103f0 <+0>:  sub    sp, #0x8
    0x103f2 <+2>:  mov    r1, r0
    0x103f4 <+4>:  str    r0, [sp, #0x4]
->  0x103f6 <+6>:  ldr    r0, [sp, #0x4]
    0x103f8 <+8>:  adds   r0, #0x1
    0x103fa <+10>: str    r1, [sp]
    0x103fc <+12>: add    sp, #0x8
    0x103fe <+14>: bx     lr

In both cases resolved address is 0x103f6 that is right according to the debug information:

Line Number Statements:
 [0x00000025]  Extended opcode 2: set Address to 0x103f0
 [0x0000002c]  Special opcode 6: advance Address by 0 to 0x103f0 and Line by 1 to 2
 [0x0000002d]  Set column to 10
 [0x0000002f]  Set prologue_end to true
 [0x00000030]  Special opcode 90: advance Address by 6 to 0x103f6 and Line by 1 to 3

But lldb uses arm breakpoint in the first case and thumb one in the second.

[labath]

I see, thanks for explaining that.

[kbaladurin]

Is it ready to commit?

[labath]

Yes, I take it you need someone to check it in?

[kbaladurin]

Yes, as I haven't commit access. Thank you!

[labath]

I've committed this as r331173. I've had to revert the lldb-test changes, as something similar has been implemented already, and I've merged your lldb-test test with an existing section types test.

[kbaladurin]

Thank you very much!

lldb/trunk/lit/Modules/dwarf-gnu-debugaltlink.yaml

-# RUN: yaml2obj %s > %t
-# RUN: lldb-test module-sections %t | FileCheck %s
-
-# CHECK: Name: .gnu_debugaltlink
-# CHECK-NEXT: Type: dwarf-gnu-debugaltlink
-# CHECK-NEXT: VM size: 0
-# CHECK-NEXT: File size: 8
-
---- !ELF
-FileHeader:      
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_DYN
-  Machine:         EM_X86_64
-  Entry:           0x00000000000007A0
-Sections:        
-  - Name:            .debug_info
-    Type:            SHT_PROGBITS
-    AddressAlign:    0x0000000000000001
-    Content:         DEADBEEFBAADF00D
-  - Name:            .gnu_debugaltlink
-    Type:            SHT_PROGBITS
-    AddressAlign:    0x0000000000000001
-    Content:         DEADBEEFBAADF00D
-...

lldb/trunk/lit/Modules/elf-section-types.yaml

+# RUN: yaml2obj %s > %t
+# RUN: lldb-test module-sections %t | FileCheck %s
+
+# CHECK: Name: .text
+# CHECK-NEXT: Type: code
+
+# CHECK: Name: .gnu_debugaltlink
+# CHECK-NEXT: Type: dwarf-gnu-debugaltlink
+# CHECK-NEXT: VM size: 0
+# CHECK-NEXT: File size: 8
+
+# CHECK: Name: __codesection
+# CHECK-NEXT: Type: code
+
+--- !ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+  Entry:           0x00000000000007A0
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content:         DEADBEEFBAADF00D
+  - Name:            .debug_info
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         DEADBEEFBAADF00D
+  - Name:            .gnu_debugaltlink
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x0000000000000001
+    Content:         DEADBEEFBAADF00D
+  - Name:            __codesection
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content:         DEADBEEFBAADF00D
+...

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile

+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS = -mthumb
+
+include $(LEVEL)/Makefile.rules
 No newline at end of file

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py

+"""
+Test that breakpoints correctly work in an thumb function in an arbitrary
+named codesection.
+"""
+from __future__ import print_function
+
+
+import lldb
+import os
+import time
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBreakpointThumbCodesection(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIf(archs=no_match(["arm"]))
+    def test_breakpoint(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        line = line_number('main.c', '// Set break point at this line.')
+
+        self.runCmd("target create %s" % exe)
+        bpid = lldbutil.run_break_set_by_file_and_line(self, "main.c", line)
+
+        self.runCmd("run")
+
+        self.assertIsNotNone(lldbutil.get_one_thread_stopped_at_breakpoint_id(
+            self.process(), bpid), "Process is not stopped at breakpoint")
+
+        self.process().Continue()
+        self.assertEqual(self.process().GetState(), lldb.eStateExited, PROCESS_EXITED)

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c

+__attribute__((section("__codesection")))
+int f(int a) {
+  return a + 1; // Set break point at this line.
+}
+
+int main() {
+  return f(10);
+}

lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

for (SectionHeaderCollIter I = m_section_headers.begin();
         // the kalimba toolchain assumes that ELF section names are free-form.
         // It does
         // support linkscripts which (can) give rise to various arbitrarily
         // named
         // sections being "Code" or "Data".
         sect_type = kalimbaSectionType(m_header, header);
       }
 
+      // In common case ELF code section can have arbitrary name (for example,
+      // we can specify it using section attribute for particular function) so
+      // assume that section is a code section if it has SHF_EXECINSTR flag set
+      // and has SHT_PROGBITS type.
+      if (eSectionTypeOther == sect_type &&
+          llvm::ELF::SHT_PROGBITS == header.sh_type &&
+          (header.sh_flags & SHF_EXECINSTR)) {
+        sect_type = eSectionTypeCode;
+      }
+
       const uint32_t target_bytes_size =
           (eSectionTypeData == sect_type || eSectionTypeZeroFill == 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);