From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qGKzMiMRdWKjqQQAWB0awg (envelope-from ) for ; Fri, 06 May 2022 08:14:27 -0400 Received: by simark.ca (Postfix, from userid 112) id CC8A61E058; Fri, 6 May 2022 08:14:27 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=Sc/XIp9T; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 061E11E01D for ; Fri, 6 May 2022 08:14:27 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9C9FC395253C for ; Fri, 6 May 2022 12:14:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9C9FC395253C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1651839266; bh=9nyexhQvBKYSsLd8BLST59RIffhc3x25Sa1v/px7kQw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Sc/XIp9TBlgFlmSxzoTIZKvkC+CdwPVzixIfFctq8j3u0/yBy44njox04w4z/jhno 3MYZeBxj5Akf4bG/V+4Ov/4MZd3DIFKdq5ytpmyv4GSNw24fKnH6KdImtD4D878jQ7 arYtXpzzwXxV9jhGXO2p2ZuSrKsXTgokl9bU1nCQ= Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by sourceware.org (Postfix) with ESMTPS id E231D395381F for ; Fri, 6 May 2022 12:12:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E231D395381F X-IronPort-AV: E=McAfee;i="6400,9594,10338"; a="331439263" X-IronPort-AV: E=Sophos;i="5.91,203,1647327600"; d="scan'208";a="331439263" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2022 05:12:54 -0700 X-IronPort-AV: E=Sophos;i="5.91,203,1647327600"; d="scan'208";a="585949699" Received: from mulvlfelix.iul.intel.com (HELO localhost) ([172.28.48.92]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 May 2022 05:12:53 -0700 To: gdb-patches@sourceware.org Subject: [PATCH 3/4] gdb, gdbserver: Allocate only a sane amount of buffer when fetching registers. Date: Fri, 6 May 2022 14:12:25 +0200 Message-Id: <20220506121226.137608-4-felix.willgerodt@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220506121226.137608-1-felix.willgerodt@intel.com> References: <20220506121226.137608-1-felix.willgerodt@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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: , From: Felix Willgerodt via Gdb-patches Reply-To: Felix Willgerodt Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" A couple of functions blindly allocate a buffer of the size of I386_MAX_REGISTER_SIZE. With the addition of AMX, this size has increased drastically from 64 bytes to 8192. This changes these buffer allocations to only use the actual amount needed, similar to how it is already done in amd64-tdep.c (amd64_pseudo_register_read_value). For the i387_collect_xsave and i387_cache_to_xsave functions any feedback is welcome. I opted to take the middle ground and only distinguish between "AMX" and "Not-AMX". That might be unnecessary optimization, we could alternatively be okay with using an 8kB buffer unconditionally or be okay with having many smaller buffer allocations. --- gdb/i386-tdep.c | 21 ++++++++++++++++----- gdb/i387-tdep.c | 19 ++++++++++++++----- gdbserver/i387-fp.cc | 8 +++++++- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 921b24ab60f..94106668e50 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -2944,7 +2944,7 @@ i386_extract_return_value (struct gdbarch *gdbarch, struct type *type, { i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch); int len = TYPE_LENGTH (type); - gdb_byte buf[I386_MAX_REGISTER_SIZE]; + gdb_byte buf[register_size (gdbarch, I386_ST0_REGNUM)]; /* _Float16 and _Float16 _Complex values are returned via xmm0. */ if (((type->code () == TYPE_CODE_FLT) && len == 2) @@ -3006,7 +3006,7 @@ i386_store_return_value (struct gdbarch *gdbarch, struct type *type, if (type->code () == TYPE_CODE_FLT) { ULONGEST fstat; - gdb_byte buf[I386_MAX_REGISTER_SIZE]; + gdb_byte buf[register_size (gdbarch, I386_ST0_REGNUM)]; if (tdep->st0_regnum < 0) { @@ -3591,13 +3591,13 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, int regnum, struct value *result_value) { - gdb_byte raw_buf[I386_MAX_REGISTER_SIZE]; enum register_status status; gdb_byte *buf = value_contents_raw (result_value).data (); if (i386_mmx_regnum_p (gdbarch, regnum)) { int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum); + gdb_byte raw_buf[register_size (gdbarch, regnum)]; /* Extract (always little endian). */ status = regcache->raw_read (fpnum, raw_buf); @@ -3613,6 +3613,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, if (i386_bnd_regnum_p (gdbarch, regnum)) { regnum -= tdep->bnd0_regnum; + gdb_byte raw_buf[register_size (gdbarch, I387_BND0R_REGNUM (tdep))]; /* Extract (always little endian). Read lower 128bits. */ status = regcache->raw_read (I387_BND0R_REGNUM (tdep) + regnum, @@ -3636,6 +3637,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_k_regnum_p (gdbarch, regnum)) { regnum -= tdep->k0_regnum; + gdb_byte raw_buf[register_size (gdbarch, tdep->k0_regnum)]; /* Extract (always little endian). */ status = regcache->raw_read (tdep->k0_regnum + regnum, raw_buf); @@ -3647,6 +3649,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_zmm_regnum_p (gdbarch, regnum)) { regnum -= tdep->zmm0_regnum; + gdb_byte raw_buf[register_size (gdbarch, tdep->zmm0_regnum)]; if (regnum < num_lower_zmm_regs) { @@ -3698,6 +3701,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_ymm_regnum_p (gdbarch, regnum)) { regnum -= tdep->ymm0_regnum; + gdb_byte raw_buf[register_size (gdbarch, tdep->ymm0_regnum)]; /* Extract (always little endian). Read lower 128bits. */ status = regcache->raw_read (I387_XMM0_REGNUM (tdep) + regnum, @@ -3717,6 +3721,8 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_ymm_avx512_regnum_p (gdbarch, regnum)) { regnum -= tdep->ymm16_regnum; + gdb_byte raw_buf[register_size (gdbarch, tdep->ymm0_regnum)]; + /* Extract (always little endian). Read lower 128bits. */ status = regcache->raw_read (I387_XMM16_REGNUM (tdep) + regnum, raw_buf); @@ -3735,6 +3741,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_word_regnum_p (gdbarch, regnum)) { int gpnum = regnum - tdep->ax_regnum; + gdb_byte raw_buf[register_size (gdbarch, gpnum)]; /* Extract (always little endian). */ status = regcache->raw_read (gpnum, raw_buf); @@ -3747,6 +3754,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch, else if (i386_byte_regnum_p (gdbarch, regnum)) { int gpnum = regnum - tdep->al_regnum; + gdb_byte raw_buf[register_size (gdbarch, gpnum % 4)]; /* Extract (always little endian). We read both lower and upper registers. */ @@ -3784,11 +3792,10 @@ void i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, int regnum, const gdb_byte *buf) { - gdb_byte raw_buf[I386_MAX_REGISTER_SIZE]; - if (i386_mmx_regnum_p (gdbarch, regnum)) { int fpnum = i386_mmx_regnum_to_fp_regnum (regcache, regnum); + gdb_byte raw_buf[register_size (gdbarch, regnum)]; /* Read ... */ regcache->raw_read (fpnum, raw_buf); @@ -3813,6 +3820,8 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, upper = extract_unsigned_integer (buf + size, size, byte_order); /* Fetching register buffer. */ + gdb_byte raw_buf[register_size (gdbarch, + I387_BND0R_REGNUM (tdep) + regnum)]; regcache->raw_read (I387_BND0R_REGNUM (tdep) + regnum, raw_buf); @@ -3874,6 +3883,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, else if (i386_word_regnum_p (gdbarch, regnum)) { int gpnum = regnum - tdep->ax_regnum; + gdb_byte raw_buf[register_size (gdbarch, gpnum)]; /* Read ... */ regcache->raw_read (gpnum, raw_buf); @@ -3885,6 +3895,7 @@ i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache, else if (i386_byte_regnum_p (gdbarch, regnum)) { int gpnum = regnum - tdep->al_regnum; + gdb_byte raw_buf[register_size (gdbarch, gpnum)]; /* Read ... We read both lower and upper registers. */ regcache->raw_read (gpnum % 4, raw_buf); diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c index 38ffa3f967b..56239539402 100644 --- a/gdb/i387-tdep.c +++ b/gdb/i387-tdep.c @@ -350,7 +350,7 @@ i387_register_to_value (struct frame_info *frame, int regnum, int *optimizedp, int *unavailablep) { struct gdbarch *gdbarch = get_frame_arch (frame); - gdb_byte from[I386_MAX_REGISTER_SIZE]; + gdb_byte from[register_size (gdbarch, regnum)]; gdb_assert (i386_fp_regnum_p (gdbarch, regnum)); @@ -384,7 +384,7 @@ i387_value_to_register (struct frame_info *frame, int regnum, struct type *type, const gdb_byte *from) { struct gdbarch *gdbarch = get_frame_arch (frame); - gdb_byte to[I386_MAX_REGISTER_SIZE]; + gdb_byte to[register_size (gdbarch, regnum)]; gdb_assert (i386_fp_regnum_p (gdbarch, regnum)); @@ -1419,7 +1419,6 @@ i387_collect_xsave (const struct regcache *regcache, int regnum, enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (gdbarch); gdb_byte *p, *regs = (gdb_byte *) xsave; - gdb_byte raw[I386_MAX_REGISTER_SIZE]; ULONGEST initial_xstate_bv, clear_bv, xstate_bv = 0; unsigned int i; /* See the comment in i387_supply_xsave(). */ @@ -1604,6 +1603,14 @@ i387_collect_xsave (const struct regcache *regcache, int regnum, if (regclass == all) { + /* This used to blindly allocate I386_MAX_REGISTER_SIZE of space. + With AMX that became a bit much to do unconditionally. For now + this seems to be the best trade-off between saving space and + the performance penalty for adding individual allocations. */ + const uint32_t buf_size + = (tdep->xcr0 & X86_XSTATE_TILEDATA) ? I386_MAX_REGISTER_SIZE : 64; + gdb_byte raw[buf_size]; + /* Check if the tilecfg register is changed. */ if ((tdep->xcr0 & X86_XSTATE_TILECFG)) { @@ -1791,6 +1798,7 @@ i387_collect_xsave (const struct regcache *regcache, int regnum, else { /* Check if REGNUM is changed. */ + gdb_byte raw[register_size (gdbarch, regnum)]; regcache->raw_collect (regnum, raw); switch (regclass) @@ -1988,10 +1996,11 @@ i387_collect_xsave (const struct regcache *regcache, int regnum, } else { - int regsize; + int regsize = register_size (gdbarch, i); + gdb_byte raw[regsize]; regcache->raw_collect (i, raw); - regsize = regcache_register_size (regcache, i); + p = FXSAVE_ADDR (tdep, regs, i); if (memcmp (raw, p, regsize)) { diff --git a/gdbserver/i387-fp.cc b/gdbserver/i387-fp.cc index 2d22b0419f8..0b80d287a47 100644 --- a/gdbserver/i387-fp.cc +++ b/gdbserver/i387-fp.cc @@ -265,7 +265,6 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf) unsigned long val, val2; unsigned long long xstate_bv = 0; unsigned long long clear_bv = 0; - char raw[8192]; char *p; /* Amd64 has 16 xmm regs; I386 has 8 xmm regs. */ @@ -348,6 +347,13 @@ i387_cache_to_xsave (struct regcache *regcache, void *buf) } } + /* This used to blindly allocate 64 bytes of space. + With AMX that became a bit much to do unconditionally. For now + this seems to be the best trade-off between saving space and + the performance penalty for adding individual allocations. */ + const uint32_t buf_size = (x86_xcr0 & X86_XSTATE_TILEDATA) ? 8192 : 64; + char raw[buf_size]; + /* Check if any x87 registers are changed. */ if ((x86_xcr0 & X86_XSTATE_X87)) { -- 2.34.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928