[Lldb-commits] [lldb] r325927 - Replace HashStringUsingDJB with llvm::djbHash

[Pavel Labath via lldb-commits]

On Thu, 8 Mar 2018 at 18:46, Jim Ingham <jingham at apple.com> wrote:

>
>
> > On Mar 8, 2018, at 2:49 AM, Pavel Labath <labath at google.com> wrote:
> >
> >
> >
> >
> > On Thu, 8 Mar 2018 at 02:46, Davide Italiano <dccitaliano at gmail.com>
> wrote:
> > On Wed, Mar 7, 2018 at 6:39 PM, Jim Ingham <jingham at apple.com> wrote:
> > > The hashing algorithm gives different values - at least for foobár -
> between the two implementations.  So if you build with an older clang, and
> test with a new lldb, the type lookup fails.
> > >
> > When I landed this patch, the two algorithms were identical, but this
> wasn't always the case. The llvm's version of the algorithm used to be
> nondeterministic (as in, it's result depended on the signedness of char on
> your platform). This was recently fixed, and this patch was meant to make
> sure they always stay in sync.
> >
> > We considered a couple of options when we noticed this discrepancy, but
> since this had never really worked (llvm and lldb had always disagreed on
> the hash value of strings with high bit set) and nobody noticed, we decided
> to just fix the llvm implementation. The other option was to create a
> signedDJBHash function, and make lldb&llvm always use that for apple
> accelerator tables. I guess it's still not too late to do that...
>
>
> I have no idea how common the use of high bit characters in C identifiers
> is.  I can't remember ever seeing a usage in browsing through the Darwin
> sources.  We do have to support users who want to debug released products
> using older dSYM's, but if high bit identifiers are an uncommon practice,
> then just making sure lldb & clang & llvm-dsymutil stay in sync in the
> future is probably good enough.  IIRC the old dsymutil could run fixups on
> dsym's, and so if somebody ends up being really stuck because of this, we
> could teach llvm-dsymutil to regenerate the apple tables in the dSYM with
> the new hashes.  dsymutil doesn't use the apple tables from .o files, it
> rebuilds them, so doing this should be pretty straightforward.  The harder
> part is likely conveying to users when it is necessary to do so...
>
> >
> >
> > This is not my case, I think? I'm building from the monorepo so clang
> > and lldb are supposed to be in sync (in fact, the version of clang in
> > my test is 7.0).
> >
> >
> > My best (and only) guess would be that this is because of different
> versions of dsymutil (we are using the system dsymutil to build the dsym
> bundle), though I'm not sure then why the test doesn't fail for me as well,
> as I have the stock dsymutil that ships which XCode, which presumably still
> has the broken hash function.
> >
> > Could you send me the main.o and a.out.dSYM files that are built by this
> test (or at least the output of running llvm-dwarfdump -apple-types on
> them), so I can compare them with mine? The interesting question is what
> does "foobár" hash to (for me it's 0xba721fe1 in both files).
> >
>
> IIRC Davide said only the dSYM variant of the test was failing for him.
> If that's true then using the system dsymutil explains the difference.  But
> I don't know why the dSYM variant isn't failing for you.
>
>
> That's a good question. I've tried a small experiment:

I've reverted djbHash to use signed char and rebuilt clang only (so lldb
still uses the unsigned version): the dwarf and gmodules variants failed,
but the dsym was fine.
Then I've rebuilt lldb as well (so both use signed hashes): dwarf and
gmodules variants succeeded, but dsym failed.

So it looks like my dsymutil is somehow magically producing the correct
(unsigned) hashes (i've verified that the test suite runs the dsymutil in
the xcode toolchain dir), but I have no idea how is that possible...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180309/0a6eafeb/attachment.html>