From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id BC8823858D28 for ; Wed, 30 Oct 2024 08:36:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BC8823858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BC8823858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730277401; cv=none; b=TmksbBwtbDMnkcomxZEab8zyd422LrF2qNStm4H7C94hFpjgB6X4PgEGjTTqqG6xqQWXwI3R4RKGwh+v8W5W9FYphJ31o8odaR14yjr1W3AnTnYD8gll1zzEi+cLDcP8gPvblbi1+qafL0ye0Wt91jbOhvcKq1zheAnM5XmUEEU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730277401; c=relaxed/simple; bh=U4lGmeXleZtLj1i5iFGZF9eIHaGu7kK/0Fb6zOkZBPc=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: MIME-Version:Date:From:To:Subject:Message-ID; b=NxyMwVwZZh/Xzxe0i2MA4rcQIScJv53PQZOT3R4FDq+WIGkAnhezi4YvrrusWl7938SpniKqrB2AtbSrYz0+IG3K7l45qZS+AKHM711kgLn3BOmw8VcOHLs91l6mxiegoVd6t2NbINqo7mvivep2KVwy8gglkom/Jre+HVij0iI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 8F4CB21E70; Wed, 30 Oct 2024 08:36:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1730277385; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3yA5T64MNPQvoaCQhY65+8CYwhTPeYL7edxi25C7+8E=; b=cz2M4lD0QhSx0J5cJkphpd9zDX3EM+q+E4uAN2Fw+9iPxeYceGOL9YrhNoiMyWBCOPshNv y0BVUy2AzO+J331EFnpHJvWtHYJGU2WUGiPbVGmej45cT+ARAnrQ49HpasKdmxbp9TWtHW s1Lg9u0pzKXfGLKQU4X9Ql0e9P+jYQU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1730277385; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3yA5T64MNPQvoaCQhY65+8CYwhTPeYL7edxi25C7+8E=; b=Xqk9+R+FGe75CKly9btITM7UA2W2Kt2cw6eMxGZNm+Ar3lynMhiRwGuW3jL/TJYFy3N8bs sT+InqErKns/mHDQ== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1730277385; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3yA5T64MNPQvoaCQhY65+8CYwhTPeYL7edxi25C7+8E=; b=cz2M4lD0QhSx0J5cJkphpd9zDX3EM+q+E4uAN2Fw+9iPxeYceGOL9YrhNoiMyWBCOPshNv y0BVUy2AzO+J331EFnpHJvWtHYJGU2WUGiPbVGmej45cT+ARAnrQ49HpasKdmxbp9TWtHW s1Lg9u0pzKXfGLKQU4X9Ql0e9P+jYQU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1730277385; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3yA5T64MNPQvoaCQhY65+8CYwhTPeYL7edxi25C7+8E=; b=Xqk9+R+FGe75CKly9btITM7UA2W2Kt2cw6eMxGZNm+Ar3lynMhiRwGuW3jL/TJYFy3N8bs sT+InqErKns/mHDQ== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 8940F136A5; Wed, 30 Oct 2024 08:36:25 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id x3ZTIQnwIWc4FAAAD6G6ig (envelope-from ); Wed, 30 Oct 2024 08:36:25 +0000 MIME-Version: 1.0 Date: Wed, 30 Oct 2024 08:36:25 +0000 From: tdevries To: Hannes Domani Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] [gdb/tdep] Use std::array in amd64-windows-tdep.c In-Reply-To: <384379824.12394712.1730228908303@mail.yahoo.com> References: <20241029114925.5582-1-tdevries@suse.de> <384379824.12394712.1730228908303@mail.yahoo.com> User-Agent: Roundcube Webmail Message-ID: <79bdea67cd411346b9bd85b1d8f721f8@suse.de> X-Sender: tdevries@suse.de Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Level: X-Spamd-Result: default: False [-4.29 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; XM_UA_NO_VERSION(0.01)[]; FREEMAIL_TO(0.00)[yahoo.de]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCPT_COUNT_TWO(0.00)[2]; FREEMAIL_ENVRCPT(0.00)[yahoo.de]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; RCVD_TLS_ALL(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; FUZZY_BLOCKED(0.00)[rspamd.com]; DBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:helo,suse.de:email,suse.de:mid] X-Spam-Score: -4.29 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2024-10-29 19:08, Hannes Domani wrote: > Am Dienstag, 29. Oktober 2024 um 12:50:12 MEZ hat Tom de Vries > Folgendes geschrieben: > >> I noticed commit 84786372e1c ("Fix size of register buffer") fixing a >> stack-buffer-overflow found by AddressSanitizer in >> amd64_windows_store_arg_in_reg: >> ... >> -  gdb_byte buf[8]; >> +  gdb_byte buf[16]; >> ... >> and wondered if we could have found this without AddressSanitizer. >> >> I realized that the problem is that this: >> ... >>    gdb_byte buf[N]; >>    ... >>    regcache->cooked_write (regno, buf); >> ... >> is using the deprecated variant of cooked_write instead of the one >> using >> gdb::array_view: >> ... >>    /* Transfer of pseudo-registers.  */ >>    void cooked_write (int regnum, gdb::array_view >> src); >> >>    /* Deprecated overload of the above.  */ >>    void cooked_write (int regnum, const gdb_byte *src); >> ... >> and consequently cooked_write does not know the size of buf. >> >> Fix this by using std::array, and likewise in other places in >> gdb/amd64-windows-tdep.c. >> >> In the process I fixed another out of bounds access here: >> ... >>      gdb_byte imm16[2]; >>    ... >>      cache->prev_sp = cur_sp >>        + extract_unsigned_integer (imm16, 4, byte_order); >> ... >> where we're reading 4 bytes from the 2-byte buffer imm16. >> >> Tested by rebuilding on x86_64-linux. >> --- >> gdb/amd64-windows-tdep.c | 46 ++++++++++++++++++++-------------------- >> 1 file changed, 23 insertions(+), 23 deletions(-) >> >> diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c >> index 57dcc4f56bd..b20c587c2f9 100644 >> --- a/gdb/amd64-windows-tdep.c >> +++ b/gdb/amd64-windows-tdep.c >> @@ -208,11 +208,11 @@ amd64_windows_store_arg_in_reg (struct regcache >> *regcache, >>    struct type *type = arg->type (); >>    const gdb_byte *valbuf = arg->contents ().data (); >>    /* We only set 8 bytes, buf if it's a XMM register, 16 bytes are >> read.  */ >> -  gdb_byte buf[16]; >> +  std::array buf; >> >>    gdb_assert (type->length () <= 8); >> -  memset (buf, 0, sizeof buf); >> -  memcpy (buf, valbuf, std::min (type->length (), (ULONGEST) 8)); >> +  memset (buf.data (), 0, buf.size ()); >> +  memcpy (buf.data (), valbuf, std::min (type->length (), (ULONGEST) >> 8)); >>    regcache->cooked_write (regno, buf); >> } >> > > The rest of the patch seems fine, but with this hunk applied, I get the > following result when trying to call any function with arguments: > > (gdb) p (int)abs(-5) > C:/src/repos/binutils-gdb.git/gdb/regcache.c:864: internal-error: > raw_write: Assertion `src.size () == m_descr->sizeof_register[regnum]' > failed. > A problem internal to GDB has been detected, > > > (top-gdb) bt > #0  internal_error_loc (file=file@entry=0x140e592d8 > "C:/src/repos/binutils-gdb.git/gdb/regcache.c", > line=line@entry=864, fmt=fmt@entry=0x140e5923e > "%s: Assertion `%s' failed.") at > C:/src/repos/binutils-gdb.git/gdbsupport/errors.cc:53 > #1  0x000000013feaf1ea in regcache::raw_write (this=0x4926c20, > regnum=, src=array_view with 16 elements = {...}) at > C:/src/repos/binutils-gdb.git/gdb/regcache.c:864 > #2  0x000000013feb0869 in regcache::cooked_write (this=0x4926c20, > regnum=2, src=array_view with 16 elements = {...}) at > C:/src/repos/binutils-gdb.git/gdb/regcache.c:914 > #3  0x000000013fb2ff0d in amd64_windows_store_arg_in_reg > (regcache=0x4926c20, arg=, regno=2) at > C:/src/repos/binutils-gdb.git/gdb/amd64-windows-tdep.c:217 > #4  0x000000013fb30a12 in amd64_windows_push_arguments > (regcache=regcache@entry=0x4926c20, nargs=nargs@entry=1, > args=0x3ced30, args@entry=0x4c48a70, sp=sp@entry=0x3cf8a0, > return_method=return_method@entry=return_method_normal) at > C:/src/repos/binutils-gdb.git/gdb/amd64-windows-tdep.c:279 > #5  0x000000013fb30d97 in amd64_windows_push_dummy_call > (gdbarch=, function=, > regcache=0x4926c20, bp_addr=0x3cf8af, nargs=1, args=0x4c48a70, > sp=0x3cf8a0, return_method=return_method_normal, struct_addr=0x0) at > C:/src/repos/binutils-gdb.git/gdb/amd64-windows-tdep.c:324 > #6  0x000000013fd2f0a9 in call_function_by_hand_dummy > (function=0x59e14d0, > default_return_type=default_return_type@entry=0x4417670, args=..., > dummy_dtor=dummy_dtor@entry=0x0, > dummy_dtor_data=dummy_dtor_data@entry=0x0) at > C:/src/repos/binutils-gdb.git/gdb/infcall.c:1463 > #7  0x000000013fd31abb in call_function_by_hand (function= out>, default_return_type=default_return_type@entry=0x4417670, > args=...) at C:/src/repos/binutils-gdb.git/gdb/infcall.c:1020 > #8  0x000000013fcb556b in evaluate_subexp_do_call > (exp=exp@entry=0x59cb480, noside=noside@entry=EVAL_NORMAL, > callee=, callee@entry=0x59e14d0, argvec=array_view with > 1 elements = {...}, function_name=function_name@entry=0x52d2840 "abs", > default_return_type=default_return_type@entry=0x4417670) at > C:/src/repos/binutils-gdb.git/gdb/eval.c:673 > #9  0x000000013fcb5a24 in expr::operation::evaluate_funcall > (this=this@entry=0x435faa0, expect_type=0x4417670, exp=0x59cb480, > noside=EVAL_NORMAL, function_name=0x52d2840 "abs", args=std::vector of > length 1, capacity 1 = {...}) at > C:/src/repos/binutils-gdb.git/gdb/eval.c:705 > #10 0x000000014095acc2 in > expr::var_msym_value_operation::evaluate_funcall (this=0x435faa0, > expect_type=, exp=, noside= out>, args=std::vector of length 1, capacity 1 = {...}) at > C:/src/repos/binutils-gdb.git/gdb/expop.h:746 > #11 0x00000001409563cb in expr::funcall_operation::evaluate > (this=, expect_type=, exp= out>, noside=) at > C:/src/repos/binutils-gdb.git/gdb/expop.h:2207 > #12 0x000000013fcb093e in expr::operation::evaluate_for_cast > (this=, expect_type=0x4417670, exp=, > noside=) at > C:/src/repos/binutils-gdb.git/gdb/eval.c:2626 > #13 0x000000013fcb1892 in expression::evaluate (this=0x59cb480, > expect_type=expect_type@entry=0x0, noside=noside@entry=EVAL_NORMAL) at > C:/src/repos/binutils-gdb.git/gdb/eval.c:110 > #14 0x000000013fe48946 in process_print_command_args (args= out>, print_opts=print_opts@entry=0x3cf7d0, > voidprint=voidprint@entry=true) at > C:/src/repos/binutils-gdb.git/gdb/printcmd.c:1327 > #15 0x000000013fe49211 in print_command_1 (args=, > voidprint=1) at C:/src/repos/binutils-gdb.git/gdb/printcmd.c:1340 > #16 0x000000013fbe46a6 in cmd_func (cmd=, > args=, from_tty=) at > C:/src/repos/binutils-gdb.git/gdb/cli/cli-decode.c:2741 > #17 0x000000013ffefaa5 in execute_command (p=, > p@entry=0x59cb220 "p (int)abs(-5)", from_tty=1) at > C:/src/repos/binutils-gdb.git/gdb/top.c:581 > #18 0x000000013fcb7c9f in command_handler (command=0x59cb220 "p > (int)abs(-5)") at C:/src/repos/binutils-gdb.git/gdb/event-top.c:579 > #19 0x000000013fcb9625 in command_line_handler (rl=...) at > C:/src/repos/binutils-gdb.git/gdb/event-top.c:815 > #20 0x000000013fcb86d1 in gdb_rl_callback_handler (rl=0x59cb3c0 "p > (int)abs(-5)") at C:/src/repos/binutils-gdb.git/gdb/event-top.c:271 > #21 0x000000014008af0f in rl_callback_read_char () at > C:/src/repos/binutils-gdb.git/readline/readline/callback.c:290 > #22 0x000000013fcb885e in gdb_rl_callback_read_char_wrapper_noexcept > () at C:/src/repos/binutils-gdb.git/gdb/event-top.c:196 > #23 0x000000013fcb8982 in gdb_rl_callback_read_char_wrapper > (client_data=) at > C:/src/repos/binutils-gdb.git/gdb/event-top.c:235 > #24 0x00000001400250ac in stdin_event_handler (error=, > client_data=0x4653b0) at C:/src/repos/binutils-gdb.git/gdb/ui.c:154 > #25 0x0000000140569045 in handle_file_event (file_ptr=, > ready_mask=) at > C:/src/repos/binutils-gdb.git/gdbsupport/event-loop.cc:551 > #26 gdb_wait_for_event (block=block@entry=1) at > C:/src/repos/binutils-gdb.git/gdbsupport/event-loop.cc:694 > #27 0x00000001405699d4 in gdb_wait_for_event (block=1) at > C:/src/repos/binutils-gdb.git/gdbsupport/event-loop.cc:571 > #28 gdb_do_one_event (mstimeout=mstimeout@entry=-1) at > C:/src/repos/binutils-gdb.git/gdbsupport/event-loop.cc:263 > #29 0x000000013fda02ea in start_event_loop () at > C:/src/repos/binutils-gdb.git/gdb/main.c:404 > #30 captured_command_loop () at > C:/src/repos/binutils-gdb.git/gdb/main.c:468 > #31 0x000000013fda4b35 in captured_main (data=0x3cfdd0) at > C:/src/repos/binutils-gdb.git/gdb/main.c:1361 > #32 gdb_main (args=args@entry=0x3cfe40) at > C:/src/repos/binutils-gdb.git/gdb/main.c:1380 > #33 0x0000000140a7c530 in main (argc=3, argv=0x437f20) at > C:/src/repos/binutils-gdb.git/gdb/gdb.c:38 > > > It's because raw_write expects an array_view with the exact size of > the register: > > regcache::raw_write (int regnum, gdb::array_view src) > { >   assert_regnum (regnum); >   gdb_assert (src.size () == m_descr->sizeof_register[regnum]); > > Hi Hannes, thanks for testing. I've submitted a v2 that hopefully should take of this. Thanks, - Tom > Hannes