[review v3] [AArch64, SVE] Improve target description check for SVE in gdbserver

["Simon Marchi (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/690
......................................................................


Patch Set 3:

(1 comment)

| --- gdb/gdbserver/tdesc.c
| +++ gdb/gdbserver/tdesc.c
| @@ -186,3 +186,19 @@ tdesc_create_feature (struct target_desc *tdesc, const char *name)
|    tdesc->features.emplace_back (new_feature);
|    return new_feature;
|  }
| +
| +/* See gdbsupport/tdesc.h.  */
| +
| +bool
| +tdesc_contains_feature (const target_desc *tdesc, const std::string feature)
| +{
| +  if (tdesc && !tdesc->features.empty ())

PS2, Line 195:

> No use case in particular for a NULL tdesc, though i tend to think having the check here prevents having to add such a check elsewhere. We don't want gdbserver to die when we see a NULL tdesc for example.
> 
> Unless a NULL tdesc is a major error we don't want to allow. Then i agree an assert would be good. Thoughts?

I don't know if it's possible to execute with no target description in
gdbserver, or if we always have one.

regcache->tdesc is initialized in init_register_cache.  I checked the
various paths that could be called, and it appears to me like it's
always non-NULL.  For example:

- the new_register_cache call at server.c:4110, passes
current_target_desc.  current_target_desc returns a default target
description at worst
- get_thread_regcache passes a tdesc, which it asserts to be not null
- in tracepoint.c, we call get_ipa_tdesc, which is target-specific.  I
did a quick scan of the architectures, and it looks like they always
return something non-NULL.

I believe that it's not possible to get a NULL tdesc here, so I would
vote for making it an assert.

| +    {
| +      for (const tdesc_feature_up &f : tdesc->features)
| +	{
| +	  if (f->name.compare (feature) == 0)
| +	    return true;
| +	}
| +    }
| +
| +  return false;

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I28b782cb1677560ca9a06a1be442974b25aabae4
Gerrit-Change-Number: 690
Gerrit-PatchSet: 3
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 19 Nov 2019 15:02:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Luis Machado <luis.machado@linaro.org>
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment