[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin created this revision.
kbaladurin added a reviewer: clayborg.
kbaladurin added a project: LLDB.
Herald added subscribers: llvm-commits, kristof.beyls, arichardson, emaste.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D44998

Files:
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1947,6 +1947,12 @@
 sect_type = kalimbaSectionType(m_header, header);
   }
 
+  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()


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1947,6 +1947,12 @@
 sect_type = kalimbaSectionType(m_header, header);
   }
 
+  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()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added a comment.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin updated this revision to Diff 140609.
kbaladurin added a comment.
Herald added a subscriber: javed.absar.

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.


https://reviews.llvm.org/D44998

Files:
  include/lldb/Core/Section.h
  lit/Modules/elf-code-section.yaml
  packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
  
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
  packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -212,6 +212,8 @@
   auto S = Sections->GetSectionAtIndex(I);
   assert(S);
   Printer.formatLine("Index: {0}", I);
+  Printer.formatLine("Type: {0}",
+ Section::GetSectionTypeAsCString(S->GetType()));
   Printer.formatLine("Name: {0}", S->GetName().GetStringRef());
   Printer.formatLine("VM size: {0}", S->GetByteSize());
   Printer.formatLine("File size: {0}", S->GetFileSize());
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1947,6 +1947,12 @@
 sect_type = kalimbaSectionType(m_header, header);
   }
 
+  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()
Index: source/Core/Section.cpp
===
--- source/Core/Section.cpp
+++ source/Core/Section.cpp
@@ -27,7 +27,7 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static const char *GetSectionTypeAsCString(lldb::SectionType sect_type) {
+const char *Section::GetSectionTypeAsCString(lldb::SectionType sect_type) {
   switch (sect_type) {
   case eSectionTypeInvalid:
 return "invalid";
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
@@ -0,0 +1,8 @@
+__attribute__((section("__codesection")))
+int f(int a) {
+  return a + 1; // Set break point at this line.
+}
+
+int main() {
+  return f(10);
+}
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
@@ -0,0 +1,35 @@
+"""
+Test that breakpoints correctly work in an thumb function in an atritbary
+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)
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS = -mthumb

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin updated this revision to Diff 140628.
kbaladurin added a comment.

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


https://reviews.llvm.org/D44998

Files:
  include/lldb/Core/Section.h
  lit/Modules/elf-code-section.yaml
  packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
  
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
  packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
  source/Core/Section.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -212,6 +212,8 @@
   auto S = Sections->GetSectionAtIndex(I);
   assert(S);
   Printer.formatLine("Index: {0}", I);
+  Printer.formatLine("Type: {0}",
+ Section::GetSectionTypeAsCString(S->GetType()));
   Printer.formatLine("Name: {0}", S->GetName().GetStringRef());
   Printer.formatLine("VM size: {0}", S->GetByteSize());
   Printer.formatLine("File size: {0}", S->GetFileSize());
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1947,6 +1947,16 @@
 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()
Index: source/Core/Section.cpp
===
--- source/Core/Section.cpp
+++ source/Core/Section.cpp
@@ -27,7 +27,7 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static const char *GetSectionTypeAsCString(lldb::SectionType sect_type) {
+const char *Section::GetSectionTypeAsCString(lldb::SectionType sect_type) {
   switch (sect_type) {
   case eSectionTypeInvalid:
 return "invalid";
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c
@@ -0,0 +1,8 @@
+__attribute__((section("__codesection")))
+int f(int a) {
+  return a + 1; // Set break point at this line.
+}
+
+int main() {
+  return f(10);
+}
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py
@@ -0,0 +1,35 @@
+"""
+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)
Index: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
===
--- /dev/null
+++ packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS = -mthumb
+
+include $(LEVEL)/Makefile.rules
\ No newline at end of file
Index: lit/Modules/elf-code-section.yaml
===
--- /dev/null
+++ lit/Modules/elf-code-section.yaml
@@ -0,0 +

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added inline comments.



Comment at: 
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c:1
+__attribute__((section("__codesection")))
+int f(int a) {

clayborg wrote:
> 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?
What compilers do we use with test suite? This construction works fine with gcc 
and clang. Is it enough for us?



Comment at: source/Core/Section.cpp:30
 
-static const char *GetSectionTypeAsCString(lldb::SectionType sect_type) {
+const char *Section::GetSectionTypeAsCString(lldb::SectionType sect_type) {
   switch (sect_type) {

clayborg wrote:
> 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.
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?


https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added inline comments.
Herald added a reviewer: espindola.



Comment at: 
packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/TestBreakpointThumbCodesection.py:29
+
+self.runCmd("run")
+

labath wrote:
> 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.
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>: ldrr3, [r5], #4
0x103d4 <+36>: movr2, r9
0x103d6 <+38>: movr1, r8
0x103d8 <+40>: movr0, 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")))
   2int f(int a) {
-> 3  return a + 1; // Set break point at this line.
   4}
   5
   6int main() {
   7  return f(10);
(lldb) disas
thumb-bp`f:
0x103f0 <+0>:  subsp, #0x8
0x103f2 <+2>:  movr1, r0
0x103f4 <+4>:  strr0, [sp, #0x4]
->  0x103f6 <+6>:  ldrr0, [sp, #0x4]
0x103f8 <+8>:  adds   r0, #0x1
0x103fa <+10>: strr1, [sp]
0x103fc <+12>: addsp, #0x8
0x103fe <+14>: bx lr

```

In both cases resolved address is 0x103f6 that is right according to the debug 
information:
```
 Line Number Statements:
  [0x0025]  Extended opcode 2: set Address to 0x103f0
  [0x002c]  Special opcode 6: advance Address by 0 to 0x103f0 and Line by 1 
to 2
  [0x002d]  Set column to 10
  [0x002f]  Set prologue_end to true
  [0x0030]  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. 


https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added a comment.
Herald added a reviewer: javed.absar.

Is it ready to commit?


https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added a comment.

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


https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

[Konstantin Baladurin via Phabricator via lldb-commits]

kbaladurin added a comment.

Thank you very much!


Repository:
  rL LLVM

https://reviews.llvm.org/D44998



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits