Re: [PATCH] arc: Pass proper CPU value to disassembler

[Pedro Alves <palves at redhat dot com>]

On 10/10/2017 08:22 PM, Anton Kolesov wrote:

> +/* ARC EM and ARC HS are unique BFD arches, however they share the same machine
> +   number as "ARCv2".  */
> +
> +static inline bool
> +arc_arch_is_hs (const struct bfd_arch_info* arch)
> +{
> +  return CONST_STRNEQ (arch->printable_name, "HS");
> +}
> +
> +static inline bool
> +arc_arch_is_em (const struct bfd_arch_info* arch)
> +{
> +  return CONST_STRNEQ (arch->printable_name, "EM");
> +}

I'd prefer if you used startswith instead.  There's not much
point in using CONST_STRNEQ nowadays, and we don't tend to
use it in GDB -- compilers have no trouble constant folding
the length of string literals.

> +gdb_start
> +
> +# Test whether it ok to have `arc:HS` in target description architecture.

"it's OK"  ... "in the target"

> +# `HS` is a valid BFD architecture name, however disassembler doesn't accept

"the disassembler"

> +# it as a CPU name.  This test checks that GDB doesn't pass architecture from
> +# target description directly to disassembler and instead uses one of the

"the target description" ... "the disassembler"

> +# valid CPU names.
> +
> +set filename $srcdir/$subdir/arc-tdesc-cpu.xml
> +
> +set cmd "set tdesc filename $filename"
> +gdb_test $cmd
> +
> +# Error message is emitted by disassembler, therefore it is not shown unless

"An error" ... "the disassembler" (x2).

> +# disassembler is actually invoked.  Address "0" is not invalid, but that
> +# doesn't matter for this test case, because it is only disassembler error
> +# message that is interesting.

"the disassembler"

> +set cmd "x /i 0"
> +set msg "setting HS architecture"
> +gdb_test_multiple $cmd $msg {
> +    -re "Unrecognised disassembler CPU option: HS" {

Must match $gdb_prompt too, otherwise the prompt is left
in the expect buffer and confuses
following gdb_test/gdb_test_multiple calls.

> +	fail $msg
> +    }
> +    -re "\r\n$gdb_prompt" {

This seems a bit fragile.  If the error output ever changes,
then this will match, and thus will always pass.  Can this
regex be tightened a bit to include something more than
just the prompt?

> +	pass $msg
> +    }
> +}

Thanks,
Pedro Alves