[cmb69]

It seems reasonable to have an ASan job on Windows, especially to be able to check Windows specific code. Since the tests may take about 70 minutes, it doesn't make sense to add these for pushes, but for a nightly job that should be okay.

---

I expect some tests to fail yet (see PR #17086).

[iluuu1994]

Very nice! From what I understand, ASAN is more effective in debug builds (whether that also applies to MSVC, I don't know). However, it seems .github/scripts/windows/build_task.bat always sets --disable-debug-pack, which I assume is the equivalent of --disable-debug? In that case, it might be worth enabling debug builds for this job.

[cmb69]

Argh, completely forgot to commit this after having tested locally: a bit of a hack, but ASan builds should also pass --enable-debug-pack which is then effective. The difference between --enable-debug-pack and --enable-debug is that the former only produces debug symbol files, while the latter also disables optimizations (basically -O0) and uses debug CRT DLLs. This slows down testing without real benefits, so --enable-debug is (currently) not even supported for --enable-sanitizer under MSVC (see #16999).

PS: see also https://learn.microsoft.com/en-us/cpp/sanitizers/asan#command-prompt

[cmb69]

Cancelled the workflow since it is meaningless anyway for this PR.

[TimWolla]

I trust you on the Windows changes, the changes to the GHA workflow LGTM.

[cmb69]

Let's see how this goes tonight. :)

[iluuu1994]

@cmb69 You probably saw, but there were some failures. 🙂
https://github.com/php/php-src/actions/runs/12247662853/job/34165930799

[cmb69]

@iluuu1994, yes, these have been expected; see #17086. And sapi_windows_set_ctrl_handler.phpt likely blocked.

[cmb69]

Oh, but now I saw that the ASan job was also run for other branches than master, where it makes no sense, since --enable-sanitizer doesn't do anything there (for MSVC). Obviously, https://github.com/php/php-src/pull/17087/files#diff-0d5658b415099a82c11c03a06ca4ec765b4003a1f4b2f3f1943980a882cf8aa6R975 didn't do what I had expected.

Any hints how this can be solved?

[iluuu1994]

@cmb69 It's only a cosmetic issue, but you can add an input about whether windows should run with asan, and pass this value from root.yml depending on the version. Then you can extend the matrix.asan condition, or even use exclude: to skip the asan job.