labath added inline comments.
================
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:38
DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP §ion,
lldb::RegisterKind reg_kind, bool is_eh_frame);
----------------
clayborg wrote:
> 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.
Thanks for the quick review. I'm in a moderate hurry, as an android compiler
change caused us to fail to unwind on x86 without debug_frame, but I'll spin up
a separate patch for that.
https://reviews.llvm.org/D34613
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
