Re: [lldb-dev] hashing pointers to strings
If it's dead code, that certainly simplifies things! I've sent a patch for review to remove the unused, untested code. https://reviews.llvm.org/D43202 On Mon, Feb 12, 2018 at 1:52 AM, Pavel Labath wrote: > [ Clang's emission of pubnames/pubtypes is controlled by the "debugger > tuning" parameter. With -glldb they don't get emitted, with -ggdb, > they do. (-glldb is default on mac, -ggdb elsewhere). In dwarf5 these > sections have been dropped in favour of the new .debug_names section. > ] > > On 10 February 2018 at 02:25, Jim Ingham via lldb-dev > wrote: > > A quick glance indicates that this is all dead code. I can't see > anything that actually instantiates either of the pubnames classes. > > > > The DWARF pubnames tables were a previous attempt at producing DWARF > indexes, but they didn't contain enough information to actually forstall > deep dives into the DWARF, so they weren't actually useful. clang doesn't > emit them on macOS for sure, does it emit them on linux? They are > superseded by the new DWARF 5 accelerator tables, and I couldn't find any > reference to pubnames in the DWARF 5 draft standard. > > > > We should check with Greg about this, but I don't think we're ever > likely to get any use out of DWARF pubnames tables. We should just delete > this code. > > > > Jim > > > > > >> On Feb 9, 2018, at 4:35 PM, Adrian McCarthy via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > >> > >> DWARFDebugPubnamesSet.h has a type definition like this: > >> > >> typedef std::unordered_multimap >> std::hash, > >> CStringEqualBinaryPredicate> > >> cstr_to_index_mmap; > >> > >> In particular, note that the hasher will hash the pointer rather than > the string it points to. > >> > >> It looks like this mostly works because the map is built once from > string pointers that don't appear to be changed during the lifetime of the > multimap. That's fragile, and I'm not sure it's really working in all > cases. For example, there could be two different pointers to identical > strings--since this is a multimap rather than a map, I assume we'd want > those values merged under the same key, but since the pointers are > distinct, they won't be. > >> > >> I'd like to change the key to a std::string or a StringRef for > correctness and robustness, but that'll likely be a tad slower because > hashing variable-length strings is more work than hashing fixed-length > pointers. (I don't think it'll change comparisons much, since the > comparator is checking the strings.) > >> > >> Objections or better suggestions? > >> > >> It's also hard for me to test, as most of the LLDB DWARF tests are > still broken on Windows because the inferiors are generated with CodeView > rather than DWARF. I'm working on that, but it'll take changes to the > lld-link driver. > >> > >> Adrian. > >> ___ > >> lldb-dev mailing list > >> lldb-dev@lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > > > ___ > > lldb-dev mailing list > > lldb-dev@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hashing pointers to strings
> "Pavel" == Pavel Labath via lldb-dev writes: Pavel> [ Clang's emission of pubnames/pubtypes is controlled by the "debugger Pavel> tuning" parameter. With -glldb they don't get emitted, with -ggdb, Pavel> they do. gdb has never read .debug_pubnames, so this setting doesn't really make sense. Tom ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hashing pointers to strings
[ Clang's emission of pubnames/pubtypes is controlled by the "debugger tuning" parameter. With -glldb they don't get emitted, with -ggdb, they do. (-glldb is default on mac, -ggdb elsewhere). In dwarf5 these sections have been dropped in favour of the new .debug_names section. ] On 10 February 2018 at 02:25, Jim Ingham via lldb-dev wrote: > A quick glance indicates that this is all dead code. I can't see anything > that actually instantiates either of the pubnames classes. > > The DWARF pubnames tables were a previous attempt at producing DWARF indexes, > but they didn't contain enough information to actually forstall deep dives > into the DWARF, so they weren't actually useful. clang doesn't emit them on > macOS for sure, does it emit them on linux? They are superseded by the new > DWARF 5 accelerator tables, and I couldn't find any reference to pubnames in > the DWARF 5 draft standard. > > We should check with Greg about this, but I don't think we're ever likely to > get any use out of DWARF pubnames tables. We should just delete this code. > > Jim > > >> On Feb 9, 2018, at 4:35 PM, Adrian McCarthy via lldb-dev >> wrote: >> >> DWARFDebugPubnamesSet.h has a type definition like this: >> >> typedef std::unordered_multimap> std::hash, >> CStringEqualBinaryPredicate> >> cstr_to_index_mmap; >> >> In particular, note that the hasher will hash the pointer rather than the >> string it points to. >> >> It looks like this mostly works because the map is built once from string >> pointers that don't appear to be changed during the lifetime of the >> multimap. That's fragile, and I'm not sure it's really working in all >> cases. For example, there could be two different pointers to identical >> strings--since this is a multimap rather than a map, I assume we'd want >> those values merged under the same key, but since the pointers are distinct, >> they won't be. >> >> I'd like to change the key to a std::string or a StringRef for correctness >> and robustness, but that'll likely be a tad slower because hashing >> variable-length strings is more work than hashing fixed-length pointers. (I >> don't think it'll change comparisons much, since the comparator is checking >> the strings.) >> >> Objections or better suggestions? >> >> It's also hard for me to test, as most of the LLDB DWARF tests are still >> broken on Windows because the inferiors are generated with CodeView rather >> than DWARF. I'm working on that, but it'll take changes to the lld-link >> driver. >> >> Adrian. >> ___ >> lldb-dev mailing list >> lldb-dev@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] hashing pointers to strings
A quick glance indicates that this is all dead code. I can't see anything that actually instantiates either of the pubnames classes. The DWARF pubnames tables were a previous attempt at producing DWARF indexes, but they didn't contain enough information to actually forstall deep dives into the DWARF, so they weren't actually useful. clang doesn't emit them on macOS for sure, does it emit them on linux? They are superseded by the new DWARF 5 accelerator tables, and I couldn't find any reference to pubnames in the DWARF 5 draft standard. We should check with Greg about this, but I don't think we're ever likely to get any use out of DWARF pubnames tables. We should just delete this code. Jim > On Feb 9, 2018, at 4:35 PM, Adrian McCarthy via lldb-dev > wrote: > > DWARFDebugPubnamesSet.h has a type definition like this: > > typedef std::unordered_multimap std::hash, > CStringEqualBinaryPredicate> > cstr_to_index_mmap; > > In particular, note that the hasher will hash the pointer rather than the > string it points to. > > It looks like this mostly works because the map is built once from string > pointers that don't appear to be changed during the lifetime of the multimap. > That's fragile, and I'm not sure it's really working in all cases. For > example, there could be two different pointers to identical strings--since > this is a multimap rather than a map, I assume we'd want those values merged > under the same key, but since the pointers are distinct, they won't be. > > I'd like to change the key to a std::string or a StringRef for correctness > and robustness, but that'll likely be a tad slower because hashing > variable-length strings is more work than hashing fixed-length pointers. (I > don't think it'll change comparisons much, since the comparator is checking > the strings.) > > Objections or better suggestions? > > It's also hard for me to test, as most of the LLDB DWARF tests are still > broken on Windows because the inferiors are generated with CodeView rather > than DWARF. I'm working on that, but it'll take changes to the lld-link > driver. > > Adrian. > ___ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
[lldb-dev] hashing pointers to strings
DWARFDebugPubnamesSet.h has a type definition like this: typedef std::unordered_multimap, CStringEqualBinaryPredicate> cstr_to_index_mmap; In particular, note that the hasher will hash the *pointer *rather than the string it points to. It looks like this mostly works because the map is built once from string pointers that don't appear to be changed during the lifetime of the multimap. That's fragile, and I'm not sure it's really working in all cases. For example, there could be two different pointers to identical strings--since this is a multimap rather than a map, I assume we'd want those values merged under the same key, but since the pointers are distinct, they won't be. I'd like to change the key to a std::string or a StringRef for correctness and robustness, but that'll likely be a tad slower because hashing variable-length strings is more work than hashing fixed-length pointers. (I don't think it'll change comparisons much, since the comparator *is *checking the strings.) Objections or better suggestions? It's also hard for me to test, as most of the LLDB DWARF tests are still broken on Windows because the inferiors are generated with CodeView rather than DWARF. I'm working on that, but it'll take changes to the lld-link driver. Adrian. ___ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
