* [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string @ 2023-03-24 13:41 Gerd Hoffmann 2023-03-24 13:59 ` Ni, Ray 2023-03-24 14:56 ` Laszlo Ersek 0 siblings, 2 replies; 6+ messages in thread From: Gerd Hoffmann @ 2023-03-24 13:41 UTC (permalink / raw) To: devel Cc: Eric Dong, Gerd Hoffmann, Oliver Steffen, Rahul Kumar, Ray Ni, Pawel Polawski, Laszlo Ersek BufferPages is UINTN, so we need "%Lu" when printing it. Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c index cf69161caa4b..08663b1b1ab4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c @@ -880,7 +880,7 @@ PiCpuSmmEntry ( BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * (mMaxNumberOfCpus - 1)); Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); if (Buffer == NULL) { - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", BufferPages)); CpuDeadLoop (); return EFI_OUT_OF_RESOURCES; } -- 2.39.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string 2023-03-24 13:41 [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string Gerd Hoffmann @ 2023-03-24 13:59 ` Ni, Ray 2023-03-24 15:27 ` Michael D Kinney 2023-03-24 14:56 ` Laszlo Ersek 1 sibling, 1 reply; 6+ messages in thread From: Ni, Ray @ 2023-03-24 13:59 UTC (permalink / raw) To: Gerd Hoffmann, devel@edk2.groups.io, Kinney, Michael D Cc: Dong, Eric, Oliver Steffen, Kumar, Rahul R, Pawel Polawski, Laszlo Ersek Gerd, "%d" tells "int" type value is in the stack. This actually works in both 32bit and 64bit case assuming the BufferPages is less than MAX_UINT32. But if using "%Lu", it tells that "uint_64" type value is in the stack. This precisely describes the stack content in 64bit mode. But it may print random value in the stack as high-32 bit in 32bit mode. @Kinney, Michael D, any comments? Thanks, Ray > -----Original Message----- > From: Gerd Hoffmann <kraxel@redhat.com> > Sent: Friday, March 24, 2023 9:42 PM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Pawel Polawski > <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com> > Subject: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > BufferPages is UINTN, so we need "%Lu" when printing it. > > Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > index cf69161caa4b..08663b1b1ab4 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > @@ -880,7 +880,7 @@ PiCpuSmmEntry ( > BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * > (mMaxNumberOfCpus - 1)); > Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > if (Buffer == NULL) { > - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); > + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", > BufferPages)); > CpuDeadLoop (); > return EFI_OUT_OF_RESOURCES; > } > -- > 2.39.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string 2023-03-24 13:59 ` Ni, Ray @ 2023-03-24 15:27 ` Michael D Kinney 2023-03-24 15:35 ` Michael D Kinney 0 siblings, 1 reply; 6+ messages in thread From: Michael D Kinney @ 2023-03-24 15:27 UTC (permalink / raw) To: Ni, Ray, Gerd Hoffmann, devel@edk2.groups.io Cc: Dong, Eric, Oliver Steffen, Kumar, Rahul R, Pawel Polawski, Laszlo Ersek, Kinney, Michael D 1) Keep %d and change BufferPages to type UINT32. 2) Change to %Ld and change BufferPages to type UINT64. Mike > -----Original Message----- > From: Ni, Ray <ray.ni@intel.com> > Sent: Friday, March 24, 2023 7:00 AM > To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; > Pawel Polawski <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com> > Subject: RE: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > Gerd, > "%d" tells "int" type value is in the stack. > This actually works in both 32bit and 64bit case assuming the BufferPages is less than MAX_UINT32. > > But if using "%Lu", it tells that "uint_64" type value is in the stack. > This precisely describes the stack content in 64bit mode. > But it may print random value in the stack as high-32 bit in 32bit mode. > > @Kinney, Michael D, any comments? > > Thanks, > Ray > > > > -----Original Message----- > > From: Gerd Hoffmann <kraxel@redhat.com> > > Sent: Friday, March 24, 2023 9:42 PM > > To: devel@edk2.groups.io > > Cc: Dong, Eric <eric.dong@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > > Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R > > <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Pawel Polawski > > <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com> > > Subject: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > > > BufferPages is UINTN, so we need "%Lu" when printing it. > > > > Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") > > Reported-by: Laszlo Ersek <lersek@redhat.com> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > --- > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > index cf69161caa4b..08663b1b1ab4 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > @@ -880,7 +880,7 @@ PiCpuSmmEntry ( > > BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * > > (mMaxNumberOfCpus - 1)); > > Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > > if (Buffer == NULL) { > > - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); > > + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", > > BufferPages)); > > CpuDeadLoop (); > > return EFI_OUT_OF_RESOURCES; > > } > > -- > > 2.39.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string 2023-03-24 15:27 ` Michael D Kinney @ 2023-03-24 15:35 ` Michael D Kinney 0 siblings, 0 replies; 6+ messages in thread From: Michael D Kinney @ 2023-03-24 15:35 UTC (permalink / raw) To: Ni, Ray, Gerd Hoffmann, devel@edk2.groups.io Cc: Dong, Eric, Oliver Steffen, Kumar, Rahul R, Pawel Polawski, Laszlo Ersek, Kinney, Michael D And from Laszlo's responses, 3 total options. 1) Keep %d and change BufferPages to type UINT32. 2) Change to %Ld and change BufferPages to type UINT64. 3) Change to %Ld and typecast BufferPages to UINT64 in DEBUG(). All 3 will work. Best suggestion is (3) from Laszlo. Mike > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Friday, March 24, 2023 8:27 AM > To: Ni, Ray <ray.ni@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; > Pawel Polawski <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com> > Subject: RE: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > 1) Keep %d and change BufferPages to type UINT32. > 2) Change to %Ld and change BufferPages to type UINT64.> > Mike > > > -----Original Message----- > > From: Ni, Ray <ray.ni@intel.com> > > Sent: Friday, March 24, 2023 7:00 AM > > To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com> > > Cc: Dong, Eric <eric.dong@intel.com>; Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; > > Pawel Polawski <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com> > > Subject: RE: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > > > Gerd, > > "%d" tells "int" type value is in the stack. > > This actually works in both 32bit and 64bit case assuming the BufferPages is less than MAX_UINT32. > > > > But if using "%Lu", it tells that "uint_64" type value is in the stack. > > This precisely describes the stack content in 64bit mode. > > But it may print random value in the stack as high-32 bit in 32bit mode. > > > > @Kinney, Michael D, any comments? > > > > Thanks, > > Ray > > > > > > > -----Original Message----- > > > From: Gerd Hoffmann <kraxel@redhat.com> > > > Sent: Friday, March 24, 2023 9:42 PM > > > To: devel@edk2.groups.io > > > Cc: Dong, Eric <eric.dong@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; > > > Oliver Steffen <osteffen@redhat.com>; Kumar, Rahul R > > > <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>; Pawel Polawski > > > <ppolawsk@redhat.com>; Laszlo Ersek <lersek@redhat.com> > > > Subject: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string > > > > > > BufferPages is UINTN, so we need "%Lu" when printing it. > > > > > > Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") > > > Reported-by: Laszlo Ersek <lersek@redhat.com> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > > --- > > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > > index cf69161caa4b..08663b1b1ab4 100644 > > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > > > @@ -880,7 +880,7 @@ PiCpuSmmEntry ( > > > BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * > > > (mMaxNumberOfCpus - 1)); > > > Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > > > if (Buffer == NULL) { > > > - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); > > > + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", > > > BufferPages)); > > > CpuDeadLoop (); > > > return EFI_OUT_OF_RESOURCES; > > > } > > > -- > > > 2.39.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string 2023-03-24 13:41 [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string Gerd Hoffmann 2023-03-24 13:59 ` Ni, Ray @ 2023-03-24 14:56 ` Laszlo Ersek 2023-03-24 14:58 ` Laszlo Ersek 1 sibling, 1 reply; 6+ messages in thread From: Laszlo Ersek @ 2023-03-24 14:56 UTC (permalink / raw) To: Gerd Hoffmann, devel Cc: Eric Dong, Oliver Steffen, Rahul Kumar, Ray Ni, Pawel Polawski Hi Gerd, On 3/24/23 14:41, Gerd Hoffmann wrote: > BufferPages is UINTN, so we need "%Lu" when printing it. > > Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > index cf69161caa4b..08663b1b1ab4 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c > @@ -880,7 +880,7 @@ PiCpuSmmEntry ( > BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * (mMaxNumberOfCpus - 1)); > Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); > if (Buffer == NULL) { > - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); > + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", BufferPages)); > CpuDeadLoop (); > return EFI_OUT_OF_RESOURCES; > } you missed part of my off-list comment; it was: > "BufferPages" has type UINTN, it should not be printed with "%d" (%d > is for INT32). The proper way to print a UINTN value with DEBUG is to > cast the value to UINT64, and print that with %Lu (or %Lx). Refer to "cast the value to UINT64". UINTN is either UINT32 or UINT64. There is no format specifier that's dedicated specifically to UINTN. You can't use fixed %u with UINTN because that will be wrong on 64-bit platforms (%u takes UINT32 but UINTN will be UINT64), and you can't use fixed %Lu with UINTN because that will be wrong on 32-bit platforms (%Lu takes UINT64 but UINTN will be UINT32). The proper way is to (1) cast the UINTN to UINT64 first (which will be a no-op on 64-bit platforms, and it will widen UINT32 to UINT64 on 32-bit platforms, without changing the value), and then (2) print *that* (the now certainly UINT64 value) with %Lu. DEBUG (( DEBUG_ERROR, "Failed to allocate %Lu pages.\n", (UINT64)BufferPages )); ( Here's an idea: > diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h > index 8d523cac528d..bdacec9777f0 100644 > --- a/MdePkg/Include/Library/PrintLib.h > +++ b/MdePkg/Include/Library/PrintLib.h > @@ -197,6 +197,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define PREFIX_ZERO 0x20 > #define RADIX_HEX 0x80 > > +#if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_LOONGARCH64) || \ > + defined (MDE_CPU_RISCV64) || defined (MDE_CPU_X64) > +#define PRIuN "%Lu" > +#define PRIxN "%Lx" > +#elif defined (MDE_CPU_ARM) || defined (MDE_CPU_IA32) > +#define PRIuN "%Lu" > +#define PRIxN "%Lx" > +#else > +#error Unknown CPU architecture. > +#endif > + > /** > Produces a Null-terminated Unicode string in an output buffer based on > a Null-terminated Unicode format string and a VA_LIST argument list. and then you could do DEBUG (( DEBUG_ERROR, "Failed to allocate " PRIuN " pages.\n", BufferPages )); (Note the lack of a cast.) Note: I do not cover MDE_CPU_EBC above, in "PrintLib.h". I don't know how EBC (EFI Byte Code) deals with 32-bit vs. 64-bit. The UEFI 2.10 spec says in "22.1.1 Processor Architecture Independence": "64-bit C source code" and "The interpreter handles 32 vs. 64 bit issues". Unfortunately, that doesn't seem to help: it does not help us decide the size of UINTN at compile time. More precisely: if you check "MdePkg/Include/Ebc/ProcessorBind.h", it contains: > /// > /// Signed value of native width. (4 bytes on supported 32-bit processor instructions, > /// 8 bytes on supported 64-bit processor instructions) > /// "long" type scales to the processor native size with EBC compiler > /// > typedef long INTN; > /// > /// The unsigned value of native width. (4 bytes on supported 32-bit processor instructions; > /// 8 bytes on supported 64-bit processor instructions) > /// "long" type scales to the processor native size with the EBC compiler. > /// > typedef unsigned long UINTN; In other words, the size of UINTN *is* chosen at compile time, but the choice cannot be determined from MDE_CPU_EBC. And no other macro that "MdePkg/Include/Ebc/ProcessorBind.h" defines seems to be suitable for investigation with #if preprocessing directives. Anyway: the explicit (UINT64) cast with the open-coded %Lu conversion specifier will work fine on EBC as well, so I suggest sticking with that. ) Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string 2023-03-24 14:56 ` Laszlo Ersek @ 2023-03-24 14:58 ` Laszlo Ersek 0 siblings, 0 replies; 6+ messages in thread From: Laszlo Ersek @ 2023-03-24 14:58 UTC (permalink / raw) To: Gerd Hoffmann, devel Cc: Eric Dong, Oliver Steffen, Rahul Kumar, Ray Ni, Pawel Polawski On 3/24/23 15:56, Laszlo Ersek wrote: > Hi Gerd, > > On 3/24/23 14:41, Gerd Hoffmann wrote: >> BufferPages is UINTN, so we need "%Lu" when printing it. >> >> Fixes: 4f441d024bee ("UefiCpuPkg/PiSmmCpuDxeSmm: fix error handling") >> Reported-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c >> index cf69161caa4b..08663b1b1ab4 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c >> @@ -880,7 +880,7 @@ PiCpuSmmEntry ( >> BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize * (mMaxNumberOfCpus - 1)); >> Buffer = AllocateAlignedCodePages (BufferPages, SIZE_4KB); >> if (Buffer == NULL) { >> - DEBUG ((DEBUG_ERROR, "Failed to allocate %d pages.\n", BufferPages)); >> + DEBUG ((DEBUG_ERROR, "Failed to allocate %Lu pages.\n", BufferPages)); >> CpuDeadLoop (); >> return EFI_OUT_OF_RESOURCES; >> } > > you missed part of my off-list comment; it was: > >> "BufferPages" has type UINTN, it should not be printed with "%d" (%d >> is for INT32). The proper way to print a UINTN value with DEBUG is to >> cast the value to UINT64, and print that with %Lu (or %Lx). > > Refer to "cast the value to UINT64". > > UINTN is either UINT32 or UINT64. There is no format specifier that's > dedicated specifically to UINTN. You can't use fixed %u with UINTN > because that will be wrong on 64-bit platforms (%u takes UINT32 but > UINTN will be UINT64), and you can't use fixed %Lu with UINTN because > that will be wrong on 32-bit platforms (%Lu takes UINT64 but UINTN will > be UINT32). The proper way is to (1) cast the UINTN to UINT64 first > (which will be a no-op on 64-bit platforms, and it will widen UINT32 to > UINT64 on 32-bit platforms, without changing the value), and then (2) > print *that* (the now certainly UINT64 value) with %Lu. > > DEBUG (( > DEBUG_ERROR, > "Failed to allocate %Lu pages.\n", > (UINT64)BufferPages > )); > > ( > > Here's an idea: > >> diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h >> index 8d523cac528d..bdacec9777f0 100644 >> --- a/MdePkg/Include/Library/PrintLib.h >> +++ b/MdePkg/Include/Library/PrintLib.h >> @@ -197,6 +197,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >> #define PREFIX_ZERO 0x20 >> #define RADIX_HEX 0x80 >> >> +#if defined (MDE_CPU_AARCH64) || defined (MDE_CPU_LOONGARCH64) || \ >> + defined (MDE_CPU_RISCV64) || defined (MDE_CPU_X64) >> +#define PRIuN "%Lu" >> +#define PRIxN "%Lx" >> +#elif defined (MDE_CPU_ARM) || defined (MDE_CPU_IA32) >> +#define PRIuN "%Lu" >> +#define PRIxN "%Lx" aargh, how annoying. I'm in a rush, sorry. I over-edited this code and forgot to remove the "L" from the latter two. Laszlo >> +#else >> +#error Unknown CPU architecture. >> +#endif >> + >> /** >> Produces a Null-terminated Unicode string in an output buffer based on >> a Null-terminated Unicode format string and a VA_LIST argument list. > > and then you could do > > DEBUG (( > DEBUG_ERROR, > "Failed to allocate " PRIuN " pages.\n", > BufferPages > )); > > (Note the lack of a cast.) > > Note: I do not cover MDE_CPU_EBC above, in "PrintLib.h". I don't know > how EBC (EFI Byte Code) deals with 32-bit vs. 64-bit. The UEFI 2.10 spec > says in "22.1.1 Processor Architecture Independence": "64-bit C source > code" and "The interpreter handles 32 vs. 64 bit issues". Unfortunately, > that doesn't seem to help: it does not help us decide the size of UINTN > at compile time. > > More precisely: if you check "MdePkg/Include/Ebc/ProcessorBind.h", it > contains: > >> /// >> /// Signed value of native width. (4 bytes on supported 32-bit processor instructions, >> /// 8 bytes on supported 64-bit processor instructions) >> /// "long" type scales to the processor native size with EBC compiler >> /// >> typedef long INTN; >> /// >> /// The unsigned value of native width. (4 bytes on supported 32-bit processor instructions; >> /// 8 bytes on supported 64-bit processor instructions) >> /// "long" type scales to the processor native size with the EBC compiler. >> /// >> typedef unsigned long UINTN; > > In other words, the size of UINTN *is* chosen at compile time, but the > choice cannot be determined from MDE_CPU_EBC. And no other macro that > "MdePkg/Include/Ebc/ProcessorBind.h" defines seems to be suitable for > investigation with #if preprocessing directives. > > Anyway: the explicit (UINT64) cast with the open-coded %Lu conversion > specifier will work fine on EBC as well, so I suggest sticking with > that. > > ) > > Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-24 15:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-24 13:41 [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: fix format string Gerd Hoffmann 2023-03-24 13:59 ` Ni, Ray 2023-03-24 15:27 ` Michael D Kinney 2023-03-24 15:35 ` Michael D Kinney 2023-03-24 14:56 ` Laszlo Ersek 2023-03-24 14:58 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox