labath added a comment.

In https://reviews.llvm.org/D34613#790745, @clayborg wrote:

> Glad this is happening. Does this mean we won't see the "bad eh frame" 
> warnings anymore on linux? See inlined comments.


Unfortunately, I doubt it. This does nothing about eh_frame parsing, it just 
adds debug_frame support. So, you may start seeing more of those, as we will be 
parsing more stuff. :)

That said, I haven't seen that warning happen, at least not on x86. If you have 
a simple repro case, I could take a look.



================
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:38
   DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP &section,
                      lldb::RegisterKind reg_kind, bool is_eh_frame);
 
----------------
clayborg wrote:
> Remove "reg_kind" and "is_eh_frame" and replace with 
> DWARFCallFrameInfo::Type. See inlined comment below by the CFIVersion enum.
I think the enum could be used more prominently internally (I haven't checked 
that more closely yet). However, I think that using it here is wrong. The 
reason is that the decision which debug_frame version we are parsing does not 
happen at this level. This is a per-CIE property -- the same debug_frame 
section can contain CIE's with different version numbers (e.g. if they were 
compiled with different compilers or flags -- in fact, that's how I built my 
test module). At this level, all we need to know is whether we are parsing an 
eh_frame or debug_frame section, which is a still boolean value.

Theoretically, we could have a separate two-valued (eh, dwarf) enum here, and 
then, internally, when speaking about a specific CIE, use the 4-valued enum you 
proposed.

The register kind argument could probably be removed though.


================
Comment at: source/Symbol/FuncUnwinders.cpp:219-221
+  // Only supported on x86 architectures where we get debug_frame from the
+  // compiler that describes the prologue instructions perfectly, and sometimes
+  // the epilogue instructions too.
----------------
clayborg wrote:
> What compiler suddenly started including complete unwind info in 
> .debug_frame? They are all supposed, but none ever have. MacOSX x86 and 
> x86_64 has never done this right. I think this if statement is too inclusive. 
> Can you narrow it down? I would love to see an example of this in action. I 
> have never seen the x86 PIC bump code be properly described in any compiler. 
> Are we trying to use this for unwinding frame zero? I would hope not unless 
> we really and carefully know the exact compiler that produced the info. It 
> would be preferable marked with an augmentation code that says "yes, I am 
> really complete everywhere".
This is just copied from the equivalent eh_frame function, line 147 (which 
seems to have been added in r224689). I think what's that trying to say is that 
eh_frame/debug_frame augmentation is only supported on x86.


https://reviews.llvm.org/D34613



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

Reply via email to