add profile flag when linking to fix apiscan errors

[AdamYoblick]

Fixes #1547

APIScan logs are reporting that the following two files are invalid binaries:

The executable image d:\a\_work\1\.gdn\.r\apiscan\001\logs\api0000_debugpy_20240_9427092\temp\x\]5\inject_dll_x86.exe or its symbol file is incorrect.
ENDMESSAGE

The executable image d:\a\_work\1\.gdn\.r\apiscan\001\logs\api0000_debugpy_20240_9427092\temp\x\]5\inject_dll_amd64.exe or its symbol file is incorrect.
ENDMESSAGE

Reading the docs at https://eng.ms/docs/products/apiscan/howto/preparinginput/binaries/creating_vulcan_ready_files, we are supposed to link with the /PROFILE flag (or a combination of three other flags). I've modified the flags for the two failing binaries, and created this PR. The PR checks perform a compile of the binaries, so I downloaded the windows artifacts from https://github.com/microsoft/debugpy/actions/runs/8729118226?pr=1567. Then I committed those binaries back into the repo.

I'm not entire sure how to test these changes yet. Need to investigate that before merging. @int19h Do you know how I can test these re-compiled "inject" binaries?

@rchiodo @debonte - I'm not a C++ expert, but I read through the docs. Do either of you see any issues with the flag changes I made?

[rchiodo]

Did you remove these flags on purpose? I believe these flags generate a DEBUG dll. Did we need/want a release mode DLL now?

[AdamYoblick]

Yeah you're right. I guess I need both /DEBUG and /PROFILE

[rchiodo]

Oh nevermind. I guess /PROFILE implies this:
https://learn.microsoft.com/en-us/cpp/build/reference/profile-performance-tools-profiler?view=msvc-170#remarks

[heejaechang]

https://learn.microsoft.com/en-us/cpp/build/reference/debug-generate-debug-info?view=msvc-170

without /DEBUG it won't generate pdb so you won't be able to debug the dll once it generated. I mean at least on source level.

[heejaechang]

/OPT:REF and /OPT:ICF seems optimization options to reduce dll file size. so I guess it could be removed if that doesn't work as expected?

[AdamYoblick]

Oh nevermind. I guess /PROFILE implies this: learn.microsoft.com/en-us/cpp/build/reference/profile-performance-tools-profiler?view=msvc-170#remarks

Yes! I remember reading that and I forgot why I removed debug, that's why!

[heejaechang]

isn't /PROFILE for profiling? I mean when you want to inject profiling stub on each symbols to gather profiling info. I thought those are usually not used unless dll is specificallly built for profiling.

[AdamYoblick]

According to https://eng.ms/docs/products/apiscan/howto/preparinginput/binaries/creating_vulcan_ready_files, we have two options for APIScan to work:

Linking/link.exe
/profile (https://learn.microsoft.com/en-us/cpp/build/reference/profile-performance-tools-profiler)
This informs the linker to emit full fixup information so that Vulcancompletely identifies code and data cross-references. Be aware thatthis implies the following linker switches that affect codegeneration: /incremental:no /opt:ref /opt:noicf. The /opt switchescan be overridden on the command-line after /profile is used.

Optionally for non public source bases the following switches can be used instead of /profile:

/debug:full (https://learn.microsoft.com/en-us/cpp/build/reference/debug-generate-debug-info)

/debugtype:cv,fixup (https://learn.microsoft.com/en-us/cpp/build/reference/debugtype-debug-info-options)

/incremental:no (https://learn.microsoft.com/en-us/cpp/build/reference/incremental-link-incrementally)

So I can use the three debug switches instead of profile, It doesn't matter to me. 😄

[rchiodo]

No read this:
https://learn.microsoft.com/en-us/cpp/build/reference/profile-performance-tools-profiler?view=msvc-170#remarks

/PROFILE implies /DEBUG:full

[heejaechang]

surprised that /PROFILE flag is recommended to be enabled by default. I guess now it generates fast enough code to okay to be enabled by default.

[karthiknadig]

@AdamYoblick To test this you can run some python script in a loop with a timer. Then use the process ID of that process and attach to it from VS Code. You can set the debugAdapterPath to your local build with the new binaries.

[int19h]

We have test_attach_pid_client. It has been disabled in CI because of flakiness a long time ago, but we might want to revisit that. Either way, it should be possible to run it locally, as many times as desired.

[AdamYoblick]

We have test_attach_pid_client. It has been disabled in CI because of flakiness a long time ago, but we might want to revisit that. Either way, it should be possible to run it locally, as many times as desired.

Thanks Pavel. How can I ensure that the inject* dlls that I built are used? Are they always used in the attach scenario?

[AdamYoblick]

Ok I tested this by writing an infinite loop with a timer, then using the following attach configuration in launch.json in vscode:

{
      "name": "Python Debugger: Attach using Process Id",
      "type": "debugpy",
      "request": "attach",
      "processId": "${command:pickProcess}",
      "debugAdapterPath": "E:/repos/github/debugpy/src/debugpy/adapter"
    },

I was able to attach (with python 3.11 as 3.12 is not supported in the attach code yet) and set breakpoints and everything worked as expected.

[AdamYoblick]

The next step after merging this in is to run the debugpy compliance pipeline to see if apiscan still reports errors on these binaries.