From patchwork Thu Jan 11 05:51:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Frysinger X-Patchwork-Id: 83828 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 031BA3858D38 for ; Thu, 11 Jan 2024 05:52:30 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.gentoo.org (woodpecker.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) by sourceware.org (Postfix) with ESMTP id 60650385802D for ; Thu, 11 Jan 2024 05:51:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 60650385802D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gentoo.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gentoo.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 60650385802D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:470:ea4a:1:5054:ff:fec7:86e4 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704952275; cv=none; b=cLLFJzbav6QTVt5kMOmFuLCDFbKrpmTkqiAK2dVM9u4cs4Y/Pl/+c5lxySp7zuZEcTo7lkCdGyoBlOa1XJvk8Er8cEZydpApphHGjv2C2kGreqUyE5z4U3weal0FZ5GqzhZVKbBJee7KZ0cV1xaQJ1s9RU30ZHd4NixJFrkNFYk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704952275; c=relaxed/simple; bh=JmTxZ20Z5G0bc08zGclMFpbLBCry2icGJ3IqEU+zOWg=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=S7mNNaDkD0abfBdPFKUEDuLv+ksrHrydhu/2jpfx5gy/pAQ7BTmcYsxUJaMoMMXgVQpWsOkyJVi7lr8GSKITK2nhD/heaJy965A+RPAACWc067QQVCq+Y1di2hs4OR/gnuCMV9/mdX5LuR4T5Z4/w2WfFT2I15EBEapNV6GLwz8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by smtp.gentoo.org (Postfix, from userid 559) id E906A34310F; Thu, 11 Jan 2024 05:51:12 +0000 (UTC) From: Mike Frysinger To: gdb-patches@sourceware.org Subject: [PATCH] sim: ppc: switch register read/writes to union to avoid aliasing issues Date: Thu, 11 Jan 2024 00:51:11 -0500 Message-ID: <20240111055111.29200-1-vapier@gentoo.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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+patchwork=sourceware.org@sourceware.org This code creates a small buffer on the stack w/alloca, then proceeds to write to it with a cast to a pointer type based on the register type, then reads from it with a cast to a pointer type based on the register size. gcc will flags only one of these lines as "maybe used uninitialized", but it seems to track back to this memory abuse. Create a large union with all the possible types that this code will read or write as, and then use those. It's a bit ugly, but is probably better than using raw memcpy's everywhere. --- sim/ppc/psim.c | 125 +++++++++++++++++++++++++++++-------------------- 1 file changed, 75 insertions(+), 50 deletions(-) diff --git a/sim/ppc/psim.c b/sim/ppc/psim.c index 645e29a636d6..2c900e64100a 100644 --- a/sim/ppc/psim.c +++ b/sim/ppc/psim.c @@ -789,7 +789,21 @@ psim_read_register(psim *system, transfer_mode mode) { register_descriptions description; - char *cooked_buf; + union { + uint8_t bytes[16]; + unsigned_word unsigned_word; + unsigned_1 unsigned_1; + unsigned_2 unsigned_2; + unsigned_4 unsigned_4; + unsigned_8 unsigned_8; + creg creg; + fpreg fpreg; + fpscreg fpscreg; + gpreg gpreg; + msreg msreg; + spreg spreg; + sreg sreg; + } cooked_buf; cpu *processor; /* find our processor */ @@ -808,81 +822,80 @@ psim_read_register(psim *system, description = register_description(reg); if (description.type == reg_invalid) return 0; - cooked_buf = alloca (description.size); /* get the cooked value */ switch (description.type) { case reg_gpr: - *(gpreg*)cooked_buf = cpu_registers(processor)->gpr[description.index]; + cooked_buf.gpreg = cpu_registers(processor)->gpr[description.index]; break; case reg_spr: - *(spreg*)cooked_buf = cpu_registers(processor)->spr[description.index]; + cooked_buf.spreg = cpu_registers(processor)->spr[description.index]; break; case reg_sr: - *(sreg*)cooked_buf = cpu_registers(processor)->sr[description.index]; + cooked_buf.sreg = cpu_registers(processor)->sr[description.index]; break; case reg_fpr: - *(fpreg*)cooked_buf = cpu_registers(processor)->fpr[description.index]; + cooked_buf.fpreg = cpu_registers(processor)->fpr[description.index]; break; case reg_pc: - *(unsigned_word*)cooked_buf = cpu_get_program_counter(processor); + cooked_buf.unsigned_word = cpu_get_program_counter(processor); break; case reg_cr: - *(creg*)cooked_buf = cpu_registers(processor)->cr; + cooked_buf.creg = cpu_registers(processor)->cr; break; case reg_msr: - *(msreg*)cooked_buf = cpu_registers(processor)->msr; + cooked_buf.msreg = cpu_registers(processor)->msr; break; case reg_fpscr: - *(fpscreg*)cooked_buf = cpu_registers(processor)->fpscr; + cooked_buf.fpscreg = cpu_registers(processor)->fpscr; break; case reg_insns: - *(unsigned_word*)cooked_buf = mon_get_number_of_insns(system->monitor, + cooked_buf.unsigned_word = mon_get_number_of_insns(system->monitor, which_cpu); break; case reg_stalls: if (cpu_model(processor) == NULL) error("$stalls only valid if processor unit model enabled (-I)\n"); - *(unsigned_word*)cooked_buf = model_get_number_of_stalls(cpu_model(processor)); + cooked_buf.unsigned_word = model_get_number_of_stalls(cpu_model(processor)); break; case reg_cycles: if (cpu_model(processor) == NULL) error("$cycles only valid if processor unit model enabled (-I)\n"); - *(unsigned_word*)cooked_buf = model_get_number_of_cycles(cpu_model(processor)); + cooked_buf.unsigned_word = model_get_number_of_cycles(cpu_model(processor)); break; #ifdef WITH_ALTIVEC case reg_vr: - *(vreg*)cooked_buf = cpu_registers(processor)->altivec.vr[description.index]; + cooked_buf.vreg = cpu_registers(processor)->altivec.vr[description.index]; break; case reg_vscr: - *(vscreg*)cooked_buf = cpu_registers(processor)->altivec.vscr; + cooked_buf.vscreg = cpu_registers(processor)->altivec.vscr; break; #endif #ifdef WITH_E500 case reg_gprh: - *(gpreg*)cooked_buf = cpu_registers(processor)->e500.gprh[description.index]; + cooked_buf.gpreg = cpu_registers(processor)->e500.gprh[description.index]; break; case reg_evr: - *(uint64_t*)cooked_buf = EVR(description.index); + cooked_buf.uint64_t = EVR(description.index); break; case reg_acc: - *(accreg*)cooked_buf = cpu_registers(processor)->e500.acc; + cooked_buf.accreg = cpu_registers(processor)->e500.acc; break; #endif @@ -898,36 +911,36 @@ psim_read_register(psim *system, /* FIXME - assumes that all registers are simple integers */ switch (description.size) { case 1: - *(unsigned_1*)buf = H2T_1(*(unsigned_1*)cooked_buf); + *(unsigned_1*)buf = H2T_1(cooked_buf.unsigned_1); break; case 2: - *(unsigned_2*)buf = H2T_2(*(unsigned_2*)cooked_buf); + *(unsigned_2*)buf = H2T_2(cooked_buf.unsigned_2); break; case 4: - *(unsigned_4*)buf = H2T_4(*(unsigned_4*)cooked_buf); + *(unsigned_4*)buf = H2T_4(cooked_buf.unsigned_4); break; case 8: - *(unsigned_8*)buf = H2T_8(*(unsigned_8*)cooked_buf); + *(unsigned_8*)buf = H2T_8(cooked_buf.unsigned_8); break; #ifdef WITH_ALTIVEC case 16: if (HOST_BYTE_ORDER != CURRENT_TARGET_BYTE_ORDER) { union { vreg v; unsigned_8 d[2]; } h, t; - memcpy(&h.v/*dest*/, cooked_buf/*src*/, description.size); + memcpy(&h.v/*dest*/, cooked_buf.bytes/*src*/, description.size); { _SWAP_8(t.d[0] =, h.d[1]); } { _SWAP_8(t.d[1] =, h.d[0]); } memcpy(buf/*dest*/, &t/*src*/, description.size); break; } else - memcpy(buf/*dest*/, cooked_buf/*src*/, description.size); + memcpy(buf/*dest*/, cooked_buf.bytes/*src*/, description.size); break; #endif } } else { - memcpy(buf/*dest*/, cooked_buf/*src*/, description.size); + memcpy(buf/*dest*/, cooked_buf.bytes/*src*/, description.size); } return description.size; @@ -945,7 +958,21 @@ psim_write_register(psim *system, { cpu *processor; register_descriptions description; - char *cooked_buf; + union { + uint8_t bytes[16]; + unsigned_word unsigned_word; + unsigned_1 unsigned_1; + unsigned_2 unsigned_2; + unsigned_4 unsigned_4; + unsigned_8 unsigned_8; + creg creg; + fpreg fpreg; + fpscreg fpscreg; + gpreg gpreg; + msreg msreg; + spreg spreg; + sreg sreg; + } cooked_buf; /* find our processor */ if (which_cpu == MAX_NR_PROCESSORS) { @@ -960,7 +987,6 @@ psim_write_register(psim *system, description = register_description(reg); if (description.type == reg_invalid) return 0; - cooked_buf = alloca (description.size); if (which_cpu == -1) { int i; @@ -977,16 +1003,16 @@ psim_write_register(psim *system, if (mode == raw_transfer) { switch (description.size) { case 1: - *(unsigned_1*)cooked_buf = T2H_1(*(unsigned_1*)buf); + cooked_buf.unsigned_1 = T2H_1(*(unsigned_1*)buf); break; case 2: - *(unsigned_2*)cooked_buf = T2H_2(*(unsigned_2*)buf); + cooked_buf.unsigned_2 = T2H_2(*(unsigned_2*)buf); break; case 4: - *(unsigned_4*)cooked_buf = T2H_4(*(unsigned_4*)buf); + cooked_buf.unsigned_4 = T2H_4(*(unsigned_4*)buf); break; case 8: - *(unsigned_8*)cooked_buf = T2H_8(*(unsigned_8*)buf); + cooked_buf.unsigned_8 = T2H_8(*(unsigned_8*)buf); break; #ifdef WITH_ALTIVEC case 16: @@ -996,86 +1022,85 @@ psim_write_register(psim *system, memcpy(&t.v/*dest*/, buf/*src*/, description.size); { _SWAP_8(h.d[0] =, t.d[1]); } { _SWAP_8(h.d[1] =, t.d[0]); } - memcpy(cooked_buf/*dest*/, &h/*src*/, description.size); + memcpy(cooked_buf.bytes/*dest*/, &h/*src*/, description.size); break; } else - memcpy(cooked_buf/*dest*/, buf/*src*/, description.size); + memcpy(cooked_buf.bytes/*dest*/, buf/*src*/, description.size); #endif } } else { - memcpy(cooked_buf/*dest*/, buf/*src*/, description.size); + memcpy(cooked_buf.bytes/*dest*/, buf/*src*/, description.size); } /* put the cooked value into the register */ switch (description.type) { case reg_gpr: - cpu_registers(processor)->gpr[description.index] = *(gpreg*)cooked_buf; + cpu_registers(processor)->gpr[description.index] = cooked_buf.gpreg; break; case reg_fpr: - cpu_registers(processor)->fpr[description.index] = *(fpreg*)cooked_buf; + cpu_registers(processor)->fpr[description.index] = cooked_buf.fpreg; break; case reg_pc: - cpu_set_program_counter(processor, *(unsigned_word*)cooked_buf); + cpu_set_program_counter(processor, cooked_buf.unsigned_word); break; case reg_spr: - cpu_registers(processor)->spr[description.index] = *(spreg*)cooked_buf; + cpu_registers(processor)->spr[description.index] = cooked_buf.spreg; break; case reg_sr: - cpu_registers(processor)->sr[description.index] = *(sreg*)cooked_buf; + cpu_registers(processor)->sr[description.index] = cooked_buf.sreg; break; case reg_cr: - cpu_registers(processor)->cr = *(creg*)cooked_buf; + cpu_registers(processor)->cr = cooked_buf.creg; break; case reg_msr: - cpu_registers(processor)->msr = *(msreg*)cooked_buf; + cpu_registers(processor)->msr = cooked_buf.msreg; break; case reg_fpscr: - cpu_registers(processor)->fpscr = *(fpscreg*)cooked_buf; + cpu_registers(processor)->fpscr = cooked_buf.fpscreg; break; #ifdef WITH_E500 case reg_gprh: - cpu_registers(processor)->e500.gprh[description.index] = *(gpreg*)cooked_buf; + cpu_registers(processor)->e500.gprh[description.index] = cooked_buf.gpreg; break; case reg_evr: { uint64_t v; - v = *(uint64_t*)cooked_buf; + v = cooked_buf.uint64_t; cpu_registers(processor)->e500.gprh[description.index] = v >> 32; cpu_registers(processor)->gpr[description.index] = v; break; } case reg_acc: - cpu_registers(processor)->e500.acc = *(accreg*)cooked_buf; + cpu_registers(processor)->e500.acc = cooked_buf.accreg; break; #endif #ifdef WITH_ALTIVEC case reg_vr: - cpu_registers(processor)->altivec.vr[description.index] = *(vreg*)cooked_buf; + cpu_registers(processor)->altivec.vr[description.index] = cooked_buf.vreg; break; case reg_vscr: - cpu_registers(processor)->altivec.vscr = *(vscreg*)cooked_buf; + cpu_registers(processor)->altivec.vscr = cooked_buf.vscreg; break; #endif default: - printf_filtered("psim_write_register(processor=%p,cooked_buf=%p,reg=%s) %s\n", - processor, cooked_buf, reg, - "read of this register unimplemented"); + printf_filtered("psim_write_register(processor=%p,buf=%p,reg=%s) %s\n", + processor, buf, reg, "read of this register unimplemented"); return 0; }