[Lldb-commits] [PATCH] D70745: [LLDB] [PECOFF] Look for the truncated ".eh_fram" section name

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D70745#1761544 <https://reviews.llvm.org/D70745#1761544>, @mstorsjo wrote:

> In D70745#1761421 <https://reviews.llvm.org/D70745#1761421>, @labath wrote:
>
> > In D70745#1761392 <https://reviews.llvm.org/D70745#1761392>, @mstorsjo wrote:
> >
> > > > As for the long if-else cascade, the equivalent elf code was recently refactored to use llvm::StringSwitch. Doing the same here would be nice.
> > >
> > > A StringSwitch sounds nice. Some of the cases come with extra conditions (header flags and checking section sizes) though - is it best to keep that as is, if the StringSwitch didn't match anything?
> >
> >
> > I don't really know what are the conventions in the COFF world (you probably know that better), but I would tend to trust the section flags *more* than any name-based deductions. So I'd try to factor this such (and this is what I've done in the elf case) to do the section flag checks first, and only then fall back to the section names. The checks for the size 0 in data vs. bss sections looks weird to me, and I don't know if its really needed, but if you think it is, then it's probably best to pull that those cases out separately.
>
>
> Some of those special cases were added rather recently (1 year ago), so I'd rather not touch them. Checking the flags alone doesn't say much for e.g. "data" sections (that can be anything between actual data and debug info), so I didn't find much I dared to touch here, but it can at least be made some amount more readable, in D70778 <https://reviews.llvm.org/D70778>.
>
> >> As for @amccarth's suggestion on generically checking for truncated forms, I guess that's not doable with a StringSwitch, but would require e.g. creating more of an ad-hoc table, that we could iterate over, checking for both full and truncated matches? Does that sound sensible to you?
> > 
> > If you think it's good to check the truncated name on all sections, then yes, some kind of a table would probably be nice. If it's just one or two cases, then you can just spell out both forms in the StringSwitch (`.Cases(".eh_frame", ".eh_fram", eSectionTypeEHFrame)`
>
> Ok, going with just a single Cases here. A table and generic logic for checking truncated forms might be nice, but I don't see a direct need right now.


SGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70745/new/

https://reviews.llvm.org/D70745