From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out30-57.freemail.mail.aliyun.com (out30-57.freemail.mail.aliyun.com [115.124.30.57]) by sourceware.org (Postfix) with ESMTPS id ECF943857C6C for ; Wed, 2 Mar 2022 11:38:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ECF943857C6C X-Alimail-AntiSpam: AC=PASS; BC=-1|-1; BR=01201311R131e4; CH=green; DM=||false|; DS=||; FP=0|-1|-1|-1|0|-1|-1|-1; HT=e01e04395; MF=lifang_xia@linux.alibaba.com; NM=1; PH=DS; RN=3; SR=0; TI=SMTPD_---0V62o4tP_1646221076; Received: from PF1K7H4W(mailfrom:lifang_xia@linux.alibaba.com fp:SMTPD_---0V62o4tP_1646221076) by smtp.aliyun-inc.com(127.0.0.1); Wed, 02 Mar 2022 19:37:57 +0800 From: To: "'Andrew Burgess'" , "'Lifang Xia'" , References: <20220301073231.1927-1-lifang_xia@c-sky.com> <87h78iynam.fsf@redhat.com> <002b01d82dd9$7bfc38f0$73f4aad0$@linux.alibaba.com> <877d9czmnc.fsf@redhat.com> In-Reply-To: <877d9czmnc.fsf@redhat.com> Subject: =?UTF-8?Q?=E7=AD=94=E5=A4=8D:_=E7=AD=94=E5=A4=8D:_=5BPATCH=5D_=5BRISC-?= =?UTF-8?Q?V=5D:_Handling_optimized_prologue?= Date: Wed, 2 Mar 2022 19:37:56 +0800 Message-ID: <003c01d82e29$f69cd600$e3d68200$@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGE4vXgp0fDXhjmwTq6t3IR4mHuCgITSJ0DAXhZyioDrCHeRa0Yvtsw Content-Language: zh-cn X-Spam-Status: No, score=-20.6 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Wed, 02 Mar 2022 11:38:05 -0000 > >> > >> I assume it's the bnez that's causing the problem for the prologue > >> scan > > here? > >> > >> Not that I really understand what that instruction is about, it > >> appears to > > be > >> branching to the next instruction. I wonder if this objdump output > >> is for > > the > >> object file, rather than the executable? I'm guessing the bnez has = a > > reloc > >> which points at, maybe, the ret at 0x100a8? > >> > >> I wonder if a better solution in this case would be to allow the > >> prologue > > scan > >> to skip over forward branches within the prologue, maybe with the > > limitation > >> that the destination has to be within the function bounds? > >> > > > > Yes, You are right. bnez cause the prologue scan failed. > > It's one of the cases I have met. As mentioned before, The optimizer > > of compiler might shove anything to the prologue. > > It could be multiple instructions, shift instructions, condition > > branches ... > > We can't handle all of instructions from the ISA specs or vendor > > extensions here. > > > > Actually, if cache is not nullptr, that means we need to build the > > frame information from prologue. Before the loop, we get the base > > address and the prologue_end, traversing all the prologue is not a > > bad choice to build the frame(if cache is not nullptr). >=20 > Except right now, if we declare that we know something based on the = prologue > then we can be reasonably sure that what we claim to know is true. >=20 > Under your proposal we would claim to know things for which we = actually have > no knowledge of whether it is true or not, e.g. if some previously = unknown > instruction has adjusted the contents of a register in some way, but = we ignore > the adjustment and just claim to know the previous register contents = anyway. >=20 > This then raises the question, would we prefer GDB to say "I don't = know", if it's > not sure, or say "The answer is ....", when that might not be true? >=20 > Are there other targets that do this aggressive prologue scanning? At first, my idea was that mark the action of storing RA as a = flag_could_be_stop and we can stop the scanning if flag_could_be_stop = or "cache is nullptr". But it still not a best choice, it might lost some registers' value = which are saved in this frame. When I saw ARM/NDS32 do this, and I followed them.=20 >=20 > An alternative might be to offer a new setting, which is off by = default, but which > allows for the aggressive prologue scanning you suggest. > However, I think such an option would have to include a suitable = warning to > indicate that some prologue instructions might be ignored. It's not a good choice. This phenomenon occurs frequently when we debug = an optimized program. >=20 > I still think you should consider just adding support for forward = branches to the > prologue scanner. Though any instruction _could_ be moved into the = prologue, > I suspect (guess) the number that actually _do_ get moved up is pretty = small. I thought about doing this, but after discussing it with my colleagues = who maintaining the compilers. There are many instructions that may appear here, and it may not be = possible to list them all. >=20 > A different strategy might be to handle unknown instructions based on = their > instruction class (R, I, S, etc). If the instruction is only touching = registers that we > don't need to preserve over the unwind, or is touching registers for = which the > previous value has already been saved, then maybe we don't need to = care about > the actual meaning of the instruction, we can just mark the = destination register > as unknown and move on. I think this sounds much closer to what you = want. I can try to find a way to solve it with your idea. But it may take a = few of days. =20 Thanks a lot, Lifang >=20 > Thanks, > Andrew >=20 >=20 >=20 > > > >> > > >> > gdb/ > >> > * riscv-tdep.c (riscv_scan_prologue): Keep scaning if cache is = not > >> NULL. > >> > --- > >> > gdb/riscv-tdep.c | 10 +++++++++- > >> > 1 file changed, 9 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index > >> > 886996c..e46d441 100644 > >> > --- a/gdb/riscv-tdep.c > >> > +++ b/gdb/riscv-tdep.c > >> > @@ -1987,7 +1987,15 @@ riscv_scan_prologue (struct gdbarch = *gdbarch, > >> > else > >> > { > >> > end_prologue_addr =3D cur_pc; > >> > - break; > >> > + > >> > + /* The optimizer might shove anything into the prologue, if > >> > + we build up cache (cache !=3D NULL) from scanning = prologue, > >> > + we just skip what we don't recognize and scan further to > >> > + make cache as complete as possible. However, if we skip > >> > + prologue, we'll stop immediately on unrecognized > >> > + instruction. */ > >> > + if (cache =3D=3D NULL) > >> > >> Use nullptr not NULL please. > >> > >> Thanks, > >> Andrew > >> > >> > >> > >> > + break; > >> > } > >> > } > >> > > >> > -- > >> > 2.7.4