Yes, that would definitely be a good idea. I was just saying that we shouldn't gate *any* .debug_frame support on being able to read a dwarf4 .debug_frame section.
BTW, we've had a short chat about this with Abidh on IRC, I am going to paste the relevant part here, to make sure everyone is on the same page: <labath> after looking at the code, i think the eh_frame/debug_frame choice should be per function, not per-module <labath> that's way more useful, and it should allow more reasonable testing <abidh> I am a little confused. You are saying that compiler should provide this per function choice <labath> no <labath> right <labath> so what the current patch does is: <labath> if (we_have_eh_frame()) frame = eh_frame() else frame = debug_frame() <labath> (this is in the initialization code) <labath> and then later, when we actually try to get an unwind plan for some function, we do: <labath> if (frame->can_you_unwind(function)) return frame->get_unwind_plan(function) <labath> what I am proposing instead is for the init code to do: <labath> frame1 = eh_frame(); frame2 = debug_frame(); <labath> and then later on (this would be the UnwindTable::GetFuncUnwindersContainingAddress function) <labath> if (frame1->can_you_unwind(function)) return frame1->get_unwind_plan(function) <labath> else if (frame2->can_you_unwind(function)) return frame2->get_unwind_plan(function) <labath> so the choice which unwind section you use happens on a per-function basis On 25 May 2017 at 22:42, Robinson, Paul <paul.robin...@sony.com> wrote: > IANA lldb developer, but I should think lldb would want to understand a > DWARF 4 .debug_frame section. (And it didn't change in DWARF 5, either!) > --paulr > >> -----Original Message----- >> From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf >> Of Pavel Labath via lldb-commits >> Sent: Thursday, May 25, 2017 9:17 AM >> To: Tatyana Krasnukha >> Cc: lldb-commits@lists.llvm.org >> Subject: Re: [Lldb-commits] [PATCH] D33504: Fix FDE indexing while scan >> debug_info section >> >> +lldb-commits >> >> Yes, that is certainly a viable approach. If there is a subset of >> dwarf that we are capable of parsing correctly then it would be great >> to enable that. >> >> On 25 May 2017 at 16:46, Tatyana Krasnukha >> <tatyana.krasnu...@synopsys.com> wrote: >> > This binary uses DWARF 4, that introduces two additional fields in CIE - >> address_size and segment_size just after augmentation field. That’s why >> values are parsed incorrectly. >> > May be we should discard unwind plan if there is unsupported dwarf >> version for now? >> > >> > Thanks, >> > Tatyana >> > >> > -----Original Message----- >> > From: Pavel Labath [mailto:lab...@google.com] >> > Sent: Thursday, 25 May, 2017 5:45 PM >> > To: tatyana.krasnu...@synopsys.com >> > Subject: Re: [PATCH] D33504: Fix FDE indexing while scan debug_info >> section >> > >> > Hey, try the attached binary for size. >> > >> > It was generated from the source file in >> lldb/test/testcases/functionalities/breakpoint/breakpoint_conditions/main. >> c, >> > while targetting android arm64. >> > >> > You should be able to create a test binary for yourself locally >> (assuming you're on linux) like this: >> > $ cat a.c >> > int a(char *x) { return *x; } >> > >> > int f() { >> > return a(0); >> > } >> > >> > int g() { >> > return 1+f(); >> > } >> > >> > int _start() { return 2+g(); } >> > >> > $ clang -fno-exceptions -fno-unwind-tables -g a.c -nostdlib >> > >> > If you debug the binary with and stop in the ParseCIE function, you will >> see that it parses the entry incorrectly: >> > Process 39688 stopped >> > * thread #1, name = 'lldb', stop reason = step over >> > frame #0: 0x00007ffff074bb53 >> > >> liblldb.so.5`lldb_private::DWARFCallFrameInfo::ParseCIE(this=0x00000000006 >> 13280, >> > cie_offset=0) at DWARFCallFrameInfo.cpp:303 >> > 300 cie_sp->data_align = (int32_t)m_cfi_data.GetSLEB128(&offset); >> > 301 cie_sp->return_addr_reg_num = m_cfi_data.GetU8(&offset); >> > 302 >> > -> 303 if (cie_sp->augmentation[0]) { >> > 304 // Get the length of the eh_frame augmentation data >> > 305 // which starts with a ULEB128 length in bytes >> > 306 const size_t aug_data_len = >> (size_t)m_cfi_data.GetULEB128(&offset); >> > >> > (lldb) fr var cie_sp._M_ptr[0] >> > (lldb_private::DWARFCallFrameInfo::CIE) cie_sp._M_ptr[0] = { >> > cie_offset = 0 >> > version = '\x04' >> > augmentation = "" >> > code_align = 8 <====== wrong >> > data_align = 0 <====== wrong >> > return_addr_reg_num = 1 <===== wrong >> > inst_offset = 0 >> > inst_length = 0 >> > ptr_encoding = '\0' >> > lsda_addr_encoding = '\xff' >> > personality_loc = 18446744073709551615 >> > initial_row = { >> > m_offset = 0 >> > m_cfa_value = { >> > m_type = unspecified >> > m_value = { >> > reg = (reg_num = 0, offset = 0) >> > expr = (opcodes = 0x0000000000000000, length = 0) >> > } >> > } >> > m_register_locations = size=0 {} >> > } >> > } >> > >> > Correct parsing can be seen by running >> > $ llvm-dwarfdump a.out >> > ..... >> > .debug_frame contents: >> > >> > 00000000 00000014 ffffffff CIE >> > Version: 4 >> > Augmentation: "" >> > Address size: 8 <=== this shows up in code_align >> > Segment desc size: 0 <=== this shows up in data_align >> > Code alignment factor: 1 <=== this shows up in return_addr_reg_num >> > Data alignment factor: -8 >> > Return address column: 16 >> > >> > DW_CFA_def_cfa: reg7 +8 >> > DW_CFA_offset: reg16 -8 >> > DW_CFA_nop: >> > DW_CFA_nop: >> > DW_CFA_nop: >> > DW_CFA_nop: >> > >> > 00000018 0000001c 00000000 FDE cie=00000000 pc=004000b0...004000c1 >> > DW_CFA_advance_loc: 1 >> > DW_CFA_def_cfa_offset: +16 >> > DW_CFA_offset: reg6 -16 >> > DW_CFA_advance_loc: 3 >> > DW_CFA_def_cfa_register: reg6 >> > .... >> > >> > >> > Interestingly, in the x86_64 linux case, lldb still manages to unwind >> correctly despite the bad parsing. >> > >> > >> > >> > >> > On 25 May 2017 at 14:40, Tatyana Krasnukha via Phabricator >> <revi...@reviews.llvm.org> wrote: >> >> tatyana-krasnukha added a comment. >> >> >> >> Yes, give those binaries, please. >> >> >> >> >> >> Repository: >> >> rL LLVM >> >> >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_ >> >> D33504&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=8NZfjV_ZLY_S7gZyQMq8mj7tiN4 >> >> vlymPiSt0Wl0jegw&m=Ze6ANiK7goV6lSPJcY8QglIJ_tZC0gXeUuqpQ8n4K40&s=1vfSh >> >> WELC8pd8-DQmAeW3QJDQgdMAJvn905_U-jVphM&e= >> >> >> >> >> >> >> _______________________________________________ >> lldb-commits mailing list >> lldb-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits