mstorsjo added a comment. In D70745#1760917 <https://reviews.llvm.org/D70745#1760917>, @amccarth wrote:
> Is `.eh_frame` the only one that matters? Should this be more general and > compare `const_sect_name` to the full name and the truncated name for any > known section names? Out of the sections name I see in executables, it's only `.eh_frame` where this is relevant. LLD produces another similarly truncated section name, `.gcc_except_table` truncated to `.gcc_exc`, but LLDB doesn't look for that one. As for doing it in general for any known section name; I think that could end up ambiguous for some of the `.debug_*` section types, like `.debug_line` and `.debug_loc` which both would end up as `.debug_l` in truncated form. Although, as those section names wouldn't be truncated in the executables, I don't think a generic check for a truncated form would hurt either. > Although not directly relevant to this patch, it seems like a lot this > cascade could be made more readable and maintainable by replacing the simple > if-name-set-section_type cases were replaced with a lookup into a table (and > that lookup could be the one place to check both the full name and the > truncated name). In D70745#1761259 <https://reviews.llvm.org/D70745#1761259>, @labath wrote: > I think you should be able to write a test with a yaml2obj + `lldb-test > object-file`. That's how the equivalent elf functionality is tested (see > `test/Shell/ObjectFile/ELF/section-types.yaml`). It won't check that the > section is actually parsed properly, but I don't think that's needed for the > kind of fix you're making here. Ok, thanks! Yes, that's exactly the kind of test I was thinking of. > 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? 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70745/new/ https://reviews.llvm.org/D70745 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
