This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] [Windows] Include windows.h and psapi.h with lowercase
ClosedPublic

Authored by mstorsjo on Sep 11 2018, 12:17 AM.

Details

Summary

This fixes building on a case sensitive filesystem with mingw-w64 headers, where all headers are lowercase, and matches how these headers are included elsewhere in compiler-rt.

Also include these headers with angle brackets, as they are system headers.

Diff Detail

Event Timeline

mstorsjo created this revision.Sep 11 2018, 12:17 AM
Herald added a subscriber: Restricted Project. · View Herald TranscriptSep 11 2018, 12:17 AM

The existing capitalization (as in before your change) is the correct one for these headers in the Windows SDK, and it's also possible to cross-compile against the Windows SDK on a case-sensitive filesystem. However, doing that cross-compilation requires using a VFS overlay or case-correcting symlinks or ciopfs or whatever anyway, because the casing in the rest of the Windows SDK is hopelessly broken, so I'm okay with this change. I'll let @rnk confirm that it's okay to change the capitalization to the non-Windows SDK form though, and I'm also CCing @zturner since he also did cross-compilation work against the Windows SDK on Linux.

The existing capitalization (as in before your change) is the correct one for these headers in the Windows SDK,

FWIW, the second file is inconsistent in this regard as it includes windows.h but Psapi.h.

and it's also possible to cross-compile against the Windows SDK on a case-sensitive filesystem. However, doing that cross-compilation requires using a VFS overlay or case-correcting symlinks or ciopfs or whatever anyway, because the casing in the rest of the Windows SDK is hopelessly broken, so I'm okay with this change. I'll let @rnk confirm that it's okay to change the capitalization to the non-Windows SDK form though, and I'm also CCing @zturner since he also did cross-compilation work against the Windows SDK on Linux.

FWIW, I'm also occasionally doing cross-compilation with the Windows SDK (although mingw is what I'm spending most effort on), and my approach so far has been to run a script over the headers (and import libraries) to lowercase them, because they are, as you noted, hopelessly broken in their current form.

The existing capitalization (as in before your change) is the correct one for these headers in the Windows SDK, and it's also possible to cross-compile against the Windows SDK on a case-sensitive filesystem. However, doing that cross-compilation requires using a VFS overlay or case-correcting symlinks or ciopfs or whatever anyway, because the casing in the rest of the Windows SDK is hopelessly broken, so I'm okay with this change. I'll let @rnk confirm that it's okay to change the capitalization to the non-Windows SDK form though, and I'm also CCing @zturner since he also did cross-compilation work against the Windows SDK on Linux.

And fwiw about casing, within compiler-rt we already seem to have 63 occasions of lowercase windows.h and 1 occasion of lowercase psapi.h, so this change just brings these new cases in line with the existing status quo.

The day the windows sdk actually has self-consistent casing (and in case that casing isn't all lowercase), and people migrate code for cross-building towards using that spelling consistently, I guess mingw-w64 will have to start providing such casing as well in the form of symlinks.

xbolva00 added inline comments.
lib/fuzzer/FuzzerExtFunctionsDlsymWin.cpp
16

<windows.h>

mstorsjo updated this revision to Diff 164824.Sep 11 2018, 2:16 AM
mstorsjo edited the summary of this revision. (Show Details)

Updated to use angle brackets for including system headers.

smeenai accepted this revision.Sep 11 2018, 10:21 AM

All right, those are all fair arguments. LGTM.

This revision is now accepted and ready to land.Sep 11 2018, 10:21 AM
This revision was automatically updated to reflect the committed changes.