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

[Davide Italiano via lldb-commits]

On Thu, Mar 8, 2018 at 10:46 AM, 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.
>
> Jim
>

Yes, that's the reason. The tests seem to pick the system `dsymutil`
instead of the one that's built `in-tree`. I'll have a chat with Jonas
about this.