From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id CFBBF389364B for ; Thu, 6 Aug 2020 17:59:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CFBBF389364B Received: by mail-qk1-x72e.google.com with SMTP id x69so45820454qkb.1 for ; Thu, 06 Aug 2020 10:59:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=Et9XzM0LvucO+oqzDrGlmh5GjIXbruoKr/xnLMqQkIs=; b=rm1rCSPAE8JYLPX8n/wDFf/P/D3z2DtEg+KdwVqyruyV6AhpUZ7HybcXiq6wehFdLY UHq1uVEpAcH1PZxLWkWi71NDwArvRQ7HhlQslHnJZSpAF2a0qVXu0SAfFzh6xWwco8A+ bqVgozBgr8Ans8dzxGGg7+RybVzVhb6DFwLec/yEWAOPjFG0TTmmLLPzhG3arpG7ruel 6Bsx4IYzKlJOfJ/zKumkmCt/TrlVzk1EgxkpcpXYzB99KwjOMUGPf4sY1CIIj03ygCEB 6OrirWazOliaj1AhXvLMd20XG8K/hI7L0N3krV2AKPg6Ic4Jwpv6fdROm6E4pkTb+Dt7 gm9A== X-Gm-Message-State: AOAM531c3AvlL2op1O7R2nzbYyh9pMASAudDIYP2Hec1e0jpIDEu60Uw q6mBX1Fnks5e7g/OROnFBch3CaQte6JX4A== X-Google-Smtp-Source: ABdhPJxiOZKIijgsb940yav0rUmIVxB+8pPLcL1dDW0kHYY10Yg4567WHJ/wSQqlJ+2tw74ZfQ2f1Q== X-Received: by 2002:a05:620a:123c:: with SMTP id v28mr8929324qkj.366.1596736765867; Thu, 06 Aug 2020 10:59:25 -0700 (PDT) Received: from localhost.localdomain ([2804:7f0:8283:6f2c:b77d:47b2:730b:373]) by smtp.gmail.com with ESMTPSA id r18sm5593438qtc.90.2020.08.06.10.59.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Aug 2020 10:59:25 -0700 (PDT) From: Luis Machado To: gdb-patches@sourceware.org, Alan.Hayward@arm.com Subject: [PATCH] [AArch64] Improve prologue handling (and fix PR26310) Date: Thu, 6 Aug 2020 14:59:20 -0300 Message-Id: <20200806175920.8037-1-luis.machado@linaro.org> X-Mailer: git-send-email 2.17.1 X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 06 Aug 2020 17:59:28 -0000 I initially noticed the problem with the addition of gdb.dwarf2/dw2-line-number-zero.exp. The following failures showed up: FAIL: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar1 FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar1, 1st next FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar1, 2nd next FAIL: gdb.dwarf2/dw2-line-number-zero.exp: continue to breakpoint: bar2 FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar2, 1st next FAIL: gdb.dwarf2/dw2-line-number-zero.exp: bar2, 2nd next They happen because AArch64's prologue analyzer skips too many instructions and ends up indicating a stopping point further into user code. Dump of assembler code for function bar1: 0x00000000000006f8 <+0>: stp x29, x30, [sp, #-16]! 0x00000000000006fc <+4>: mov x29, sp 0x0000000000000700 <+8>: mov w0, #0x1 // #1 0x0000000000000704 <+12>: bl 0x6e4 0x0000000000000708 <+16>: mov w0, #0x2 // #2 We should've stopped at 0x700, but the analyzer actually skips that instruction and stops at 0x704. Then GDB ends up adjusting the address further, and pushes the stopping point to 0x708 based on the SAL information. I'm not sure if this adjustment to 0x708 is correct though, as it ends up skipping past a branch. But I'm leaving that aside for now. One other complicating factor is that GCC seems to be hoisting up instructions from user code, mixing them up with prologue instructions. The following patch adjusts the heuristics a little bit, and tracks when the SP and FP get used. If we notice an instruction that is not supposed to be in the prologue, and this happens *after* SP/FP adjustments and saving of registers, we stop the analysis. This means, for PR26310, that we will now stop at 0x700. I've also added a few more unit tests to make sure the updated behavior is validated. gdb/ChangeLog: YYYY-MM-DD Luis Machado PR gdb/26310 * aarch64-tdep.c (aarch64_analyze_prologue): Track use of SP/FP and act accordingly. (aarch64_analyze_prologue_test): Add more unit tests to exercise movz/str/stur/stp skipping behavior. --- gdb/aarch64-tdep.c | 134 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 5e7d0d0b86..45b2b427c2 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -287,6 +287,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, { enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch); int i; + + /* Whether the stack has been set. This should be true when we notice a SP + to FP move or if we are using the SP as the base register for storing + data, in case the FP is ommitted. */ + bool seen_stack_set = false; + /* Track X registers and D registers in prologue. */ pv_t regs[AARCH64_X_REGISTER_COUNT + AARCH64_D_REGISTER_COUNT]; @@ -326,6 +332,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, regs[rd] = pv_add_constant (regs[rn], -inst.operands[2].imm.value); } + + /* Did we move SP to FP? */ + if (rn == AARCH64_SP_REGNUM && rd == AARCH64_FP_REGNUM) + seen_stack_set = true; } else if (inst.opcode->iclass == pcreladdr && inst.operands[1].type == AARCH64_OPND_ADDR_ADRP) @@ -358,6 +368,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, else if (inst.opcode->op == OP_MOVZ) { gdb_assert (inst.operands[0].type == AARCH64_OPND_Rd); + + /* If this shows up before we set the stack, keep going. Otherwise + stop the analysis. */ + if (seen_stack_set) + break; + regs[inst.operands[0].reg.regno] = pv_unknown (); } else if (inst.opcode->iclass == log_shift @@ -399,6 +415,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, stack.store (pv_add_constant (regs[rn], inst.operands[1].addr.offset.imm), size, regs[rt]); + + /* Are we storing with SP as a base? */ + if (rn == AARCH64_SP_REGNUM) + seen_stack_set = true; } else if ((inst.opcode->iclass == ldstpair_off || (inst.opcode->iclass == ldstpair_indexed @@ -442,6 +462,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, if (inst.operands[2].addr.writeback) regs[rn] = pv_add_constant (regs[rn], imm); + /* Ignore the instruction that allocates stack space and sets + the SP. */ + if (rn == AARCH64_SP_REGNUM && !inst.operands[2].addr.writeback) + seen_stack_set = true; } else if ((inst.opcode->iclass == ldst_imm9 /* Signed immediate. */ || (inst.opcode->iclass == ldst_pos /* Unsigned immediate. */ @@ -464,6 +488,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, stack.store (pv_add_constant (regs[rn], imm), size, regs[rt]); if (inst.operands[1].addr.writeback) regs[rn] = pv_add_constant (regs[rn], imm); + + /* Are we storing with SP as a base? */ + if (rn == AARCH64_SP_REGNUM) + seen_stack_set = true; } else if (inst.opcode->iclass == testbranch) { @@ -690,6 +718,112 @@ aarch64_analyze_prologue_test (void) } } + /* Test handling of movz before setting the frame pointer. */ + { + static const uint32_t insns[] = { + 0xa9bf7bfd, /* stp x29, x30, [sp, #-16]! */ + 0x52800020, /* mov w0, #0x1 */ + 0x910003fd, /* mov x29, sp */ + 0x528000a2, /* mov w2, #0x5 */ + 0x97fffff8, /* bl 6e4 */ + }; + + instruction_reader_test reader (insns); + + trad_frame_reset_saved_regs (gdbarch, cache.saved_regs); + CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader); + + SELF_CHECK (end == (4 - 1) * 4); + SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM); + SELF_CHECK (cache.framesize == 16); + } + + /* Test handling of movz/stp when using the stack pointer as frame + pointer. */ + { + static const uint32_t insns[] = { + 0xa9bc7bfd, /* stp x29, x30, [sp, #-64]! */ + 0x52800020, /* mov w0, #0x1 */ + 0x290207e0, /* stp w0, w1, [sp, #16] */ + 0xa9018fe2, /* stp x2, x3, [sp, #24] */ + 0x528000a2, /* mov w2, #0x5 */ + 0x97fffff8, /* bl 6e4 */ + }; + + instruction_reader_test reader (insns); + + trad_frame_reset_saved_regs (gdbarch, cache.saved_regs); + CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader); + + SELF_CHECK (end == (5 - 1) * 4); + SELF_CHECK (cache.framereg == AARCH64_SP_REGNUM); + SELF_CHECK (cache.framesize == 64); + } + + /* Test handling of movz/str when using the stack pointer as frame + pointer */ + { + static const uint32_t insns[] = { + 0xa9bc7bfd, /* stp x29, x30, [sp, #-64]! */ + 0x52800020, /* mov w0, #0x1 */ + 0xb9002be4, /* str w4, [sp, #40] */ + 0xf9001be5, /* str x5, [sp, #48] */ + 0x528000a2, /* mov w2, #0x5 */ + 0x97fffff8, /* bl 6e4 */ + }; + + instruction_reader_test reader (insns); + + trad_frame_reset_saved_regs (gdbarch, cache.saved_regs); + CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader); + + SELF_CHECK (end == (5 - 1) * 4); + SELF_CHECK (cache.framereg == AARCH64_SP_REGNUM); + SELF_CHECK (cache.framesize == 64); + } + + /* Test handling of movz/stur when using the stack pointer as frame + pointer. */ + { + static const uint32_t insns[] = { + 0xa9bc7bfd, /* stp x29, x30, [sp, #-64]! */ + 0x52800020, /* mov w0, #0x1 */ + 0xb80343e6, /* stur w6, [sp, #52] */ + 0xf80383e7, /* stur x7, [sp, #56] */ + 0x528000a2, /* mov w2, #0x5 */ + 0x97fffff8, /* bl 6e4 */ + }; + + instruction_reader_test reader (insns); + + trad_frame_reset_saved_regs (gdbarch, cache.saved_regs); + CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader); + + SELF_CHECK (end == (5 - 1) * 4); + SELF_CHECK (cache.framereg == AARCH64_SP_REGNUM); + SELF_CHECK (cache.framesize == 64); + } + + /* Test handling of movz when there is no frame pointer set or no stack + pointer used. */ + { + static const uint32_t insns[] = { + 0xa9bf7bfd, /* stp x29, x30, [sp, #-16]! */ + 0x52800020, /* mov w0, #0x1 */ + 0x528000a2, /* mov w2, #0x5 */ + 0x97fffff8, /* bl 6e4 */ + }; + + instruction_reader_test reader (insns); + + trad_frame_reset_saved_regs (gdbarch, cache.saved_regs); + CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache, reader); + + SELF_CHECK (end == (4 - 1) * 4); + SELF_CHECK (cache.framereg == AARCH64_SP_REGNUM); + SELF_CHECK (cache.framesize == 16); + } + /* Test a prologue in which there is a return address signing instruction. */ if (tdep->has_pauth ()) { -- 2.17.1