clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

If the CIEs can change per CIE,l then this patch is fine.



================
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:38
   DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP &section,
                      lldb::RegisterKind reg_kind, bool is_eh_frame);
 
----------------
labath wrote:
> 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.
Ah, interesting. Might be nice to make a DWARF/EHFrame enum later to make the 
code more readable when DWARFCallFrameInfo are created. No need to do that now 
if you need to get this in. 


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