From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by sourceware.org (Postfix) with ESMTPS id 015CD3857C6F for ; Wed, 31 Mar 2021 20:23:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 015CD3857C6F Received: by mail-qv1-xf36.google.com with SMTP id q12so10603723qvc.8 for ; Wed, 31 Mar 2021 13:23:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=hrdvuMdKQXzuVAy3w+T0Zhpr4BTm9KswzUtVU2ts+PI=; b=eBUVzX2j9LPLWtJ7xjYyBvTxD5op6dFSN9xteVw87BBMQdRyqs/bbrwSaXjE7SJbO4 ybp9r9eYuqm7l6AVRBnLfmSmPoIgFWH3haDx8eLP++vCqx1Npk20PMvmT52+U1UndSHm JKOzqnwOMN4sbkHnOhgpddwEZE1uRQtVhy6ytdb4bkX7bdCQx7xtxz8qc1b4phtQ3Fxk fKFNOfcZ7+ETPFDm+JH+qzZakAYbe85DJYiaQ2yWblfOgcQ24gYYvpZw4zouNXOLL9vl 8r3t4NIQTPR419LDvww9tqNy6oCqGf3qwjw+MREfHTE0mLnme68ygs4TukvG7DhsjKvT /a0Q== X-Gm-Message-State: AOAM531pO9WnKm+LZRvtpPdhD6ioyV5T2u4t0lO5KVV9e/4SMWEhidzW CfX7R9jwMHYjuqU7Br6uAqrXIhE36XPSAA== X-Google-Smtp-Source: ABdhPJyR0CnbG9drZoaYEmY/UPtSNxPVNLH6cb2CI8P/FBtz81zVcxdWSX+CcX+lJZaytRjiTKi/ww== X-Received: by 2002:ad4:5429:: with SMTP id g9mr5009823qvt.39.1617222231918; Wed, 31 Mar 2021 13:23:51 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:2841:397e:3682:580a:695? ([2804:7f0:4841:2841:397e:3682:580a:695]) by smtp.gmail.com with ESMTPSA id l14sm2011509qtp.4.2021.03.31.13.23.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 31 Mar 2021 13:23:51 -0700 (PDT) Subject: Re: [PATCH v3 3/7] start/stop btrace with coresight etm and parse etm buffer. nat independant To: Zied Guermazi , gdb-patches@sourceware.org References: <20210331025234.518688-1-zied.guermazi@trande.de> <20210331025234.518688-4-zied.guermazi@trande.de> From: Luis Machado Message-ID: Date: Wed, 31 Mar 2021 17:23:48 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210331025234.518688-4-zied.guermazi@trande.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Mar 2021 20:23:57 -0000 On 3/30/21 11:52 PM, Zied Guermazi wrote: > > This patch extend branch tracing by adding the functions needed > to collect parameters for decoding ETM traces and decoding them. > > gdb/ChangeLog > > * arch/arm.h: Add defines for exception numbers as encoded > in the ETM traces > * btrace.c (ftrace_new_function): log new function > (ftrace_remove_last_insn): New. > (cs_etm_get_etmv3_config): New. > (cs_etm_get_etmv4_config): New. > (cs_etm_get_isa_flag): New. > (cs_etm_get_instruction_class): New. > (cs_etm_update_btrace_with_inst_range): New. > (cs_etm_update_btrace_with_exception): New. > (cs_etm_update_btrace_with_trace_on): New. > (cs_etm_trace_element_callback): New. > (cs_etm_free_decoder): New. > (cs_etm_create_decoder): New. > (btrace_etm_readmem_callback): New. > (cs_etm_add_mem_access_callback): New. > (cs_etm_process_data_block): New. > (btrace_print_all): New. > (btrace_compute_ftrace_etm): New. > (btrace_compute_ftrace_1): add handling of CoreSight traces. > (btrace_enable): add error message if ETM unavailable. > (btrace_stitch_trace): add handling of CoreSight traces. > (maint_info_btrace_cmd): add handling of CoreSight trace format. > * btrace.h (btrace_insn_flag): add ARM ISA flags. > * record-btrace.c (cs_etm_reconstruct_cpsr): New. > (record_btrace_target::fetch_registers): fetch cpsr register > from insn->flags when applicable > > gdbsupport/ChangeLog > > * btrace-common.h (cs_etmv3_trace_params): New > (cs_etmv4_trace_params): New. > (cs_etm_trace_params): New. > (cs_etm_decoder_params): New > (btrace_data_etm_config): New. > (btrace_data_etm): New. > (btrace_data): add a btrace_data_etm. > * btrace-common.cc (btrace_data::fini): handle CoreSight traces. > (btrace_data::empty): handle CoreSight traces. > (btrace_data_append): handle CoreSight traces. > > > --- > gdb/ChangeLog | 27 +- > gdb/arch/arm.h | 33 ++ > gdb/btrace.c | 606 ++++++++++++++++++++++++++++++++++++ > gdb/btrace.h | 16 +- > gdb/record-btrace.c | 56 +++- > gdbsupport/ChangeLog | 13 + > gdbsupport/btrace-common.cc | 40 +++ > gdbsupport/btrace-common.h | 93 ++++++ > 8 files changed, 876 insertions(+), 8 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index a63fdfa1911..4453a3e8185 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,28 @@ > +2021-02-25 Zied Guermazi > + > + * btrace.c (ftrace_remove_last_insn): New. > + (cs_etm_get_etmv3_config): New. > + (cs_etm_get_etmv4_config): New. > + (cs_etm_update_btrace_with_inst_range): New. > + (cs_etm_update_btrace_with_exception): New. > + (cs_etm_update_btrace_with_trace_on): New. > + (cs_etm_trace_element_callback): New. > + (cs_etm_create_decoder): New. > + (cs_etm_free_decoder): New. > + (btrace_etm_readmem_callback): New. > + (cs_etm_add_mem_access_callback): New. > + (cs_etm_process_data_block): New. > + (btrace_print_all): New. > + (btrace_compute_ftrace_etm): New. > + (btrace_compute_ftrace_1): add handling of CoreSight traces. > + (btrace_enable): add error message if ETM unavailable. > + (btrace_stitch_trace): add handling of CoreSight traces. > + (maint_info_btrace_cmd): add handling of CoreSight trace format. > + * btrace.h (record_btrace_reg_entry): new. > + (btrace_insn): add a vector of record_btrace_reg_entry. > + * record-btrace.c (record_btrace_target::fetch_registers): fetch > + registers from insn->registers when available > + > 2021-02-25 Zied Guermazi > > * record-btrace.c (record_btrace_print_etm_conf): New. > @@ -9,7 +34,7 @@ > (_initialize_record_btrace): add commands for ETM traces. > * record.c (record_start): add starting ETM traces. > > -2021-02-02 Zied Guermazi > +2021-02-25 Zied Guermazi > > * Makefile.in LIBOPENCSD_C_API: set the flag. > * config.in LIBOPENCSD_C_API: undefine it if > diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h > index fa589fd0582..93fd8b7d163 100644 > --- a/gdb/arch/arm.h > +++ b/gdb/arch/arm.h > @@ -150,6 +150,39 @@ enum arm_m_profile_type { > #define BranchDest(addr,instr) \ > ((CORE_ADDR) (((unsigned long) (addr)) + 8 + (sbits (instr, 0, 23) << 2))) > > +/* Exception numbers as encoded in ETMv3.4 for ARMv7-M processors. > + see IHI0014Q_etm_architecture_spec table 7.11 */ see -> See > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_USAGE_FAULT 0x009 > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_NMI 0x00a > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_SVC 0x00b > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_DEBUG_MONITOR 0x00c > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_MEM_USAGE 0x00d > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_PEND_SV 0x00e > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_SYS_TICK 0x00f > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_PROCESSOR_RESET 0x011 > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_HARD_FAULT 0x013 > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_BUS_FAULT 0x015 > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_IRQ(n) \ > + (n == 0 ? 0x008 : n < 8 ? n : n + 0x010) > +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_IS_IRQ(n) \ > + ((n > 0x000 && n < 0x009)||(n > 0x017 && n < 0x200)) > + > +/* Exception numbers as encoded in ETMv3.4 for non ARMv7-M processors. > + see IHI0014Q_etm_architecture_spec table 7.12 */ see -> See > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_HALTING_DEBUG 0x01 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SECURE_MONITOR_CALL 0x02 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_ENTRY_TO_HYP_MODE 0x03 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_ASYNCHRONOUS_DATA_ABORT 0x04 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_JAZELLE_THUMBEE_CHECK 0x05 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_PROCESSOR_RESET 0x08 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_UNDEFINED_INSTRUCTION 0x09 > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SUPERVISOR_CALL 0x0a > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_PREFETCH_ABORT_SW_BKPT 0x0b > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SYNCHRONOUS_DATA_ABORT_SW_WP 0x0c > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_GENERIC 0x0d > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_IRQ 0x0e > +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_FIQ 0x0f > + > /* Forward declaration. */ > struct regcache; > > diff --git a/gdb/btrace.c b/gdb/btrace.c > index c697f37f46c..1ab5a74f0b6 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -42,6 +42,7 @@ > #include > #include > #include > +#include "arch/arm.h" > > /* Command lists for btrace maintenance commands. */ > static struct cmd_list_element *maint_btrace_cmdlist; > @@ -257,6 +258,8 @@ ftrace_new_function (struct btrace_thread_info *btinfo, > } > > btinfo->functions.emplace_back (mfun, fun, number, insn_offset, level); > + ftrace_debug (&btinfo->functions.back (), "new function"); > + > return &btinfo->functions.back (); > } > > @@ -671,6 +674,36 @@ ftrace_update_insns (struct btrace_function *bfun, const btrace_insn &insn) > ftrace_debug (bfun, "update insn"); > } > > +#if defined (HAVE_LIBOPENCSD_C_API) > + /* Remove last instruction from BFUN's list. > + > + This function is not generic and is granted to work properly > + only if the same removed instruction will be added later. */ > + The indentation of the comment block seems wrong. The comment itself reads a bit confusing, but maybe that's me not understanding the tracing mechanisms. Could it be clarified? > +static void > +ftrace_remove_last_insn (struct btrace_thread_info *btinfo) > +{ > + /* If we didn't have a function, we return. */ > + if (btinfo->functions.empty ()) > + return; > + > + struct btrace_function *bfun; spurious space before *bfun. > + bfun = &btinfo->functions.back (); > + /* If we had a gap before, we return. */ > + if (bfun->errcode != 0) > + return; > + > + if (!bfun->insn.empty ()) > + bfun->insn.pop_back (); > + else > + { > + /* A valid function must have at least one insn. */ insn -> instruction, given it is a comment. I think we should spell it out completely. > + internal_error (__FILE__, __LINE__, > + _("Attempt to remove last instruction from an empty function")); Indentation is wrong for the error message. > + } > +} > +#endif /* defined (HAVE_LIBOPENCSD_C_API) */ > + > /* Classify the instruction at PC. */ > > static enum btrace_insn_class > @@ -1502,6 +1535,562 @@ btrace_compute_ftrace_pt (struct thread_info *tp, > > #endif /* defined (HAVE_LIBIPT) */ > > +#if defined (HAVE_LIBOPENCSD_C_API) > + > +/* This structure holds the status and the context for decoding ETM traces. > +It is also used in the ETM trace element callback to get the context. */ Indentation of the second line is off. > +struct cs_etm_decoder > +{ > + /* The tree representing CoreSight architecture in the SoC. */ > + dcd_tree_handle_t dcd_tree; > + > + /* Callback function to allow the decoder to access program memory. */ > + Fn_MemAcc_CB mem_access; The GNU Coding Standards says variables should be lower case. > + > + /* thread_info of traced thread. */ > + struct thread_info *t_info; > + > + /* returned value of previously processed block. */ The GNU Coding Standards says sentences should start with a capital letter unless they are identifiers, like the thread_info case. Please fix all further cases of this throughout the series. > + ocsd_datapath_resp_t prev_return; > + > + /* ARM architecture version of associated core. */ > + ocsd_arch_version_t arch_version; > + > + /* list of gaps in the execution record. */ > + std::vector &gaps; > +}; > + > +/* fills a ocsd_etmv3_cfg from a cs_etm_trace_params. */ > + > +static void > +cs_etm_get_etmv3_config (const struct cs_etm_trace_params *params, > + ocsd_etmv3_cfg *config) > +{ > + config->reg_idr = params->etmv3.reg_idr; > + config->reg_ctrl = params->etmv3.reg_ctrl; > + config->reg_ccer = params->etmv3.reg_ccer; > + config->reg_trc_id = params->etmv3.reg_trc_id; > + config->arch_ver = (ocsd_arch_version_t)params->arch_ver; > + config->core_prof = (ocsd_core_profile_t)params->core_profile; > +} > + > +/* fills a ocsd_etmv4_cfg from a cs_etm_trace_params. */ > + > +static void > +cs_etm_get_etmv4_config (const struct cs_etm_trace_params *params, > + ocsd_etmv4_cfg *config) > +{ > + config->reg_configr = params->etmv4.reg_configr; > + config->reg_traceidr = params->etmv4.reg_traceidr; > + config->reg_idr0 = params->etmv4.reg_idr0; > + config->reg_idr1 = params->etmv4.reg_idr1; > + config->reg_idr2 = params->etmv4.reg_idr2; > + config->reg_idr8 = params->etmv4.reg_idr8; > + config->reg_idr9 = 0; > + config->reg_idr10 = 0; > + config->reg_idr11 = 0; > + config->reg_idr12 = 0; > + config->reg_idr13 = 0; > + config->arch_ver = (ocsd_arch_version_t)params->arch_ver; > + config->core_prof = (ocsd_core_profile_t)params->core_profile; > +} > + > +/* Set the insn flag to track ISA mode of ARM processor. */ > + insn -> instruction > +static btrace_insn_flags > +cs_etm_get_isa_flag (const ocsd_generic_trace_elem *elem) > +{ > + switch (elem->isa) > + { > + case ocsd_isa_arm: > + return BTRACE_INSN_FLAG_ISA_ARM; > + > + case ocsd_isa_thumb2: > + return BTRACE_INSN_FLAG_ISA_THUMB2; > + > + case ocsd_isa_aarch64: > + return BTRACE_INSN_FLAG_ISA_AARCH64; > + > + case ocsd_isa_tee: > + return BTRACE_INSN_FLAG_ISA_TEE; > + > + case ocsd_isa_jazelle: > + return BTRACE_INSN_FLAG_ISA_JAZELLE; > + > + case ocsd_isa_custom: > + return BTRACE_INSN_FLAG_ISA_CUSTOM; > + > + case ocsd_isa_unknown: > + return BTRACE_INSN_FLAG_ISA_UNKNOWN; > + > + default: > + return 0; Does "0" map to any meaningful btrace_insn_flags value? It doesn't look like it. We should either definete it, or error/assert here. > + } > +} > + > +static enum btrace_insn_class > +cs_etm_get_instruction_class (const ocsd_generic_trace_elem *elem) > +{ > + switch (elem->last_i_type) > + { > + case OCSD_INSTR_BR: > + case OCSD_INSTR_BR_INDIRECT: > + switch (elem->last_i_subtype) > + { > + case OCSD_S_INSTR_V8_RET: > + case OCSD_S_INSTR_V8_ERET: > + case OCSD_S_INSTR_V7_IMPLIED_RET: > + return BTRACE_INSN_RETURN; > + > + case OCSD_S_INSTR_BR_LINK: > + return BTRACE_INSN_CALL; > + > + case OCSD_S_INSTR_NONE: > + return BTRACE_INSN_JUMP; > + } > + return BTRACE_INSN_OTHER; > + > + case OCSD_INSTR_ISB: > + case OCSD_INSTR_DSB_DMB: > + case OCSD_INSTR_WFI_WFE: > + case OCSD_INSTR_OTHER: > + return BTRACE_INSN_OTHER; > + > + default: > + return BTRACE_INSN_OTHER; > + > + } > +} > + > +/* Update btrace in the case of an instruction range. */ > + > +static void > +cs_etm_update_btrace_with_inst_range (const void *context, > + const ocsd_generic_trace_elem *elem) > +{ > + gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_INSTR_RANGE); > + > + struct cs_etm_decoder *etm_decoder; > + etm_decoder = (struct cs_etm_decoder *) context; > + if (etm_decoder->t_info == nullptr) > + return; > + > + struct thread_info *tp; > + tp = etm_decoder->t_info; > + > + struct btrace_thread_info *btinfo; > + btinfo = &tp->btrace; > + > + struct gdbarch *gdbarch; > + gdbarch = target_gdbarch (); It is better to have it like so: struct gdbarch *gdbarch = target_gdbarch (); > + > + struct btrace_insn insn; > + CORE_ADDR pc; > + int size; > + pc = elem->st_addr; Make it shorter like so: CORE_ADDR pc = elem->st_addr; > + for (int i = 0; i< elem->num_instr_range; i++) > + { > + insn.pc = pc; > + try > + { > + size = gdb_insn_length (gdbarch, pc); > + } > + catch (const gdb_exception_error &err) > + { > + error (_("Failed to get the size of the instruction.")); > + } > + > + struct btrace_function *bfun; > + bfun = ftrace_update_function (btinfo, pc); > + insn.iclass = BTRACE_INSN_OTHER; > + insn.size = size; > + if (etm_decoder->arch_version == ARCH_V7) > + insn.flags = cs_etm_get_isa_flag (elem); > + > + if (i == elem->num_instr_range -1) > + insn.iclass = cs_etm_get_instruction_class (elem); > + > + ftrace_update_insns (bfun, insn); > + pc = pc + size; > + } > +} > + > +/* Update btrace in the case of an exception. */ > + > +static void > +cs_etm_update_btrace_with_exception (const void *context, > + const ocsd_generic_trace_elem *elem) > +{ > + gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_EXCEPTION); > + > + struct cs_etm_decoder *etm_decoder; > + etm_decoder = (struct cs_etm_decoder *) context; > + if (etm_decoder->t_info == nullptr) > + return; > + > + struct thread_info *tp; > + tp = etm_decoder->t_info; > + > + struct btrace_thread_info *btinfo; > + btinfo = &tp->btrace; > + /* Handle the implementation of breakpoints in gdb for arm (v7) architecture > + using undefined instructions. */ > + > + if (etm_decoder->arch_version == ARCH_V7) > + { > + if (elem->exception_number > + == CS_ETMV3_4_CORTEX_A_R_EXCEPTION_UNDEFINED_INSTRUCTION) > + { > + DEBUG ("handle breakpoints implementation in gdb for ARMv7"); > + ftrace_remove_last_insn (btinfo); > + } > + } > +} > + > +/* Update btrace in the case of a trace on. */ > + > +static void > +cs_etm_update_btrace_with_trace_on (const void *context, > + const ocsd_generic_trace_elem *elem) > +{ > + gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_TRACE_ON); > + > + struct cs_etm_decoder *etm_decoder; > + etm_decoder = (struct cs_etm_decoder *)context; struct cs_etm_decoder *etm_decoder = (struct cs_etm_decoder *) context; > + if (etm_decoder->t_info == nullptr) > + return; > + > + struct thread_info *tp; > + tp = etm_decoder->t_info; struct thread_info *tp = etm_decoder->t_info; > + > + struct btrace_thread_info *btinfo; > + btinfo = &tp->btrace; struct btrace_thread_info *btinfo = &tp->btrace; > + > + if (elem->trace_on_reason != TRACE_ON_NORMAL) > + ftrace_new_gap (btinfo, elem->trace_on_reason, etm_decoder->gaps); > + Spurious new line. > +} > + > +/* Callback function when a ocsd_generic_trace_elem is emitted. */ > + > +static ocsd_datapath_resp_t > +cs_etm_trace_element_callback (const void *context, > + const ocsd_trc_index_t indx, indx -> index? > + const uint8_t trace_chan_id, > + const ocsd_generic_trace_elem *elem) > +{ > + char str_buffer[128]; How is this buffer size determined? Is it safe for it to be hardcoded like this? Or should we use a std:string instead? > + if (ocsd_gen_elem_str (elem, str_buffer, 128) == OCSD_OK) > + DEBUG ("ETM trace_element: index= %d, channel= 0x%x, %s", > + indx, trace_chan_id, str_buffer); Given GDB can be built as a cross debugger, we shouldn't expect sizes to be the same for all architectures. From what I've read, ocsd_trc_index_t can be either 32-bit or 64-bit. I think we should print it as %s using pulongest instead. We can also print %x values using %s and phex/phex_nz if you want. > + > + switch (elem->elem_type) > + { > + case OCSD_GEN_TRC_ELEM_TRACE_ON: > + cs_etm_update_btrace_with_trace_on (context, elem); > + break; > + > + case OCSD_GEN_TRC_ELEM_INSTR_RANGE: > + cs_etm_update_btrace_with_inst_range (context, elem); > + break; > + > + case OCSD_GEN_TRC_ELEM_EXCEPTION: > + cs_etm_update_btrace_with_exception (context, elem); > + break; > + > + case OCSD_GEN_TRC_ELEM_UNKNOWN: > + case OCSD_GEN_TRC_ELEM_EO_TRACE: > + case OCSD_GEN_TRC_ELEM_NO_SYNC: > + case OCSD_GEN_TRC_ELEM_EXCEPTION_RET: > + case OCSD_GEN_TRC_ELEM_TIMESTAMP: > + case OCSD_GEN_TRC_ELEM_PE_CONTEXT: > + case OCSD_GEN_TRC_ELEM_ADDR_NACC: > + case OCSD_GEN_TRC_ELEM_CYCLE_COUNT: > + case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN: > + case OCSD_GEN_TRC_ELEM_EVENT: > + case OCSD_GEN_TRC_ELEM_SWTRACE: > + case OCSD_GEN_TRC_ELEM_CUSTOM: > + default: > + break; Why have the above block if we're just falling through and breaking? I'd say just remove these and let it return OCSD_RESP_CONT. > + > + } > + return OCSD_RESP_CONT; > +} > + > +/* Create a cs_etm_decoder and initialize it. */ > + > +static bool > +cs_etm_create_decoder (struct cs_etm_trace_params *t_params, > + struct cs_etm_decoder *decoder) > +{ > + const char *decoder_name; > + ocsd_etmv3_cfg config_etmv3; > + ocsd_etmv4_cfg trace_config_etmv4; > + void *trace_config; > + switch (t_params->protocol) > + { > + case OCSD_PROTOCOL_ETMV3: > + case OCSD_PROTOCOL_PTM: > + cs_etm_get_etmv3_config (t_params, &config_etmv3); > + decoder_name = (t_params->protocol == OCSD_PROTOCOL_ETMV3) > + ? OCSD_BUILTIN_DCD_ETMV3 : OCSD_BUILTIN_DCD_PTM; > + trace_config = &config_etmv3; > + decoder->arch_version = ARCH_V7; > + break; > + > + case OCSD_PROTOCOL_ETMV4I: > + cs_etm_get_etmv4_config (t_params, &trace_config_etmv4); > + decoder_name = OCSD_BUILTIN_DCD_ETMV4I; > + trace_config = &trace_config_etmv4; > + decoder->arch_version = ARCH_V8; > + break; > + > + default: > + decoder->arch_version = ARCH_UNKNOWN; > + DEBUG ("cs_etm_create_decoder: Unknown architecture version"); > + return false; > + > + } > + uint8_t csid; > + if (ocsd_dt_create_decoder (decoder->dcd_tree, decoder_name, > + OCSD_CREATE_FLG_FULL_DECODER, > + trace_config, &csid)) > + { > + DEBUG ("ocsd_dt_create_decoder failed"); > + return false; > + } > + > + if (ocsd_dt_set_gen_elem_outfn (decoder->dcd_tree, > + cs_etm_trace_element_callback, > + decoder)) > + { > + DEBUG ("ocsd_dt_set_gen_elem_outfn failed"); > + return false; > + } > + > + decoder->prev_return = OCSD_RESP_CONT; > + return true; > +} > + > +/* Free a cs_etm_decoder. */ > + > +static void > +cs_etm_free_decoder (struct cs_etm_decoder *decoder) > +{ > + if (decoder == nullptr) > + return; > + > + ocsd_destroy_dcd_tree (decoder->dcd_tree); > + decoder->dcd_tree = nullptr; > + decoder->t_info = nullptr; > + free (decoder); malloc/free are not supposed to be called directly. Use xmalloc/xfree. > +} > + > +/* Allocate a cs_etm_decoder and initialize it. */ > + > +static struct cs_etm_decoder * > +cs_etm_alloc_decoder (struct thread_info *tp, int num_cpu, > + struct cs_etm_decoder_params d_params, > + std::vector * t_params) > +{ > + ocsd_dcd_tree_src_t src_type = OCSD_TRC_SRC_SINGLE; > + uint32_t deformatterCfgFlags = 0; > + > + if (d_params.formatted) > + src_type = OCSD_TRC_SRC_FRAME_FORMATTED; > + if (d_params.frame_aligned) > + deformatterCfgFlags |= OCSD_DFRMTR_FRAME_MEM_ALIGN; > + if (d_params.fsyncs) > + deformatterCfgFlags |= OCSD_DFRMTR_HAS_FSYNCS; > + if (d_params.hsyncs) > + deformatterCfgFlags |= OCSD_DFRMTR_HAS_HSYNCS; > + if (d_params.reset_on_4x_sync) > + deformatterCfgFlags |= OCSD_DFRMTR_RESET_ON_4X_FSYNC; > + > + dcd_tree_handle_t dcdtree_handle; > + dcdtree_handle = ocsd_create_dcd_tree (src_type, deformatterCfgFlags); > + > + if (dcdtree_handle == C_API_INVALID_TREE_HANDLE) > + { > + DEBUG ("ocsd_create_dcd_tree failed"); > + return nullptr; > + } > + struct cs_etm_decoder *decoder; > + > + decoder = (struct cs_etm_decoder*) xmalloc (sizeof (struct cs_etm_decoder)); > + decoder->dcd_tree = dcdtree_handle; > + > + bool ret; > + for (int i = 0; i < num_cpu; i++) > + { > + ret = cs_etm_create_decoder (&(t_params->at (i)), decoder); > + if (ret == false) You can use ret/!ret for bools. > + { > + DEBUG ("cs_etm_create_decoder failed"); > + cs_etm_free_decoder (decoder); > + return nullptr; > + } > + > + } > + decoder->t_info = tp; > + return decoder; > +} > + > +/* A callback function to allow the trace decoder to read the inferior's > + memory. */ > + > +static uint32_t > +btrace_etm_readmem_callback (const void *p_context, const ocsd_vaddr_t address, > + const ocsd_mem_space_acc_t mem_space, > + const uint32_t reqBytes, uint8_t *byteBuffer) > +{ > + int result, errcode; > + > + result = (int) reqBytes; > + try > + { > + errcode = target_read_code ((CORE_ADDR) address, byteBuffer, reqBytes); > + if (errcode != 0) > + result = 0; > + } > + catch (const gdb_exception_error &error) > + { > + result = 0; > + } > + > + return result; > +} > + > +/* Add memory access callback to the decoder. */ > + > +static ocsd_err_t > +cs_etm_add_mem_access_callback (struct cs_etm_decoder *decoder, > + CORE_ADDR start, CORE_ADDR end, > + Fn_MemAcc_CB p_cb_func) > +{ > + ocsd_err_t error; > + error = ocsd_dt_add_callback_mem_acc (decoder->dcd_tree, > + (ocsd_vaddr_t) start, > + (ocsd_vaddr_t) end, > + OCSD_MEM_SPACE_ANY, > + p_cb_func, decoder); > + if (error == OCSD_OK) > + decoder->mem_access = p_cb_func; > + > + return error; > +} > + > +/* Process an etm traces data block. */ > + > +static int > +cs_etm_process_data_block (struct cs_etm_decoder *decoder, > + uint64_t index, const uint8_t *buf, > + size_t len, size_t *consumed) > +{ > + int ret = 0; > + ocsd_datapath_resp_t cur = OCSD_RESP_CONT; > + ocsd_datapath_resp_t prev_return = decoder->prev_return; > + size_t processed = 0; > + uint32_t count; > + > + while (processed < len) > + { > + if (OCSD_DATA_RESP_IS_WAIT (prev_return)) > + { > + cur = ocsd_dt_process_data (decoder->dcd_tree, OCSD_OP_FLUSH, > + 0, 0, nullptr, nullptr); > + } else if (OCSD_DATA_RESP_IS_CONT (prev_return)) > + { > + cur = ocsd_dt_process_data (decoder->dcd_tree, > + OCSD_OP_DATA, > + index + processed, len - processed, > + &buf[processed], &count); > + processed += count; > + } else > + { > + DEBUG_FTRACE ("ocsd_dt_process_data returned with %d.\n", cur); Same thing about GDB being built as a cross debugger. If ocsd_datapath_resp_t can be a 32-bit or 64-bit value, then it is safer to use the %s modifier and pulongest. > + ret = -EINVAL; > + break; > + } > + > + /* Return to the input code if the packet buffer is full. > + Flushing will get done once the packet buffer has been > + processed. */ > + if (OCSD_DATA_RESP_IS_WAIT (cur)) > + break; > + > + prev_return = cur; > + } > + > + decoder->prev_return = cur; > + *consumed = processed; > + > + return ret; Given this only returns 0 and -EINVAL, should we return a bool instead for success/failure? > +} > + > +/* Print all function in a btrace. */ function -> functions > + > +static void > +btrace_print_all (struct btrace_thread_info *btinfo) > +{ > + for (const btrace_function &function : btinfo->functions) > + ftrace_debug (&function, ""); > +} > + > +static void > +btrace_compute_ftrace_etm (struct thread_info *tp, > + const struct btrace_data_etm *btrace, > + std::vector &gaps) > +{ > + DEBUG_FTRACE ("btrace->size is 0x%x for thread %s", > + (unsigned int)(btrace->size), print_thread_id (tp)); > + if (btrace->size == 0) > + return; > + > + struct btrace_thread_info *btinfo; > + btinfo = &tp->btrace; struct btrace_thread_info *btinfo = &tp->btrace; > + if (btinfo->functions.empty ()) > + btinfo->level = 0; > + > + struct cs_etm_decoder *decoder; > + decoder = cs_etm_alloc_decoder (tp,btrace->config.num_cpu, > + btrace->config.etm_decoder_params, > + btrace->config.etm_trace_params); struct cs_etm_decoder *decoder = cs_etm_alloc_decoder (tp,btrace->config.num_cpu, btrace->config.etm_decoder_params, btrace->config.etm_trace_params); More cases of this below... > + if (decoder == nullptr) > + error (_("Failed to allocate ARM CoreSight ETM Trace decoder.")); > + > + ocsd_err_t ocsd_error; > + ocsd_error = cs_etm_add_mem_access_callback (decoder, > + (CORE_ADDR)0x0L, (CORE_ADDR) -1L, > + btrace_etm_readmem_callback); ... here... > + if (ocsd_error!= OCSD_OK) > + error (_("Failed to add CoreSight Trace decoder memory access callback.")); > + > + int errcode; > + size_t consumed; > + errcode = cs_etm_process_data_block (decoder, > + 0, btrace->data, > + btrace->size, &consumed); and here. > + if (errcode!=0) > + error (_("Failed to decode ARM CoreSight ETM Trace.")); > + ftrace_compute_global_level_offset (btinfo); > + btrace_add_pc (tp); > + btrace_print_all (btinfo); > + cs_etm_free_decoder (decoder); > + > +} > +#else /* defined (HAVE_LIBOPENCSD_C_API) */ > + > +static void > +btrace_compute_ftrace_etm (struct thread_info *tp, > + const struct btrace_data_etm *btrace, > + std::vector &gaps) > +{ > + Spurious newline. > + internal_error (__FILE__, __LINE__, _("Unexpected branch trace format.")); > +} > +#endif /* defined (HAVE_LIBOPENCSD_C_API) */ > + > /* Compute the function branch trace from a block branch trace BTRACE for > a thread given by BTINFO. If CPU is not NULL, overwrite the cpu in the > branch trace configuration. This is currently only used for the PT > @@ -1531,6 +2120,10 @@ btrace_compute_ftrace_1 (struct thread_info *tp, > > btrace_compute_ftrace_pt (tp, &btrace->variant.pt, gaps); > return; > + > + case BTRACE_FORMAT_ETM: > + btrace_compute_ftrace_etm (tp, &btrace->variant.etm, gaps); > + return; > } > > internal_error (__FILE__, __LINE__, _("Unknown branch trace format.")); > @@ -1599,6 +2192,10 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf) > if (conf->format == BTRACE_FORMAT_PT) > error (_("Intel Processor Trace support was disabled at compile time.")); > #endif /* !defined (HAVE_LIBIPT) */ > +#if !defined (HAVE_LIBOPENCSD_C_API) > + if (conf->format == BTRACE_FORMAT_ETM) > + error (_("ARM CoreSight Trace support was disabled at compile time.")); Maybe rephrase it as the following? error (_("ARM CoreSight Trace support disabled at compile time.")); > +#endif /* !defined (HAVE_LIBOPENCSD_C_API) */ > > DEBUG ("enable thread %s (%s)", print_thread_id (tp), > target_pid_to_str (tp->ptid).c_str ()); > @@ -1791,6 +2388,10 @@ btrace_stitch_trace (struct btrace_data *btrace, struct thread_info *tp) > case BTRACE_FORMAT_PT: > /* Delta reads are not supported. */ > return -1; > + > + case BTRACE_FORMAT_ETM: > + /* Delta reads are not supported. */ > + return -1; Given the outcome is the same, we should move this case up together with BTRACE_FORMAT_PT. > } > > internal_error (__FILE__, __LINE__, _("Unknown branch trace format.")); > @@ -3413,6 +4014,11 @@ maint_info_btrace_cmd (const char *args, int from_tty) > } > break; > #endif /* defined (HAVE_LIBIPT) */ > +#if defined (HAVE_LIBOPENCSD_C_API) > + case BTRACE_FORMAT_ETM: > + printf_unfiltered (_("Version: %s.\n"), ocsd_get_version_str ()); > + break; > +#endif /* defined (HAVE_LIBOPENCSD_C_API) */ > } > } > > diff --git a/gdb/btrace.h b/gdb/btrace.h > index 8f6ce32103d..92e14b6aadf 100644 > --- a/gdb/btrace.h > +++ b/gdb/btrace.h > @@ -34,6 +34,12 @@ > # include > #endif > > +#if defined (HAVE_LIBOPENCSD_C_API) > +# include > +# include > +# include > +#endif > + > #include > > struct thread_info; > @@ -59,7 +65,15 @@ enum btrace_insn_class > enum btrace_insn_flag > { > /* The instruction has been executed speculatively. */ > - BTRACE_INSN_FLAG_SPECULATIVE = (1 << 0) > + BTRACE_INSN_FLAG_SPECULATIVE = (1 << 0), > + BTRACE_INSN_FLAG_ISA_ARM = (1 << 24), > + BTRACE_INSN_FLAG_ISA_THUMB2 = (2 << 24), > + BTRACE_INSN_FLAG_ISA_AARCH64 = (3 << 24), > + BTRACE_INSN_FLAG_ISA_TEE = (4 << 24), > + BTRACE_INSN_FLAG_ISA_JAZELLE = (5 << 24), > + BTRACE_INSN_FLAG_ISA_CUSTOM = (6 << 24), > + BTRACE_INSN_FLAG_ISA_UNKNOWN = (7 << 24), > + BTRACE_INSN_FLAG_ISA_MASK = (15 << 24) > }; > DEF_ENUM_FLAGS_TYPE (enum btrace_insn_flag, btrace_insn_flags); > > diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c > index 0c9ab7c2529..6002526c3cd 100644 > --- a/gdb/record-btrace.c > +++ b/gdb/record-btrace.c > @@ -44,6 +44,7 @@ > #include "cli/cli-style.h" > #include "async-event.h" > #include > +#include "arch/arm.h" > > static const target_info record_btrace_target_info = { > "record-btrace", > @@ -1556,6 +1557,30 @@ record_btrace_target::remove_breakpoint (struct gdbarch *gdbarch, > return ret; > } > > +/* Reconstruct the content of CPSR register according to insn->flags. */ content of CPSR -> contents of the CPSR "insn->flags" -> instruction flags? > + > +static unsigned int > +cs_etm_reconstruct_cpsr (const struct btrace_insn *insn) > +{ > + switch (insn->flags & BTRACE_INSN_FLAG_ISA_MASK) > + { > + case BTRACE_INSN_FLAG_ISA_ARM: > + return 0; > + > + case BTRACE_INSN_FLAG_ISA_THUMB2: > + return 0x20; > + > + case BTRACE_INSN_FLAG_ISA_TEE: > + return 0x1000020; > + > + case BTRACE_INSN_FLAG_ISA_JAZELLE: > + return 0x1000000; > + For better readability, we should make all of these magic numbers constants with a meaningful name. > + default: > + return 0; What does 0 mean in this context? Should we error/assert if this isn't valid? > + } > +} > + > /* The fetch_registers method of target record-btrace. */ > > void > @@ -1576,19 +1601,38 @@ record_btrace_target::fetch_registers (struct regcache *regcache, int regno) > struct gdbarch *gdbarch; > int pcreg; > > + insn = btrace_insn_get (replay); > + gdb_assert (insn != nullptr); > + > gdbarch = regcache->arch (); > pcreg = gdbarch_pc_regnum (gdbarch); > if (pcreg < 0) > return; > - > - /* We can only provide the PC register. */ > - if (regno >= 0 && regno != pcreg) > + /* We can only provide the PC or cpsr registers here. */ For consistency... cpsr -> CPSR > + if (regno >= 0 && !(regno == pcreg || regno == ARM_PS_REGNUM)) > return; > > - insn = btrace_insn_get (replay); > - gdb_assert (insn != NULL); > - > + if (regno == pcreg) > + { > regcache->raw_supply (regno, &insn->pc); > + return; > + } > + if (regno == ARM_PS_REGNUM) > + { > + /* Provide cpsr register in the case of an armv7 target. */ cpsr -> CPSR > + const struct target_desc *tdesc; > + tdesc = gdbarch_target_desc (gdbarch); const struct target_desc *tdesc = gdbarch_target_desc (gdbarch); > + > + const char *tdesc_name; > + tdesc_name = tdesc_architecture_name (tdesc); > + if (strcmp (tdesc_name, "arm") == 0) > + { > + int cpsr; > + cpsr = cs_etm_reconstruct_cpsr (insn); > + regcache->raw_supply (regno, &cpsr); > + } > + return; > + } > } > else > this->beneath ()->fetch_registers (regcache, regno); > diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog > index ed4708b7028..38dc5a79277 100644 > --- a/gdbsupport/ChangeLog > +++ b/gdbsupport/ChangeLog > @@ -1,3 +1,16 @@ > +2021-02-25 Zied Guermazi > + > + * btrace-common.h (cs_etmv3_trace_params): New > + (cs_etmv4_trace_params): New. > + (cs_etm_trace_params): New. > + (cs_etm_decoder_params): New > + (btrace_data_etm_config): New. > + (btrace_data_etm): New. > + (btrace_data): add a btrace_data_etm. > + * btrace-common.cc (btrace_data::fini): handle CoreSight traces. > + (btrace_data::empty): handle CoreSight traces. > + (btrace_data_append): handle CoreSight traces. > + > 2021-02-25 Zied Guermazi > > * btrace-common.h (btrace_format): add BTRACE_FORMAT_ETM > diff --git a/gdbsupport/btrace-common.cc b/gdbsupport/btrace-common.cc > index 82701942aa9..0ddc140d0c9 100644 > --- a/gdbsupport/btrace-common.cc > +++ b/gdbsupport/btrace-common.cc > @@ -86,6 +86,10 @@ btrace_data::fini () > case BTRACE_FORMAT_PT: > xfree (variant.pt.data); > return; > + > + case BTRACE_FORMAT_ETM: > + xfree (variant.etm.data); > + return; > } > > internal_error (__FILE__, __LINE__, _("Unkown branch trace format.")); > @@ -106,6 +110,9 @@ btrace_data::empty () const > > case BTRACE_FORMAT_PT: > return (variant.pt.size == 0); > + > + case BTRACE_FORMAT_ETM: > + return (variant.etm.size == 0); > } > > internal_error (__FILE__, __LINE__, _("Unkown branch trace format.")); > @@ -191,6 +198,39 @@ btrace_data_append (struct btrace_data *dst, > } > } > return 0; > + > + case BTRACE_FORMAT_ETM: > + switch (dst->format) > + { > + default: > + return -1; I know this was copied from the current code, but I think this default case should go to the last position in the switch block. Though valid, it looks really odd like this (at least for me). > + > + case BTRACE_FORMAT_NONE: > + dst->format = BTRACE_FORMAT_ETM; > + dst->variant.etm.data = nullptr; > + dst->variant.etm.size = 0; > + > + /* fall-through. */ > + case BTRACE_FORMAT_ETM: > + { > + gdb_byte *data; > + size_t size; > + > + size = src->variant.etm.size + dst->variant.etm.size; > + data = (gdb_byte *) xmalloc (size); size_t size = src->variant.etm.size + dst->variant.etm.size; gdb_byte *data = (gdb_byte *) xmalloc (size); > + > + if (dst->variant.etm.size > 0) > + memcpy (data, dst->variant.etm.data, dst->variant.etm.size); > + memcpy (data + dst->variant.etm.size, src->variant.etm.data, > + src->variant.etm.size); > + > + xfree (dst->variant.etm.data); > + > + dst->variant.etm.data = data; > + dst->variant.etm.size = size; > + } > + } > + return 0; > } > > internal_error (__FILE__, __LINE__, _("Unkown branch trace format.")); > diff --git a/gdbsupport/btrace-common.h b/gdbsupport/btrace-common.h > index 153b977723a..c04e9954377 100644 > --- a/gdbsupport/btrace-common.h > +++ b/gdbsupport/btrace-common.h > @@ -187,6 +187,96 @@ struct btrace_data_pt > size_t size; > }; > > +/* Parameters needed for decoding ETMv3 traces. > + See open source coresight trace decoder library (opencsd) > + for further details. */ > +struct cs_etmv3_trace_params > +{ > + /* ETMv3 Main Control Register. */ > + uint32_t reg_ctrl; > + > + /* ETMv3 Trace ID Register. */ > + uint32_t reg_trc_id; > + > + /* ETMv3 Configuration Code Extension Register. */ > + uint32_t reg_ccer; > + > + /* ETMv3 ID Register. */ > + uint32_t reg_idr; > +}; > + > +/* Parameters needed for decoding ETMv4 traces. > + See open source coresight trace decoder library (opencsd) > + for further details. */ > +struct cs_etmv4_trace_params > +{ > + /* ETMv4 ID Register 0. */ > + uint32_t reg_idr0; > + > + /* ETMv4 ID Register 1. */ > + uint32_t reg_idr1; > + > + /* ETMv4 ID Register 2. */ > + uint32_t reg_idr2; > + > + /* ETMv4 ID Register 8. */ > + uint32_t reg_idr8; > + > + /* ETMv4 Config Register. */ > + uint32_t reg_configr; > + > + /* ETMv4 Trace ID Register. */ > + uint32_t reg_traceidr; > +}; > + > +/* Parameters of trace source. */ > +struct cs_etm_trace_params > +{ > + int arch_ver; > + int core_profile; > + int protocol; > + union { > + struct cs_etmv3_trace_params etmv3; > + struct cs_etmv4_trace_params etmv4; > + }; > +}; > + > +/* Parameters of trace sink/decoder. */ > +struct cs_etm_decoder_params > +{ > + uint8_t formatted:1, /* Formatted input, or single source input. */ > + fsyncs :1, /* Formatted data has fsyncs. */ > + hsyncs :1, /* Formatted data has hsyncs. */ > + frame_aligned :1, /* Formatted frames are memory aligned. */ > + reset_on_4x_sync :1, /* Reset decoders at 4x consecutive fsyncs. */ > + __res :3; /* Reserved, not used. */ > +}; > + > +/* Configuration information to go with the etm trace data. */ > +struct btrace_data_etm_config > +{ > + /* The number of cpu (trace sources). */ I got slightly confused with this thinking it was the number of CPU's, but it is the CPU/Core number. How about rephrasing this as the following? "The CPU number (the trace source)" > + int num_cpu; And maybe rename this variable to "cpu" or "cpu_number". Just a suggestion. > + std::vector *etm_trace_params; > + struct cs_etm_decoder_params etm_decoder_params; > +}; > + > +/* Branch trace in arm Processor Trace format. */ arm -> ARM > +struct btrace_data_etm > +{ > + /* Some configuration information to go with the data. */ > + struct btrace_data_etm_config config; > + > + /* The trace data. */ > + gdb_byte *data; > + > + /* The size of DATA in bytes. */ > + size_t size; > + > + /* Trace id for this thread. set to 0xFF to ignore it during parsing. */ Two spaces after period. > + int trace_id; > +}; > + > /* The branch trace data. */ > struct btrace_data > { > @@ -224,6 +314,9 @@ struct btrace_data > > /* Format == BTRACE_FORMAT_PT. */ > struct btrace_data_pt pt; > + > + /* Format == BTRACE_FORMAT_ETM. */ > + struct btrace_data_etm etm; > } variant; > > private: >