Re: 回复: [8u] RFR(XS) JFR generate same symbol ID for different symbols (high probabily with CDS)(Internet mail)

[Ioi Lam]

Looping in Markus Grönlund


On 10/9/20 5:57 AM, Andrew Dinn wrote:
> Hi Hui,
>
> On 09/10/2020 12:56, kalinshi(施慧) wrote:
>> “I can see that it is possible that two different symbols might
>> generate the same hash, which is what appears to motivate your patch.
>> However, you have not really explained in what circumstances that
>> problem will happen. That makes it impossible to know whether the
>> patch is really needed.“
>>
>> This problem happens when running with CDS and reproduce steps is in
>> https://bugs.openjdk.java.net/browse/JDK-8253837.
> Apologies for not seeing that explanation. When I opened the JIRA the
> description tab was not displayed. This now makes a lot more sense.
>
>> " Also, if it is needed then it is important to know whether this is
>> the only thing that needs fixing or if more needs to be done. At the
>> very least I think you will need to patch both JfrSymbolId::equals
>> methods."
>>
>> I have noticed " bool JfrSymbolId::equals(const char* query,
>> uintptr_t hash, const CStringEntry* entry) ". Plan is to investigate
>> this after finding a reproducible case, this code is quite different
>> from JDK11. In 8u, some caller is from
>> JfrSymbolId::map_cstring(uintptr_t hash) with NULL query and hash
>> code, while in 11u map_cstring method has removed and a declaration
>> is left.
> Well, I see what you mean regarding calls via map_cstring. There is no
> possibility of query and entry->literal() being different strings with
> the same hashcode when equals is called via map_cstring because it only
> calls lookup_only() with query == null.
>
> However, the cstring table has to be updated somewhere and this happens
> when method id() is called in JfrSymbolId::mark. id() calls lookup_put()
> with a non null string. Now that looks to me like it can suffer the same
> problem as the symbol insert. If two symbols can generate the same hash
> then I don't see why two C strings could not also generate the same
> hash. That may be unlikely but I don't think it is impossible. So, I
> think it is going to be necessary to change both equals methods. Do you
> have any reasoning that explains why a hash collision is not possible?
>
> I'd also still like to see a response from one of the upstream devs who
> worked on JDK-8231081 to explain why it needed to make allowance for
> hash collisions and also to ask if they see any good reason to backport
> JDK-8231081. If we need to do that anyway then we might as well apply
> all the required changes in one go. Jaroslav, any comments?
>
> regards,
>
>
> Andrew Dinn
> -----------
> Red Hat Distinguished Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill
>