[MSVC][x86] Failed to build with error C2488: 'naked' can only be applied to non-member function definitions

[NEIL-smtg]

Description

Note:
To reproduce this, you'll need wait for VS 2022 17.12 Preview or later version to ship which will contain this change.

Recently, we have been building this project using the latest build of MSVC, and we have encountered the following error:

runtime\src\coreclr\debug\ee\debugger.cpp(13904) :
error C2488: 'Debugger::GenericHijackFunc': 'naked' can only be applied to non-member function definitions

Complete build log: Build.log

Reproduction Steps

1. Open X86 Native Tools Command Prompt for VS 2022
2. git clone https://github.com/dotnet/runtime.git
3. cd runtime
4. set CL=/wd5281
5. .\build.cmd -subset clr+libs -c release -arch x86 2>&1

Expected behavior

It compiles.

Actual behavior

runtime\src\coreclr\debug\ee\debugger.cpp(13904) :
error C2488: 'Debugger::GenericHijackFunc': 'naked' can only be applied to non-member function definitions

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

This is the narrowed down repro, test.cpp:

struct Foo {
    __declspec(naked) void func1() {} // gets C2488
    void func2();
    __declspec(naked) static void func3() {} // gets C2488
};

#if defined(TARGET_X86)
__declspec(naked)
#endif
void Foo::func2() {} // should also get C2488

repro command:

cl /c test.cpp /DTARGET_X86

For current version, you will only ger error C2488 on line 2 and 4. For the upcoming version of VS, the last line will be also getting the error C2488.

[dotnet-policy-service]

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

[huoyaoyuan]

This was touched in #102834, but it was already naked then.

Does it make sense to port the naked function to asm now?

[jkotas]

Does it make sense to port the naked function to asm now?

Yes. Inline assembler has always been a factory for problems. We try to avoid inline asm for any new code.

[huoyaoyuan]

Well I want to understand more about this specific function:

runtime/src/coreclr/debug/ee/debugger.cpp

Lines 13918 to 13960 in a7efcd9

	//
	// This is the function that a thread is hijacked to by the Right Side during a variety of debug events. This function
	// must be naked.
	//
	#if defined(TARGET_X86)
	#ifdef TARGET_WINDOWS
	__declspec(naked)
	#else
	__attribute__((naked))
	#endif
	#endif // defined (_x86_)
	void Debugger::GenericHijackFunc(void)
	{
	#if defined(TARGET_X86) || defined(TARGET_AMD64)
	#if defined(TARGET_X86)
	_asm
	{
	push ebp
	mov ebp,esp
	sub esp,__LOCAL_SIZE
	}
	#endif
	// We can't have C++ classes w/ dtors in a declspec naked, so just have call into a helper.
	GenericHijackFuncHelper();
	#if defined(TARGET_X86)
	_asm
	{
	mov esp,ebp
	pop ebp
	}
	#endif
	// This signals the Right Side that this thread is ready to have its context restored.
	ExceptionNotForRuntime();
	#else
	_ASSERTE(!"@todo - port GenericHijackFunc");
	#endif // defined (_x86_)
	_ASSERTE(!"Should never get here (Debugger::GenericHijackFunc)");
	}

It just pushes and pops ebp frame around the call to GenericHijackFuncHelper. What's the requirement around this?

When written as ordinal function, it compiles to the same code of win-x64:

call GenericHijackFuncHelper
jmp ExceptionNotForRuntime

On win-arm64, the return-address saving is around the two function calls:

str lr,[sp,#-0x10]!
bl GenericHijackFuncHelper
bl ExceptionNotForRuntime
ldr lr,[sp],#0x10
ret

[jkotas]

runtime/src/coreclr/debug/di/rsthread.cpp

Lines 3879 to 3881 in a7efcd9

	// On X86 Debugger::GenericHijackFunc() ensures the stack is walkable
	// by simply using the EBP chain, therefore we can execute the hijack
	// by setting the thread's context EIP to point to this function.

comment talks about this