This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 15/25] [RFC] GDBserver unit test to i386_tdesc
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Wed, 28 Jun 2017 18:22:39 +0100
- Subject: Re: [PATCH 15/25] [RFC] GDBserver unit test to i386_tdesc
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 820F6C0587D1
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 820F6C0587D1
- References: <1497256916-4958-1-git-send-email-yao.qi@linaro.org> <1497256916-4958-16-git-send-email-yao.qi@linaro.org>
On 06/12/2017 09:41 AM, Yao Qi wrote:
> This patch adds a unit test in GDBserver to test dynamically created
> target descriptions equal to these pre-generated ones.
>
> gdb/gdbserver:
Are these temporary tests? What's the plan for the pre-generated ones?
> +#if defined GDB_SELF_TEST && !defined IN_PROCESS_AGENT
> +#include "selftest.h"
> +
> +namespace selftests {
> +namespace gdbserver {
Considering that at some point we may want to wrap
all of gdbserver under "namespace gdbserver", this
namespace choice here seems like a poor choice.
s/gdbserver/tdesc_tests/ or some such?
> +static void
> +i386_tdesc_test ()
> +{
> + const struct target_desc *tdesc = i386_get_ipa_tdesc (X86_TDESC_MMX);
> +
> + SELF_CHECK (*tdesc == *tdesc_i386_mmx_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_SSE);
> + SELF_CHECK (*tdesc == *tdesc_i386_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_AVX);
> + SELF_CHECK (*tdesc == *tdesc_i386_avx_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_MPX);
> + SELF_CHECK (*tdesc == *tdesc_i386_mpx_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_AVX_MPX);
> + SELF_CHECK (*tdesc == *tdesc_i386_avx_mpx_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_AVX_AVX512);
> + SELF_CHECK (*tdesc == *tdesc_i386_avx_avx512_linux);
> + delete tdesc;
> +
> + tdesc = i386_get_ipa_tdesc (X86_TDESC_AVX_MPX_AVX512_PKU);
> + SELF_CHECK (*tdesc == *tdesc_i386_avx_mpx_avx512_pku_linux);
> + delete tdesc;
Would something like this, based on an array, work? :
struct
{
int idx;
const target_desc *tdesc;
}
tdesc_tests[] = {
{ X86_TDESC_MMX, tdesc_i386_mmx_linux },
{ X86_TDESC_SSE, tdesc_i386_linux },
...
};
for (auto &elem : tdesc_tests)
{
std::unique_ptr<const target_desc> tdesc (i386_get_ipa_tdesc (elem.idx));
SELF_CHECK (*elem.tdesc == *tdesc);
}
> diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
> index 2d4cbfa..e64f0b3 100644
> --- a/gdb/gdbserver/tdesc.h
> +++ b/gdb/gdbserver/tdesc.h
> @@ -19,7 +19,8 @@
> #ifndef TDESC_H
> #define TDESC_H
>
> -struct reg;
> +#include "regdef.h"
> +#include <cstring>
Odd that you needed cstring? string.h is one of the
system headers always pulled in by common/common-defs.h.
>
> typedef struct reg *tdesc_reg_p;
> DEF_VEC_P(tdesc_reg_p);
> @@ -60,6 +61,45 @@ public:
> xfree (reg);
> VEC_free (tdesc_reg_p, reg_defs);
> }
> + bool operator!= (const target_desc &other) const
Same comment about "operator!=" vs "operator==" as in
a previous patch.
Thanks,
Pedro Alves