From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.96.140; helo=cam-smtp0.cambridge.arm.com; envelope-from=evan.lloyd@arm.com; receiver=edk2-devel@lists.01.org Received: from cam-smtp0.cambridge.arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3F22B21EC8D19 for ; Tue, 26 Sep 2017 13:12:25 -0700 (PDT) Received: from E111747.Emea.Arm.com (e111747.emea.arm.com [10.1.27.40]) by cam-smtp0.cambridge.arm.com (8.13.8/8.13.8) with ESMTP id v8QKFY5T017392; Tue, 26 Sep 2017 21:15:35 +0100 From: evan.lloyd@arm.com To: edk2-devel@lists.01.org Cc: "ard.biesheuvel@linaro.org"@arm.com, "leif.lindholm@linaro.org"@arm.com, "Matteo.Carlini@arm.com"@arm.com, "nd@arm.com"@arm.com Date: Tue, 26 Sep 2017 21:15:14 +0100 Message-Id: <20170926201529.11644-5-evan.lloyd@arm.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20170926201529.11644-1-evan.lloyd@arm.com> References: <20170926201529.11644-1-evan.lloyd@arm.com> Subject: [PATCH 04/19] ArmPlatformPkg: LcdGraphicsOurputDxe: Add debug asserts 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: Tue, 26 Sep 2017 20:12:26 -0000 From: Girish Pathak This change adds some debug assertions e.g to catch NULL pointer errors missing in PL11Lcd and HdLcd modules. This change also improves related error handling code. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Girish Pathak Signed-off-by: Evan Lloyd --- ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c | 44 ++++++++++++++++++-- ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c | 43 ++++++++++++++++++- ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c | 8 ++-- ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c | 8 ++-- 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c index b9859a56988f7e5be0adbaa49048a683fe586bfe..58dd9f0c77e1bc9af559a71d0c7cce72d71c6da5 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpress.c @@ -140,6 +140,7 @@ LcdPlatformInitializeDisplay ( * buffer in bytes * * @retval EFI_SUCCESS Frame buffer memory allocation success. + * @retval EFI_INVALID_PARAMETER VramBaseAddress or VramSize are NULL. * @retval !(EFI_SUCCESS) Other errors. **/ EFI_STATUS @@ -151,6 +152,13 @@ LcdPlatformGetVram ( EFI_STATUS Status; EFI_ALLOCATE_TYPE AllocationType; + // Check VramBaseAddress and VramSize are not NULL. + if (VramBaseAddress == NULL || VramSize == NULL) { + ASSERT (VramBaseAddress != NULL); + ASSERT (VramSize != NULL); + return EFI_INVALID_PARAMETER; + } + // Set the vram size *VramSize = LCD_VRAM_SIZE; @@ -169,6 +177,7 @@ LcdPlatformGetVram ( VramBaseAddress ); if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); return Status; } @@ -179,8 +188,8 @@ LcdPlatformGetVram ( *VramSize, EFI_MEMORY_WC ); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize)); return Status; } @@ -215,6 +224,7 @@ LcdPlatformSetMode ( EFI_STATUS Status; if (ModeNumber >= LcdPlatformGetMaxMode ()) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); return EFI_INVALID_PARAMETER; } @@ -264,6 +274,7 @@ LcdPlatformSetMode ( * * @retval EFI_SUCCESS Success if the requested mode is found. * @retval EFI_INVALID_PARAMETER Requested mode not found. + * @retval EFI_INVALID_PARAMETER Info is NULL. **/ EFI_STATUS LcdPlatformQueryMode ( @@ -271,7 +282,9 @@ LcdPlatformQueryMode ( OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info ) { - if (ModeNumber >= LcdPlatformGetMaxMode ()) { + if (ModeNumber >= LcdPlatformGetMaxMode () || Info == NULL) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + ASSERT (Info != NULL); return EFI_INVALID_PARAMETER; } @@ -334,6 +347,28 @@ LcdPlatformGetTimings ( ) { if (ModeNumber >= LcdPlatformGetMaxMode ()) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + return EFI_INVALID_PARAMETER; + } + + if (HRes == NULL + || HSync == NULL + || HBackPorch == NULL + || HFrontPorch == NULL + || VRes == NULL + || VSync == NULL + || VBackPorch == NULL + || VFrontPorch == NULL) + { + // One of the pointers is NULL + ASSERT (HRes != NULL); + ASSERT (HSync != NULL); + ASSERT (HBackPorch != NULL); + ASSERT (HFrontPorch != NULL); + ASSERT (VRes != NULL); + ASSERT (VSync != NULL); + ASSERT (VBackPorch != NULL); + ASSERT (VFrontPorch != NULL); return EFI_INVALID_PARAMETER; } @@ -356,6 +391,7 @@ LcdPlatformGetTimings ( * * @retval EFI_SUCCESS The requested mode is found. * @retval EFI_INVALID_PARAMETER Requested mode not found. + * @retval EFI_INVALID_PARAMETER Bpp is NULL. **/ EFI_STATUS LcdPlatformGetBpp ( @@ -363,7 +399,9 @@ LcdPlatformGetBpp ( OUT LCD_BPP * CONST Bpp ) { - if (ModeNumber >= LcdPlatformGetMaxMode ()) { + if (ModeNumber >= LcdPlatformGetMaxMode () || Bpp == NULL) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + ASSERT (Bpp != NULL); return EFI_INVALID_PARAMETER; } diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c index 6ae13f06d8b396ea1c67f0bcd735a9d70f476400..5a4abd4c6f9e84d3d690af7233c1cebfe1ad339b 100644 --- a/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpress.c @@ -191,6 +191,7 @@ LcdPlatformInitializeDisplay ( * buffer in bytes * * @retval EFI_SUCCESS Frame buffer memory allocation success. + * @retval EFI_INVALID_PARAMETER VramBaseAddress or VramSize is NULL. * @retval !(EFI_SUCCESS) Other errors. **/ EFI_STATUS @@ -203,6 +204,13 @@ LcdPlatformGetVram ( Status = EFI_SUCCESS; + // Check VramBaseAddress and VramSize are not NULL. + if (VramBaseAddress == NULL || VramSize == NULL) { + ASSERT (VramBaseAddress != NULL); + ASSERT (VramSize != NULL); + return EFI_INVALID_PARAMETER; + } + // Is it on the motherboard or on the daughterboard? switch (PL111_CLCD_SITE) { @@ -223,6 +231,7 @@ LcdPlatformGetVram ( VramBaseAddress ); if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); return Status; } @@ -233,8 +242,8 @@ LcdPlatformGetVram ( *VramSize, EFI_MEMORY_WC ); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES (*VramSize)); return Status; } @@ -293,6 +302,7 @@ LcdPlatformSetMode ( UINT32 SysId; if (ModeNumber >= LcdPlatformGetMaxMode ()) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); return EFI_INVALID_PARAMETER; } @@ -359,6 +369,7 @@ LcdPlatformSetMode ( * (on success). * * @retval EFI_SUCCESS Success if the requested mode is found. + * @retval EFI_INVALID_PARAMETER Info is NULL. * @retval EFI_INVALID_PARAMETER Requested mode not found. **/ EFI_STATUS @@ -367,7 +378,9 @@ LcdPlatformQueryMode ( OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION * CONST Info ) { - if (ModeNumber >= LcdPlatformGetMaxMode ()) { + if (ModeNumber >= LcdPlatformGetMaxMode () || Info == NULL) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + ASSERT (Info != NULL); return EFI_INVALID_PARAMETER; } @@ -415,6 +428,7 @@ LcdPlatformQueryMode ( * * @retval EFI_SUCCESS Success if the requested mode is found. * @retval EFI_INVALID_PARAMETER Requested mode not found. + * @retval EFI_INVALID_PARAMETER One of the OUT parameters is NULL. **/ EFI_STATUS LcdPlatformGetTimings ( @@ -430,6 +444,28 @@ LcdPlatformGetTimings ( ) { if (ModeNumber >= LcdPlatformGetMaxMode ()) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + return EFI_INVALID_PARAMETER; + } + + if (HRes == NULL + || HSync == NULL + || HBackPorch == NULL + || HFrontPorch == NULL + || VRes == NULL + || VSync == NULL + || VBackPorch == NULL + || VFrontPorch == NULL) + { + // One of the pointers is NULL + ASSERT (HRes != NULL); + ASSERT (HSync != NULL); + ASSERT (HBackPorch != NULL); + ASSERT (HFrontPorch != NULL); + ASSERT (VRes != NULL); + ASSERT (VSync != NULL); + ASSERT (VBackPorch != NULL); + ASSERT (VFrontPorch != NULL); return EFI_INVALID_PARAMETER; } @@ -452,6 +488,7 @@ LcdPlatformGetTimings ( * * @retval EFI_SUCCESS The requested mode is found. * @retval EFI_INVALID_PARAMETER Requested mode not found. + * @retval EFI_INVALID_PARAMETER Bpp is NULL. **/ EFI_STATUS LcdPlatformGetBpp ( @@ -460,6 +497,8 @@ LcdPlatformGetBpp ( ) { if (ModeNumber >= LcdPlatformGetMaxMode ()) { + ASSERT (ModeNumber < LcdPlatformGetMaxMode ()); + ASSERT (Bpp != NULL); return EFI_INVALID_PARAMETER; } diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c index 5f950579720fb69e0a481f697a5cc4038158b409..a266671a26f01d31e8ddb0cf7cbfe59d2f4dc49c 100644 --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcd.c @@ -109,15 +109,15 @@ LcdSetMode ( &VBackPorch, &VFrontPorch ); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { - return EFI_DEVICE_ERROR; + ASSERT_EFI_ERROR (Status); + return Status; } Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { - return EFI_DEVICE_ERROR; + ASSERT_EFI_ERROR (Status); + return Status; } BytesPerPixel = GetBytesPerPixel (LcdBpp); diff --git a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c index 386e6140a69b045f77ee7fa60c4587d8bf4e7d54..f432c8d802614e8a0e4ddab3898f6e0dbf9a1572 100644 --- a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c +++ b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111Lcd.c @@ -110,15 +110,15 @@ LcdSetMode ( &VBackPorch, &VFrontPorch ); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { - return EFI_DEVICE_ERROR; + ASSERT_EFI_ERROR (Status); + return Status; } Status = LcdPlatformGetBpp (ModeNumber, &LcdBpp); - ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { - return EFI_DEVICE_ERROR; + ASSERT_EFI_ERROR (Status); + return Status; } // Disable the CLCD_LcdEn bit -- Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")