From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5179B8039D for ; Wed, 22 Mar 2017 06:13:00 -0700 (PDT) Received: by mail-io0-x236.google.com with SMTP id f84so65130484ioj.0 for ; Wed, 22 Mar 2017 06:13:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=HwqGMY3KoJjtdGgO/6mBpTMgNb4SOsQ+GZP1g8PALx0=; b=dY5njTrPWcBf1+oL6abbxNtOQCvJC0r170FmPA6z8r8umTQ0mmmeX7lszz4zEPybxJ hZGlzssmoevtU2Zlpp84H58yU3NwZ9c6gJ0bAhOEkRg6aO1h5I9uwPe/cL8FvfC9c24h OsxI6yyBr/ptw3VmRWLSLjzaIywk6+xdSK8eA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=HwqGMY3KoJjtdGgO/6mBpTMgNb4SOsQ+GZP1g8PALx0=; b=j//E0fwcR0/Je9qJAuqx+DeyE5PrYu7lMGR/h8gxozIvowcB/RdkOvhf8788rhRIRM 2Ixpsv4MlwlycQ5+aXZ8lyYSRv0TfPxcIQSAuLwWJr4DUR5JVbHrCc2aHN4CamdgDSCN 3pcMZtD9c1jX0cJOLGlWYoJhYMlUInQ/OVOrsgPVSkMtBVo5GmOvk5c9kdqtkuh2O1a/ GCUWfnjVQKe89K960r2zI1qZdsF2bawbNaVvFrIOWZvk9cx7oNQzo1VaAQk9DBu0x2JR bOD5l7YeckCbjpeLwFM+cLgFLvWRcekLTslQ00IC85RqZHwFf844yuGsQCrTP5On7Dg+ dVqQ== X-Gm-Message-State: AFeK/H3r6u8OQQh2nzvB38xH/DQBsMU1N6gwysnDvicL+ENz3qTx1F+9QF0zLTlWuvmliXsXuYdsVcTofpqnTCVW X-Received: by 10.107.141.134 with SMTP id p128mr27885256iod.83.1490188379613; Wed, 22 Mar 2017 06:12:59 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Wed, 22 Mar 2017 06:12:59 -0700 (PDT) In-Reply-To: <20170322130743.GX16034@bivouac.eciton.net> References: <1490043181-20031-1-git-send-email-ard.biesheuvel@linaro.org> <1490043181-20031-3-git-send-email-ard.biesheuvel@linaro.org> <20170322130743.GX16034@bivouac.eciton.net> From: Ard Biesheuvel Date: Wed, 22 Mar 2017 13:12:59 +0000 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Ryan Harkin , "Cohen, Eugene" Subject: Re: [PATCH 2/3] ArmPkg/DefaultExceptionHandlerLib: walk call stack unconditionally X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Mar 2017 13:13:00 -0000 Content-Type: text/plain; charset=UTF-8 On 22 March 2017 at 13:07, Leif Lindholm wrote: > On Mon, Mar 20, 2017 at 08:53:00PM +0000, Ard Biesheuvel wrote: >> Currently, we only attempt to walk the call stack and print a backtrace >> if the program counter refers to a location covered by a PE/COFF image. >> However, regardless of the value of PC, the frame pointer may still have >> a meaningful value, and so we can still produce the remainder of the >> backtrace. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 56 +++++++++++--------- >> 1 file changed, 31 insertions(+), 25 deletions(-) >> >> diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c >> index 2f9c2ede37c1..1024bf48c63d 100644 >> --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c >> +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c >> @@ -181,37 +181,43 @@ DefaultExceptionHandler ( >> DEBUG ((EFI_D_ERROR, "PC 0x%012lx (0x%012lx+0x%08x) [ 0] %a\n", >> SystemContext.SystemContextAArch64->ELR, ImageBase, >> SystemContext.SystemContextAArch64->ELR - ImageBase, BaseName (Pdb))); >> + } else { >> + DEBUG ((EFI_D_ERROR, "PC 0x%012lx\n", SystemContext.SystemContextAArch64->ELR)); >> + } >> > > come_from: > >> - if ((UINT64 *)SystemContext.SystemContextAArch64->FP != 0) { >> - Idx = 0; >> + if ((UINT64 *)SystemContext.SystemContextAArch64->FP != 0) { >> + Idx = 0; >> >> - RootFp[0] = ((UINT64 *)SystemContext.SystemContextAArch64->FP)[0]; >> - RootFp[1] = ((UINT64 *)SystemContext.SystemContextAArch64->FP)[1]; >> - if (RootFp[1] != SystemContext.SystemContextAArch64->LR) { >> - RootFp[0] = SystemContext.SystemContextAArch64->FP; >> - RootFp[1] = SystemContext.SystemContextAArch64->LR; >> - } >> - for (Fp = RootFp; Fp[0] != 0; Fp = (UINT64 *)Fp[0]) { >> - Pdb = GetImageName (Fp[1], &ImageBase, &PeCoffSizeOfHeader); >> - if (Pdb != NULL) { >> - if (Pdb != PrevPdb) { >> - Idx++; >> - PrevPdb = Pdb; >> - } >> - DEBUG ((EFI_D_ERROR, "PC 0x%012lx (0x%012lx+0x%08x) [% 2d] %a\n", >> - Fp[1], ImageBase, Fp[1] - ImageBase, Idx, BaseName (Pdb))); >> + RootFp[0] = ((UINT64 *)SystemContext.SystemContextAArch64->FP)[0]; >> + RootFp[1] = ((UINT64 *)SystemContext.SystemContextAArch64->FP)[1]; >> + if (RootFp[1] != SystemContext.SystemContextAArch64->LR) { >> + RootFp[0] = SystemContext.SystemContextAArch64->FP; >> + RootFp[1] = SystemContext.SystemContextAArch64->LR; >> + } >> + for (Fp = RootFp; Fp[0] != 0; Fp = (UINT64 *)Fp[0]) { >> + Pdb = GetImageName (Fp[1], &ImageBase, &PeCoffSizeOfHeader); >> + if (Pdb != NULL) { >> + if (Pdb != PrevPdb) { >> + Idx++; >> + PrevPdb = Pdb; >> } >> + DEBUG ((EFI_D_ERROR, "PC 0x%012lx (0x%012lx+0x%08x) [% 2d] %a\n", >> + Fp[1], ImageBase, Fp[1] - ImageBase, Idx, BaseName (Pdb))); > > Diff's a bit iffy, but can you confirm there is no functional change > between come_from and here? Just the indentation shuffle? > Yes. diff -w output is here http://pastebin.com/gRmeeipp >> + } else { >> + DEBUG ((EFI_D_ERROR, "PC 0x%012lx\n", Fp[1])); >> } >> - PrevPdb = Pdb = GetImageName (SystemContext.SystemContextAArch64->ELR, &ImageBase, &PeCoffSizeOfHeader); >> + } >> + PrevPdb = Pdb = GetImageName (SystemContext.SystemContextAArch64->ELR, &ImageBase, &PeCoffSizeOfHeader); >> + if (Pdb != NULL) { >> DEBUG ((EFI_D_ERROR, "\n[ 0] %a\n", Pdb)); >> + } >> >> - Idx = 0; >> - for (Fp = RootFp; Fp[0] != 0; Fp = (UINT64 *)Fp[0]) { >> - Pdb = GetImageName (Fp[1], &ImageBase, &PeCoffSizeOfHeader); >> - if (Pdb != NULL && Pdb != PrevPdb) { >> - DEBUG ((EFI_D_ERROR, "[% 2d] %a\n", ++Idx, Pdb)); >> - PrevPdb = Pdb; >> - } >> + Idx = 0; >> + for (Fp = RootFp; Fp[0] != 0; Fp = (UINT64 *)Fp[0]) { >> + Pdb = GetImageName (Fp[1], &ImageBase, &PeCoffSizeOfHeader); >> + if (Pdb != NULL && Pdb != PrevPdb) { >> + DEBUG ((EFI_D_ERROR, "[% 2d] %a\n", ++Idx, Pdb)); >> + PrevPdb = Pdb; >> } >> } >> } >> -- >> 2.7.4 >> > > If so: > Reviewed-by: Leif Lindholm > Thanks