EmbeddedPkg: Fix VS2022 type conversion warnings for Issue #11737 #11762

garybeihl · 2025-11-16T22:46:52Z

Fix type conversion warnings (C4244) in GdbStub that prevent it from building with Visual Studio 2022 when the /WX flag (treat warnings as errors) is enabled.

Instead of adding explicit casts at every assignment, change function parameter types from UINTN to UINT32 to match the UINT32 bitfield definitions in the IA32_DR7 structure, as suggested by maintainer review feedback.

Addresses: #11737

Description

Fix type conversion warnings (C4244) in GdbStub that prevent EmbeddedPkg from building with Visual Studio 2022 when the /WX flag (treat warnings as errors) is enabled.

Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- Examples: Does it add a new library class or move a module to a different repo.
Impacts security?
- Security - Does this PR have a direct security impact?
- Examples: Crypto algorithm change or buffer overflow fix.
Includes tests?
- Tests - Does this PR include any explicit test code?
- Examples: Unit tests or integration tests.

How This Was Tested

Verified that the following ran without error on Windows with VS2022:

build -a IA32 -p EmbeddedPkg/EmbeddedPkg.dsc -t VS2022 -b RELEASE -m EmbeddedPkg\GdbStub\GdbStub.inf
build -a X64 -p EmbeddedPkg/EmbeddedPkg.dsc -t VS2022 -b RELEASE -m EmbeddedPkg\GdbStub\GdbStub.inf

Integration Instructions

N/A

ardbiesheuvel · 2025-11-17T09:51:45Z

Please split this up.

leiflindholm · 2025-11-17T11:02:08Z

EmbeddedPkg/GdbStub/X64/Processor.c

    Dr7.Bits.G0                          = 1;
-    Dr7.Bits.RW0                         = Type;
-    Dr7.Bits.LEN0                        = Length;
+    Dr7.Bits.RW0                         = (UINT32)Type;


The bitfields in the IA32_DR7 struct are defined as part of a UINT32.
It would be cleaner to change the function arguments to match this than to cast them in every assignment location.

garybeihl · 2025-11-17T15:30:12Z

Please split this up.

Will do - thank you. The PR has been modified to address only the GdbStub changes. I will generate 2 new PRs for the MmcDxe and VirtualRealTimeClockLib

leiflindholm · 2025-11-17T15:37:04Z

Please split this up.

Will do - thank you. The PR has been modified to address only the GdbStub changes. I will generate 2 new PRs for the MmcDxe and VirtualRealTimeClockLib

I don't think @ardbiesheuvel meant split into multiple PRs, but into multiple commits.

garybeihl · 2025-11-17T15:57:14Z

Please split this up.

Will do - thank you. The PR has been modified to address only the GdbStub changes. I will generate 2 new PRs for the MmcDxe and VirtualRealTimeClockLib

I don't think @ardbiesheuvel meant split into multiple PRs, but into multiple commits.

Ok - thank you for the clarification. I'll keep it all in this PR then.

ardbiesheuvel · 2025-11-17T16:00:04Z

Please split this up.

Will do - thank you. The PR has been modified to address only the GdbStub changes. I will generate 2 new PRs for the MmcDxe and VirtualRealTimeClockLib

I don't think @ardbiesheuvel meant split into multiple PRs, but into multiple commits.

Ok - thank you for the clarification. I'll keep it all in this PR then.

Yes, please.

leiflindholm · 2025-11-17T19:47:26Z

EmbeddedPkg/GdbStub/X64/Processor.c

  @param  SystemContext   Register content at time of the exception
  @param  Register        Register value (0 - 3)
-  @param  Address         Breakpoint address value
+  @param  Address         Breakpoint address value (UINT32)


I don't think we need to annotate the data tyoes here, it's already in the prototype below.

However, is it safe to truncate Address to UINT32 also on x64?

leiflindholm · 2025-11-17T19:49:53Z

EmbeddedPkg/GdbStub/X64/Processor.c


  // Convert length data
-  Length = ConvertLengthData (Length);
+  Length = (UINT32)ConvertLengthData (Length);


ConvertLenghtData is explicitly documented to only return small integers. Can we update it to take, and return, UINT32 instead?

leiflindholm · 2025-11-17T19:51:51Z

EmbeddedPkg/GdbStub/X64/Processor.c


  // Update Dr7 with appropriate Gn, RWn and LENn bits
-  SystemContext.SystemContextIa32->Dr7 = Dr7.UintN;
+  SystemContext.SystemContextIa32->Dr7 = (UINT32)Dr7.UintN;


OK, this is snowballing a little, but it feels like it's a pure bug that IA32_DR7 is defined as a union of a UINT32 bitfield block and a UINTN. If we fix that bug, we don't need the casts (But we end up needing to rename UintN.)

leiflindholm · 2025-11-17T20:00:23Z

EmbeddedPkg/GdbStub/X64/Processor.c


  // Write Address, length data at particular DR register
-  Status = EnableDebugRegister (SystemContext, Register, Address, Length, (UINTN)BreakType);
+  Status = EnableDebugRegister (SystemContext, Register, (UINT32)Address, (UINT32)Length, (UINT32)BreakType);


Better to update:

The prototype of EnableDebugRegister (to use BREAK_TYPE instead of ).

The types of the other local variables to match what's passed on.

leiflindholm · 2025-11-17T20:00:49Z

EmbeddedPkg/GdbStub/X64/Processor.c


  // Find matching debug register
-  Status = FindMatchingDebugRegister (SystemContext, Address, Length, (UINTN)BreakType, &Register);
+  Status = FindMatchingDebugRegister (SystemContext, (UINT32)Address, (UINT32)Length, (UINT32)BreakType, &Register);


Better to update:

The prototype of EnableDebugRegister (to use BREAK_TYPE instead of ). The types of the other local variables to match what's passed on.

leiflindholm · 2025-11-17T20:04:41Z

EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c

    // Get the Status of the card.
    CmdArg = MmcHostInstance->CardInfo.RCA << 16;
-    Status = MmcHost->SendCommand (MmcHost, MMC_CMD13, CmdArg);
+    Status = MmcHost->SendCommand (MmcHost, MMC_CMD13, (UINT32)CmdArg);


Change CmdArg definition to UINT32 to match the SendCommand prototype.

leiflindholm · 2025-11-17T20:10:36Z

EmbeddedPkg/Universal/MmcDxe/MmcBlockIo.c

    // if 1 : SDXC/SDHC
    if (MmcHostInstance->CardInfo.OCRData.AccessMode & SD_CARD_CAPACITY) {
-      CmdArg = Lba;
+      CmdArg = (UINTN)Lba;


Again, change CmdArg to UINT32 instead.

leiflindholm · 2025-11-17T20:15:02Z

EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c


  // Assign a relative address value to the card
-  MmcHostInstance->CardInfo.RCA = ++mEmmcRcaCount; // TODO: might need a more sophisticated way of doing this
+  MmcHostInstance->CardInfo.RCA = (UINT16)++ mEmmcRcaCount; // TODO: might need a more sophisticated way of doing this


It seems like a bug that mEmmcRcaCount is defined as UINT32 when CARD_INFO.RCA is defined as UINT16. Maybe change mEmmcRcaCount type instead?

leiflindholm · 2025-11-17T20:20:52Z

EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c


  Counter       = GetPerformanceCounter ();
-  EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);
+  EpochSeconds += (UINTN)DivU64x64Remainder (Counter, Freq, &Remainder);


It doesn't feel useful to explicitly call functions to explicitly perform 64-bit division even on 32-bit architectures only to then throw away the top 32 bits of the result if actually on a 32-bit architecture.

Better to change EpochSeconds to UINT64.

leiflindholm · 2025-11-17T20:21:00Z

EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c

    Counter = GetPerformanceCounter ();
-    if (EpochSeconds > DivU64x64Remainder (Counter, Freq, &Remainder)) {
-      EpochSeconds -= DivU64x64Remainder (Counter, Freq, &Remainder);
+    if (EpochSeconds > (UINTN)DivU64x64Remainder (Counter, Freq, &Remainder)) {


It doesn't feel useful to explicitly call functions to explicitly perform 64-bit division even on 32-bit architectures only to then throw away the top 32 bits if actually on a 32-bit architecture.

Better to change EpochSeconds to UINT64.

Fix type conversion warnings (C4244) in GdbStub that prevent it from building with Visual Studio 2022 when the /WX flag (treat warnings as errors) is enabled. Changes made per reviewer feedback: 1. ConvertLengthData() function signature updated: - Changed parameter and return type from UINTN to UINT32 - Removed casts at call sites in EnableDebugRegister() and FindMatchingDebugRegister() for X64 - Added casts at call sites for IA32 (converts UINTN to UINT32) 2. IA32_DR7 union definition updated: - Changed union member from UINTN UintN to UINT32 Uint32 - Bitfield members already defined as UINT32, so union member should match - Updated all references from Dr7.UintN to Dr7.Uint32 throughout X64 and IA32 implementations - Removed unnecessary (UINT32) casts when assigning Dr7.Uint32 to SystemContext (for IA32) 3. EnableDebugRegister() Type parameter updated: - Changed from UINT32 to BREAK_TYPE enum - Removed unnecessary (BREAK_TYPE) casts when comparing Type with DataRead and SoftwareBreakpoint enum values - Updated function declaration in GdbStubInternal.h 4. FindMatchingDebugRegister() Type parameter updated: - Changed from UINT32 to BREAK_TYPE enum - Removed unnecessary (BREAK_TYPE) casts - Updated function declaration in GdbStubInternal.h 5. Removed redundant type annotations from function documentation: - EnableDebugRegister(): Removed (UINT32) annotations, added missing Length parameter documentation - FindMatchingDebugRegister(): Removed (UINT32) annotations These changes align variable types with their actual usage and hardware constraints, eliminating the need for many explicit casts while making the code more maintainable. Tested on: - Windows 11 with VS2022 (X64 and IA32) - Ubuntu with GCC5 (X64) Addresses: tianocore#11737 Signed-off-by: Gary Beihl <[email protected]>

Fix type conversion warnings (C4244) in MmcDxe that prevent it from building with Visual Studio 2022 when the /WX flag (treat warnings as errors) is enabled. Changes made per reviewer feedback: 1. MmcIdentification.c: - Changed mEmmcRcaCount from UINT32 to UINT16 to match the type of CardInfo.RCA (which is UINT16) - Removed unnecessary (UINT16) cast when assigning to CardInfo.RCA 2. MmcBlockIo.c: - Changed CmdArg local variable from UINTN to UINT32 in all three functions (MmcGetCardStatus, MmcTransferBlock, MmcIoBlocks) - Removed (UINT32) casts at SendCommand() call sites (6 instances) - Changed assignments from (UINTN)Lba and (UINTN)MultU64x32() to (UINT32) casts to match UINT32 CmdArg type These changes align variable types with the actual hardware interface (SendCommand expects UINT32) and eliminate unnecessary type conversions. Tested on: - Windows 11 with VS2022 (X64 and IA32) - Ubuntu with GCC5 (X64) Addresses: tianocore#11737 Signed-off-by: Gary Beihl <[email protected]>

Fix type conversion warnings (C4244) in VirtualRealTimeClockLib that prevent it from building with Visual Studio 2022 when the /WX flag (treat warnings as errors) is enabled. Changes made per reviewer feedback: 1. VirtualRealTimeClockLib.c: - Changed EpochSeconds variable from UINTN to UINT64 in both LibGetTime() and LibSetTime() functions - Changed sizeof(UINTN) to sizeof(UINT64) in EfiGetVariable() call to match the actual variable type - Removed all (UINTN) casts from DivU64x64Remainder() return values, as the function already returns UINT64 2. TimeBaseLib (prerequisite for IA32 support): - Updated EpochToEfiTime() function signature to accept UINT64 instead of UINTN for EpochSeconds parameter - Updated implementation to use DivU64x32() and ModU64x32() for proper 64-bit arithmetic on IA32 architecture - Added BaseLib.h include for 64-bit division helpers Rationale: - EpochSeconds represents Unix epoch time in seconds, which can exceed 32-bit range on IA32 systems (Y2038 problem) - DivU64x64Remainder() returns UINT64, so casting to UINTN would truncate values on 32-bit architectures - TimeBaseLib must accept UINT64 to properly handle epoch times beyond 2038 on both IA32 and X64 architectures - Using UINT64 throughout preserves the full time value and eliminates unnecessary type conversions Tested on: - Windows 11 with VS2022 (X64 and IA32) - Ubuntu with GCC5 (X64) Addresses: tianocore#11737 Signed-off-by: Gary Beihl <[email protected]>

tianocore-assign-reviewers bot requested review from ardbiesheuvel, changab and leiflindholm November 16, 2025 22:47

leiflindholm suggested changes Nov 17, 2025

View reviewed changes

garybeihl force-pushed the feature/embeddedpkgfix branch 3 times, most recently from d3a670a to e719796 Compare November 17, 2025 15:07

garybeihl force-pushed the feature/embeddedpkgfix branch from e719796 to e81738f Compare November 17, 2025 15:46

garybeihl force-pushed the feature/embeddedpkgfix branch from b3efa77 to 3fa063f Compare November 17, 2025 17:41

leiflindholm suggested changes Nov 17, 2025

View reviewed changes

garybeihl force-pushed the feature/embeddedpkgfix branch 2 times, most recently from 01d362d to 72688ab Compare November 18, 2025 17:48

garybeihl added 3 commits November 19, 2025 16:54

garybeihl force-pushed the feature/embeddedpkgfix branch from 72688ab to adc3b9c Compare November 19, 2025 21:54

EmbeddedPkg: Fix VS2022 type conversion warnings for Issue #11737 #11762

Are you sure you want to change the base?

EmbeddedPkg: Fix VS2022 type conversion warnings for Issue #11737 #11762

Uh oh!

Conversation

garybeihl commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How This Was Tested

Integration Instructions

Uh oh!

ardbiesheuvel commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

garybeihl commented Nov 17, 2025

Uh oh!

leiflindholm commented Nov 17, 2025

Uh oh!

garybeihl commented Nov 17, 2025

Uh oh!

ardbiesheuvel commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

garybeihl commented Nov 16, 2025 •

edited

Loading