From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 2iiOJ5YdO2khbjUAWB0awg (envelope-from ) for ; Thu, 11 Dec 2025 14:37:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1765481878; bh=02t37+QYp42JaGOQ6fllrx0+9Rv5uHknI5S6iC5NaWk=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=pVYo9nAucx7BNHJfdzNReC4ysz3se9LONPJen4FSb27jlgl2xq9JXUg2CLRxPv9C6 UURzElpZQqGSLAqindngL9+om0kx7ibk2J9XTPThqYm1iUUGhaWEYA/3n5ceP3AqT0 V64zrFKBMUa7Q1KzB43bHqaYArN2BDnj6rYXgp9s= Received: by simark.ca (Postfix, from userid 112) id 8A52D1E0B6; Thu, 11 Dec 2025 14:37:58 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=QGNbtU1j; dkim-atps=neutral Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D60981E08D for ; Thu, 11 Dec 2025 14:37:57 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 542A54BA2E27 for ; Thu, 11 Dec 2025 19:37:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 542A54BA2E27 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=QGNbtU1j Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B4BD74BA2E04 for ; Thu, 11 Dec 2025 19:37:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B4BD74BA2E04 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B4BD74BA2E04 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765481850; cv=none; b=dN1LsWHBJLxjEZSX4Qq5HqRlI8epVojiBnDK+xOdm6WSpqSLgp3D1WJCcHu1u9gCdEndEvvCFT2v8z+SMkmKGQ8Y/KIJbqej+kVg8W+JHsL6vNuRxcimPcDJ44cpq7Iwrp0WZ9rvm/ddsx8Xp4jjxeb8Z9hbjvaP8/MfVAa11XA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765481850; c=relaxed/simple; bh=02t37+QYp42JaGOQ6fllrx0+9Rv5uHknI5S6iC5NaWk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=S6y/4b3vUoiXFZPLXR9eUkWm6VvtuzVAai2YLBb/AyRxfkLcbu2zMt0RRmiOPiKq7OoQNBQGgGDyKCqt6+rY9PeTGbk4sluh8vBLhvYJ6dYBvfVZkwfDRWlEwUmEir7nUC+yvD/oJbwBcmrXuUKhY2hiXTg+fDqATK/6+r1me/s= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B4BD74BA2E04 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1765481850; bh=02t37+QYp42JaGOQ6fllrx0+9Rv5uHknI5S6iC5NaWk=; h=Date:Subject:To:References:From:In-Reply-To:From; b=QGNbtU1ji7ZRV2mWhE8nbJ1NNORI6MBlUKLrefu38S1NkfGfXgRoxzhOb6BS+2m+7 UcnFz6stZCMDso1ve0XZwOiNz7jjTWlOP0MYW0O5KO4oy7moB65X5fiLjzXvx9wXUi UsqlFy7zu0aH8e98rRwF76jeffmyeK82fsGtVKGU= Received: by simark.ca (Postfix) id 293E11E08D; Thu, 11 Dec 2025 14:37:30 -0500 (EST) Message-ID: Date: Thu, 11 Dec 2025 14:37:29 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 08/44] gdb, intelgt: add disassemble feature for the Intel GT architecture. To: Tankut Baris Aktemur , gdb-patches@sourceware.org, Markus Metzger References: <20250801-upstream-intelgt-mvp-v3-0-59ce0f87075b@intel.com> <20250801-upstream-intelgt-mvp-v3-8-59ce0f87075b@intel.com> Content-Language: en-US From: Simon Marchi In-Reply-To: <20250801-upstream-intelgt-mvp-v3-8-59ce0f87075b@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org > diff --git a/gdb/configure.ac b/gdb/configure.ac > index 226e27e4fe54a9d3ac06e8be4439dd5dce8eb975..0507f03043c61385181b667d0e7117d4b2547d6d 100644 > --- a/gdb/configure.ac > +++ b/gdb/configure.ac > @@ -1351,6 +1351,58 @@ fi > AC_SUBST(SRCHIGH_LIBS) > AC_SUBST(SRCHIGH_CFLAGS) > > +# Check for Intel(R) Graphics Technology assembler library > +intelgt_target=false > + > +for targ_alias in `echo $target_alias $enable_targets | sed 's/,/ /g'` > +do > + if test "$targ_alias" = "all"; then > + intelgt_target=true > + else > + case "$targ_alias" in > + intelgt-*) > + intelgt_target=true > + ;; > + esac > + fi > +done > + > +case "${target}" in > + intelgt-*) > + intelgt_target=true > + ;; > +esac Do you really need this last "case"? I would think that the main target would be included in the loop above (as $target_alias). > + > +if test "${intelgt_target}" != true; then > + HAVE_LIBIGA64=no > +else > + AC_ARG_WITH(libiga64, > + AS_HELP_STRING([--with-libiga64], [include IntelGT disassembly support (auto/yes/no)]), > + [], [with_libiga64=auto]) Not an autoconf expert, but I think it makes more sense (and is clearer) to put the AC_ARG_WITH at the top-level. The --with-libiga64 flag will always be there, it's not like it will be predicated by $intelgt_target. > @@ -544,13 +553,75 @@ intelgt_sw_breakpoint_from_kind (gdbarch *gdbarch, int kind, int *size) > return nullptr; > } > > +#if defined (HAVE_LIBIGA64) > +/* Map CORE_ADDR to symbol names for jump labels in an IGA disassembly. */ > + > +static const char * > +intelgt_disasm_sym_cb (int addr, void *ctx) > +{ > + disassemble_info *info = (disassemble_info *) ctx; > + symbol *sym = find_pc_function (addr + (uintptr_t) info->private_data); > + return sym ? sym->linkage_name () : nullptr; > +} > +#endif /* defined (HAVE_LIBIGA64) */ > + > /* Print one instruction from MEMADDR on INFO->STREAM. */ > > static int > intelgt_print_insn (bfd_vma memaddr, struct disassemble_info *info) > { > - /* Disassembler is to be added in a later patch. */ > +#if !defined (HAVE_LIBIGA64) > + gdb_printf (_("\nDisassemble feature not available: libiga64 " > + "is missing.\n")); > return -1; > +#else > + std::unique_ptr insn (new bfd_byte[intelgt::MAX_INST_LENGTH]); Can this be statically allocated? bfd_byte insn[intelgt::MAX_INST_LENGTH]; > + > + int status = (*info->read_memory_func) (memaddr, insn.get (), > + intelgt::COMPACT_INST_LENGTH, info); > + if (status != 0) > + { > + /* Aborts disassembling with a memory_error exception. */ > + (*info->memory_error_func) (status, memaddr, info); > + return -1; > + } > + > + uint32_t device_id = get_device_id (current_inferior ()); > + gdb::array_view insn_view > + = gdb::make_array_view (insn.get (), intelgt::COMPACT_INST_LENGTH); > + unsigned int length = intelgt::inst_length (insn_view, device_id); > + > + if (length == intelgt::MAX_INST_LENGTH) > + { > + status = (*info->read_memory_func) (memaddr, insn.get (), > + intelgt::MAX_INST_LENGTH, info); > + if (status != 0) > + { > + /* Aborts disassembling with a memory_error exception. */ > + (*info->memory_error_func) (status, memaddr, info); > + return -1; > + } > + } > + > + char *dbuf; > + iga_disassemble_options_t dopts = IGA_DISASSEMBLE_OPTIONS_INIT (); > + gdb_disassemble_info *di > + = static_cast(info->application_data); Space before paren. > @@ -771,6 +842,46 @@ intelgt_gdbarch_init (gdbarch_info info, gdbarch_list *arches) > intelgt_gdbarch_tdep *data > = gdbarch_tdep (gdbarch); > > +#if defined (HAVE_LIBIGA64) > + iga_gen_t iga_version = IGA_GEN_INVALID; > + > + if (tdesc != nullptr) > + { > + const tdesc_device *device_info = tdesc_device_info (tdesc); > + if (!(device_info->vendor_id.has_value () > + && device_info->target_id.has_value ())) > + { > + warning (_("Device vendor id and target id not found.")); Perhaps say "... not found in target description."? Trying to think about a clueless user seeing this message without much context. > + gdbarch_free (gdbarch); > + return nullptr; > + } > + > + uint32_t vendor_id = *device_info->vendor_id; > + uint32_t device_id = *device_info->target_id; > + if (vendor_id != 0x8086) > + { > + warning (_("Device not recognized: vendor id=0x%04x," > + " device id=0x%04x"), vendor_id, device_id); > + gdbarch_free (gdbarch); > + return nullptr; I don't think these two gdbarch_free are needed, that is managed by gdbarch_u. > + } > + else I would remove this "else" (just de-indent the code below). > + { > + iga_version = (iga_gen_t) intelgt::get_xe_version (device_id); > + if (iga_version == IGA_GEN_INVALID) > + warning (_("Intel GT device id is unrecognized: ID 0x%04x"), > + device_id); > + } > + } > + > + /* Take the best guess in case IGA_VERSION is still invalid. */ > + if (iga_version == IGA_GEN_INVALID) > + iga_version = IGA_XE_HPC; > + > + const iga_context_options_t options = IGA_CONTEXT_OPTIONS_INIT (iga_version); > + iga_context_create (&options, &data->iga_ctx); Should probably check the return value of iga_context_create. Simon