ncroxon / gnu-efi

Develop EFI applications for ARM-64, ARM-32, x86_64, IA-64 (IPF), IA-32 (x86), and MIPS platforms using the GNU toolchain and the EFI development environment.
42 stars 10 forks source link

Fix exception on ARM32 with VS2022 when Print() is invoked #22

Closed pbatard closed 4 months ago

pbatard commented 4 months ago

On ARM32 only, it appears that whenever Visual Studio inlines the FloatToString() call (which it does for Release builds), the resulting executable produces an "Undefined OpCode Exception" on Print() invocation, regardless of whether there is an actual float to string conversion occurring there.

To work around this, add an explicit clause to prevent Visual Studio inlining.

pbatard commented 4 months ago

Sure, I should be able to work something out.

My guess however is that we have something specific to the Visual Studio code generation (e.g. alignment issue due to structs that are packed differently than with gcc), and that it's unlikely to affect other platforms. Because of time constraints, I will also state that I've stopped investigating after I found that FloatToString() was inlined by Visual Studio in the assembly and that de-inlining it removed the issue. But I only tested very basic applications, so it's very possible we may see similar issues on other function calls besides Print().

Anyway, if we agree that we want to disable the inlining for all compilers, and not just Visual Studio, please wait for an update of this PR.

gmbr3 commented 4 months ago

But I only tested very basic applications, so it's very possible we may see similar issues on other function calls besides Print().

If we see more cases of this, only on VS, then we can decide and make further investigation regarding alignment of structs etc. The NOINLINE macro should probably be created even if we find that to be true as it's useful for downstreams aswell.

pbatard commented 4 months ago

Updated as requested. I also took this opportunity to clean-up eficompiler.h as using #if not MSVC assume GNU wasn't ideal compared to explicitly defining the macros for the compilers they apply.

pbatard commented 4 months ago

Stealth update to fix a EFI_NORETURN that should have said EFI_NOINLINE

ncroxon commented 4 months ago

@Liai5555, did you mean to add a comment?

b6614502510713e348eb9675b7a89b2b7083e242