Re: [PATCH v3 1/3] arc: Add ARCv2 XML target along with refactoring

[Shahab Vahedi <shahab.vahedi@gmail.com>]

On Thu, Jul 16, 2020 at 09:28:50AM -0400, Simon Marchi wrote:
> On 2020-07-15 4:35 p.m., Shahab Vahedi wrote:
> When I search for `arc_gdbarch_features_init` in that patch I don't find anything...

You're correct. I have it in my local branch. The patch that uses it is going
to be submitted very soon. I hope you don't mind if it remains like this.
> The code in GDB constructing an arc_gdbarch_features will use the BFD to determine
> these two values, whereas the code in GDBserver will use something else (usually
> looking at the current process' properties).
> 
> In fact, the constructor is optional, you could just build a arc_gdbarch_features
> using aggregate initialization and return it from that function:
> 
>   arc_gdbarch_features features {reg_size, isa};
> 
> It doesn't really matter.  I just happen to prefer the constructor method, because
> that makes it so you can't "forget" a field and it ensures it can never be in an
> uninitialized state.

I see now. Actually, I have usecases to not initialize it immediately,
but in a few lines of code.
> > No, I haven't built it with AddressSanitizer. Could you point me in the right
> > direction how I can use it?
> 
> You already got the right information from Christian.

You were correct. There were some leaks. After adapting the code to use
"target_desc_up" on top of Andrew's fix [1], it is OK now.

[1]
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=0e26741636bee18321d6743ecee6ae9effd35d1f
> Well, I guess that depends on how the hash map implementation uses the hash value
> to map into buckets.  Let's say that there are 16 buckets, and the hash table
> implements decides which bucket a value goes in by looking at the last four bits.
> And let's imagine that you insert 32 values, all with the same ISA value but with
> 32 different reg_sizes values (assuming that would make sense).  Then even though
> the hash values are all different, they all end up in the same bucket.
> 
> Having a hash function that uniformly distributes the hash values makes that kind
> of pathological case much more unlikely.

I didn't know about the internals (buckets). Thanks for the explanations.
However, if I understand your example correctly, the implementation
practically would use the hash values partially (only 8 bits). IMHO, I don't
expect it to happen.
> 
> Anyway, I won't press more on that issue, since it's really not that important.

I'll leave it as is then.
> We're talking about just a few items, so it will never make a difference.  And as
> Luis said on IRC, it could also use a vector it a linear search.

Now specially that the leakage is not an issue anymore, I will keep it as
a map.
> If `std::hash (reg_size)` produces a uniformly-distributed hash value, and
> `std::hash (isa)` as well, then the output is surely uniformly-distributed,
> no?

I'd say yes: f(x)+f(y) should be uniform if f() is uniform.

Cheers,
Shahab