[edk2,2/3] ArmPkg/DefaultExceptionHandlerLib: walk call stack unconditionally

Message ID 1490043181-20031-3-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 173bf5c847e3ca8b42c11796ce048d8e2e916ff8
Headers show
Series
  • ArmPkg: increase robustness of the crash handler
Related show

Commit Message

Ard Biesheuvel March 20, 2017, 8:53 p.m.
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 <ard.biesheuvel@linaro.org>

---
 ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c | 56 +++++++++++---------
 1 file changed, 31 insertions(+), 25 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm March 22, 2017, 1:07 p.m. | #1
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 <ard.biesheuvel@linaro.org>

> ---

>  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?

> +        } 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 <leif.lindholm@linaro.org>


/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel March 22, 2017, 1:12 p.m. | #2
On 22 March 2017 at 13:07, Leif Lindholm <leif.lindholm@linaro.org> 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 <ard.biesheuvel@linaro.org>

>> ---

>>  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 <leif.lindholm@linaro.org>

>


Thanks
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm March 22, 2017, 1:20 p.m. | #3
On Wed, Mar 22, 2017 at 01:12:59PM +0000, Ard Biesheuvel wrote:
> On 22 March 2017 at 13:07, Leif Lindholm <leif.lindholm@linaro.org> 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 <ard.biesheuvel@linaro.org>

> >> ---

> >>  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


Thanks - that's a tad more readable :)

> >> +        } 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 <leif.lindholm@linaro.org>

> >

> 

> Thanks

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch hide | download patch | download mbox

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));
+    }
 
-      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)));
+        } 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;
         }
       }
     }