From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 8A9943858C60 for ; Thu, 2 Nov 2023 12:27:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8A9943858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8A9943858C60 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698928064; cv=none; b=sxFgOFpZBvn+BWeZfU8DfZjs3Je5nOeofW3IrMHQK4Q/CdOlhTUBurb45p79yw2jZW7NhHUCAYkvmE+27tZNF8+LISZIHWELydi8lOmCh3oLZo94inSJzxkTA161sZFdifT5Wb9T2gIHZoj2WCFWS7yZ5avjFBFQ+WzaEqkUPco= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698928064; c=relaxed/simple; bh=2OQ5krROeWNcePIbRsmFREPKGGkJ6RgZ3b2diqS1NhU=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=hE94C62zIDpesYooBE+v18Y7vXVF20FsvjYSmEg/f3af3x/21EzRuhFBuuxl8gMr0WqUvYwZ/Icr3E3uordm6VofnO4MKJuS89EI9nXVS6YUau8AY/S6x5HV4MV7Fyw+0JpI/Y+EUGvCd+q9OTq9KOtpYSO2m68JsEjUQahFVg4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1698928061; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GxDkBC5kd0E15dD7PasKCQP4EmQms/UvC7Iys7uvdx4=; b=HEudHoFpNVDKG3G9fPxL33sPkP0Y6w7mYexQrpaGoGrQZlBC5YIJwNMWmgZH+Pz4PmHFrT T1SWx3lrN3TqbG40sTLC8hYgQ8Y9iPok30FIZKxONqmhzVfSthdysCQKc5ceejXJw+dbE+ SZfX5zLxHLqHHrShqu5eQl4XsbKZIBw= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-616-OPbP22lIMamGlRAmI_RWwQ-1; Thu, 02 Nov 2023 08:27:40 -0400 X-MC-Unique: OPbP22lIMamGlRAmI_RWwQ-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-9bea60bd1adso222800366b.0 for ; Thu, 02 Nov 2023 05:27:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698928059; x=1699532859; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=GxDkBC5kd0E15dD7PasKCQP4EmQms/UvC7Iys7uvdx4=; b=tO5/Ckfr52UhDW6G8YPO9qTJd2bW02G3NFWSOXk38jUHFZgp6Zwxj5mEj5Roy/1Y1p Rpd3yYO1i+HL4bIvi2mFnMFd1dxvJgm2N5yJs0nkL7gt6uTVSVoP09ORp/D4Jlq4rnHL ErNx66QqZl7Aho5SMuSi1TyHBnXrIm0+Gl1+l1FKo9fXdPvUDkT/oEreFuXUE8vkpmLo Tnxkcd149A7+PhvdqfMdzKE1MIHG3GQvUxSVHjt5WH3cKe9qD2DAVTdh51GRUsUKCllg OpzhWca0hPuDjwX4vQmkZt1kxGOVhEGws7rhD6XwIqk+9d84NJdgY3RSnsiFB/RNNV92 Op4g== X-Gm-Message-State: AOJu0Yyr8mCx4vXQWaeGAZYzRtzPMCzouBq1HkJGFj2KsfhZ7+ccH9dI UUCI7lOaP4zXdP00VMXQp5mDqA1r14gAb5hpTfndTD78cT+OApnOs3GRhHDtsYYn1PWNoZjh2Lc z3hJvNLxyJ2l2eB3DoLTE9B9dmw== X-Received: by 2002:a17:906:3c8:b0:9a1:aaae:8207 with SMTP id c8-20020a17090603c800b009a1aaae8207mr4301627eja.20.1698928059255; Thu, 02 Nov 2023 05:27:39 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGQcuXln6AurpdPtG3Hpm5u5oBdhHks5vu+YBiYfCO8nRDHzDKVcPCQP3692+NtfBycv9EdJQ== X-Received: by 2002:a17:906:3c8:b0:9a1:aaae:8207 with SMTP id c8-20020a17090603c800b009a1aaae8207mr4301608eja.20.1698928058830; Thu, 02 Nov 2023 05:27:38 -0700 (PDT) Received: from localhost (105.226.159.143.dyn.plus.net. [143.159.226.105]) by smtp.gmail.com with ESMTPSA id v9-20020a1709060b4900b009cc1e8ed7c5sm1084532ejg.133.2023.11.02.05.27.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Nov 2023 05:27:38 -0700 (PDT) From: Andrew Burgess To: Yang Liu , binutils@sourceware.org Cc: palmer@dabbelt.com, simon.marchi@polymtl.ca, Yang Liu Subject: Re: [PATCH] gdb: RISC-V: Refine lr/sc sequence support In-Reply-To: <20231031173922.4836-1-liuyang22@iscas.ac.cn> References: <20231031173922.4836-1-liuyang22@iscas.ac.cn> Date: Thu, 02 Nov 2023 12:27:36 +0000 Message-ID: <87lebguypz.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 List-Id: Yang Liu writes: > Per RISC-V spec, the lr/sc sequence can consist of up to 16 instructions, and we > cannot insert breakpoints in the middle of this sequence. Before this, we only > detected a specific pattern (the most common one). This patch improves this part > and now supports more complex pattern detection. Thanks for working on this. GDB patches are handled on: gdb-patches@sourceware.org could you re-post to that list please. Thanks, Andrew > > gdb/ChangeLog: > > * gdb/riscv-tdep.c (class riscv_insn): Add more needed opcode enums. > (riscv_insn::decode): Decode newly added opcodes. > (riscv_insn_is_non_cti_and_allowed_in_atomic_sequence): New. > (riscv_insn_is_direct_branch): New. > (riscv_next_pc_atomic_sequence): Removed. > (riscv_deal_with_atomic_sequence): Rename from riscv_next_pc_atomic_sequence. > (riscv_software_single_step): Adjust to use the renamed one. > > Signed-off-by: Yang Liu > --- > gdb/riscv-tdep.c | 266 ++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 226 insertions(+), 40 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 3a2891c2c92..faff1aab420 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -1578,8 +1578,34 @@ class riscv_insn > BLTU, > BGEU, > /* These are needed for stepping over atomic sequences. */ > - LR, > - SC, > + SLTI, > + SLTIU, > + XORI, > + ORI, > + ANDI, > + SLLI, > + SLLIW, > + SRLI, > + SRLIW, > + SRAI, > + SRAIW, > + SUB, > + SUBW, > + SLL, > + SLLW, > + SLT, > + SLTU, > + XOR, > + SRL, > + SRLW, > + SRA, > + SRAW, > + OR, > + AND, > + LR_W, > + LR_D, > + SC_W, > + SC_D, > /* This instruction is used to do a syscall. */ > ECALL, > > @@ -1768,6 +1794,13 @@ class riscv_insn > m_imm.s = EXTRACT_CBTYPE_IMM (ival); > } > > + void decode_ca_type_insn (enum opcode opcode, ULONGEST ival) > + { > + m_opcode = opcode; > + m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S); > + m_rs2 = decode_register_index_short (ival, OP_SH_CRS2S); > + } > + > /* Fetch instruction from target memory at ADDR, return the content of > the instruction, and update LEN with the instruction length. */ > static ULONGEST fetch_instruction (struct gdbarch *gdbarch, > @@ -1882,14 +1915,62 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc) > decode_b_type_insn (BLTU, ival); > else if (is_bgeu_insn (ival)) > decode_b_type_insn (BGEU, ival); > + else if (is_slti_insn(ival)) > + decode_i_type_insn (SLTI, ival); > + else if (is_sltiu_insn(ival)) > + decode_i_type_insn (SLTIU, ival); > + else if (is_xori_insn(ival)) > + decode_i_type_insn (XORI, ival); > + else if (is_ori_insn(ival)) > + decode_i_type_insn (ORI, ival); > + else if (is_andi_insn(ival)) > + decode_i_type_insn (ANDI, ival); > + else if (is_slli_insn(ival)) > + decode_i_type_insn (SLLI, ival); > + else if (is_slliw_insn(ival)) > + decode_i_type_insn (SLLIW, ival); > + else if (is_srli_insn(ival)) > + decode_i_type_insn (SRLI, ival); > + else if (is_srliw_insn(ival)) > + decode_i_type_insn (SRLIW, ival); > + else if (is_srai_insn(ival)) > + decode_i_type_insn (SRAI, ival); > + else if (is_sraiw_insn(ival)) > + decode_i_type_insn (SRAIW, ival); > + else if (is_sub_insn(ival)) > + decode_r_type_insn (SUB, ival); > + else if (is_subw_insn(ival)) > + decode_r_type_insn (SUBW, ival); > + else if (is_sll_insn(ival)) > + decode_r_type_insn (SLL, ival); > + else if (is_sllw_insn(ival)) > + decode_r_type_insn (SLLW, ival); > + else if (is_slt_insn(ival)) > + decode_r_type_insn (SLT, ival); > + else if (is_sltu_insn(ival)) > + decode_r_type_insn (SLTU, ival); > + else if (is_xor_insn(ival)) > + decode_r_type_insn (XOR, ival); > + else if (is_srl_insn(ival)) > + decode_r_type_insn (SRL, ival); > + else if (is_srlw_insn(ival)) > + decode_r_type_insn (SRLW, ival); > + else if (is_sra_insn(ival)) > + decode_r_type_insn (SRA, ival); > + else if (is_sraw_insn(ival)) > + decode_r_type_insn (SRAW, ival); > + else if (is_or_insn(ival)) > + decode_r_type_insn (OR, ival); > + else if (is_and_insn(ival)) > + decode_r_type_insn (AND, ival); > else if (is_lr_w_insn (ival)) > - decode_r_type_insn (LR, ival); > + decode_r_type_insn (LR_W, ival); > else if (is_lr_d_insn (ival)) > - decode_r_type_insn (LR, ival); > + decode_r_type_insn (LR_D, ival); > else if (is_sc_w_insn (ival)) > - decode_r_type_insn (SC, ival); > + decode_r_type_insn (SC_W, ival); > else if (is_sc_d_insn (ival)) > - decode_r_type_insn (SC, ival); > + decode_r_type_insn (SC_D, ival); > else if (is_ecall_insn (ival)) > decode_i_type_insn (ECALL, ival); > else if (is_ld_insn (ival)) > @@ -1944,6 +2025,24 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc) > m_rd = decode_register_index (ival, OP_SH_CRS1S); > m_imm.s = EXTRACT_CITYPE_LUI_IMM (ival); > } > + else if (is_c_srli_insn (ival)) > + decode_cb_type_insn (SRLI, ival); > + else if (is_c_srai_insn (ival)) > + decode_cb_type_insn (SRAI, ival); > + else if (is_c_andi_insn (ival)) > + decode_cb_type_insn (ANDI, ival); > + else if (is_c_sub_insn (ival)) > + decode_ca_type_insn (SUB, ival); > + else if (is_c_xor_insn (ival)) > + decode_ca_type_insn (XOR, ival); > + else if (is_c_or_insn (ival)) > + decode_ca_type_insn (OR, ival); > + else if (is_c_and_insn (ival)) > + decode_ca_type_insn (AND, ival); > + else if (is_c_subw_insn (ival)) > + decode_ca_type_insn (SUBW, ival); > + else if (is_c_addw_insn (ival)) > + decode_ca_type_insn (ADDW, ival); > else if (is_c_li_insn (ival)) > decode_ci_type_insn (LI, ival); > /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only, > @@ -4404,51 +4503,138 @@ riscv_next_pc (struct regcache *regcache, CORE_ADDR pc) > return next_pc; > } > > +/* Return true if INSN is not a control transfer instruction and is allowed to > + appear in the middle of the lr/sc sequence. */ > + > +static bool > +riscv_insn_is_non_cti_and_allowed_in_atomic_sequence(const struct riscv_insn &insn) > +{ > + switch (insn.opcode ()) > + { > + case riscv_insn::LUI: > + case riscv_insn::AUIPC: > + case riscv_insn::ADDI: > + case riscv_insn::ADDIW: > + case riscv_insn::SLTI: > + case riscv_insn::SLTIU: > + case riscv_insn::XORI: > + case riscv_insn::ORI: > + case riscv_insn::ANDI: > + case riscv_insn::SLLI: > + case riscv_insn::SLLIW: > + case riscv_insn::SRLI: > + case riscv_insn::SRLIW: > + case riscv_insn::SRAI: > + case riscv_insn::ADD: > + case riscv_insn::ADDW: > + case riscv_insn::SRAIW: > + case riscv_insn::SUB: > + case riscv_insn::SUBW: > + case riscv_insn::SLL: > + case riscv_insn::SLLW: > + case riscv_insn::SLT: > + case riscv_insn::SLTU: > + case riscv_insn::XOR: > + case riscv_insn::SRL: > + case riscv_insn::SRLW: > + case riscv_insn::SRA: > + case riscv_insn::SRAW: > + case riscv_insn::OR: > + case riscv_insn::AND: > + return true; > + } > + > + return false; > +} > + > +/* Return true if INSN is a direct branch insctruction. */ > + > +static bool > +riscv_insn_is_direct_branch(const struct riscv_insn &insn) > +{ > + switch (insn.opcode ()) > + { > + case riscv_insn::BEQ: > + case riscv_insn::BNE: > + case riscv_insn::BLT: > + case riscv_insn::BGE: > + case riscv_insn::BLTU: > + case riscv_insn::BGEU: > + case riscv_insn::JAL: > + return true; > + } > + > + return false; > +} > + > /* We can't put a breakpoint in the middle of a lr/sc atomic sequence, so look > for the end of the sequence and put the breakpoint there. */ > > -static bool > -riscv_next_pc_atomic_sequence (struct regcache *regcache, CORE_ADDR pc, > - CORE_ADDR *next_pc) > +static std::vector > +riscv_deal_with_atomic_sequence (struct regcache *regcache, CORE_ADDR pc) > { > struct gdbarch *gdbarch = regcache->arch (); > struct riscv_insn insn; > - CORE_ADDR cur_step_pc = pc; > - CORE_ADDR last_addr = 0; > + CORE_ADDR cur_step_pc = pc, next_pc; > + std::vector next_pcs; > + const int atomic_sequence_length = 16; > + bool found_valid_atomic_sequence = false; > + enum riscv_insn::opcode lr_opcode; > > /* First instruction has to be a load reserved. */ > insn.decode (gdbarch, cur_step_pc); > - if (insn.opcode () != riscv_insn::LR) > - return false; > - cur_step_pc = cur_step_pc + insn.length (); > + lr_opcode = insn.opcode (); > + if (lr_opcode != riscv_insn::LR_D && lr_opcode != riscv_insn::LR_W) > + return {}; > > - /* Next instruction should be branch to exit. */ > - insn.decode (gdbarch, cur_step_pc); > - if (insn.opcode () != riscv_insn::BNE) > - return false; > - last_addr = cur_step_pc + insn.imm_signed (); > - cur_step_pc = cur_step_pc + insn.length (); > + /* A lr/sc sequence comprise at most 16 instructions placed sequentially in memory. */ > + for (int insn_count = 0; insn_count < atomic_sequence_length; ++insn_count) > + { > + cur_step_pc += insn.length (); > + insn.decode (gdbarch, cur_step_pc); > > - /* Next instruction should be store conditional. */ > - insn.decode (gdbarch, cur_step_pc); > - if (insn.opcode () != riscv_insn::SC) > - return false; > - cur_step_pc = cur_step_pc + insn.length (); > + /* The dynamic code executed between lr/sc can only contain instructions > + from the base I instruction set, excluding loads, stores, backward jumps, > + taken backward branches, JALR, FENCE, FENCE.I, and SYSTEM instructions. > + If the C extension is supported, then compressed forms of the aforementioned > + I instructions are also permitted. */ > + > + if (riscv_insn_is_non_cti_and_allowed_in_atomic_sequence (insn)) > + { > + continue; > + } > + /* Look for a conditional branch instruction, check if it's taken forward or not. */ > + else if (riscv_insn_is_direct_branch (insn)) > + { > + if (insn.imm_signed () > 0) > + next_pc = cur_step_pc + insn.imm_signed (); > + else > + break; > + } > + /* Look for a paired SC instruction which closes the atomic sequence. */ > + else if ((insn.opcode () == riscv_insn::SC_D && lr_opcode == riscv_insn::LR_D) > + || (insn.opcode () == riscv_insn::SC_W && lr_opcode == riscv_insn::LR_W)) > + { > + found_valid_atomic_sequence = true; > + } > + else > + break; > + } > + > + if (!found_valid_atomic_sequence) > + return {}; > > /* Next instruction should be branch to start. */ > insn.decode (gdbarch, cur_step_pc); > if (insn.opcode () != riscv_insn::BNE) > - return false; > + return {}; > if (pc != (cur_step_pc + insn.imm_signed ())) > - return false; > - cur_step_pc = cur_step_pc + insn.length (); > - > - /* We should now be at the end of the sequence. */ > - if (cur_step_pc != last_addr) > - return false; > + return {}; > + cur_step_pc += insn.length (); > > - *next_pc = cur_step_pc; > - return true; > + next_pc = cur_step_pc; > + next_pcs.push_back (next_pc); > + return next_pcs; > } > > /* This is called just before we want to resume the inferior, if we want to > @@ -4458,14 +4644,14 @@ riscv_next_pc_atomic_sequence (struct regcache *regcache, CORE_ADDR pc, > std::vector > riscv_software_single_step (struct regcache *regcache) > { > - CORE_ADDR pc, next_pc; > - > - pc = regcache_read_pc (regcache); > + CORE_ADDR cur_pc = regcache_read_pc (regcache), next_pc; > + std::vector next_pcs > + = riscv_deal_with_atomic_sequence (regcache, cur_pc); > > - if (riscv_next_pc_atomic_sequence (regcache, pc, &next_pc)) > - return {next_pc}; > + if (!next_pcs.empty ()) > + return next_pcs; > > - next_pc = riscv_next_pc (regcache, pc); > + next_pc = riscv_next_pc (regcache, cur_pc); > > return {next_pc}; > } > -- > 2.39.2 (Apple Git-143)