[compiler-rt] Add a prefix on the windows mmap symbols

[mstorsjo]

For Windows, the compiler-rt profile library contains a polyfill reimplementation of the mmap family of functions.

Previously, the runtime library exposed those symbols like, "mmap", in the user symbol namespace. This could cause misdetections by configure scripts that check for the "mmap" function just by linking, without including headers.

Add a prefix on the symbols, and make an undeclared function static.

This fixes such an issue reported at mstorsjo/llvm-mingw#390.

[llvmbot]

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-platform-windows

Author: Martin Storsjö (mstorsjo)

Changes

For Windows, the compiler-rt profile library contains a polyfill reimplementation of the mmap family of functions.

Previously, the runtime library exposed those symbols like, "mmap", in the user symbol namespace. This could cause misdetections by configure scripts that check for the "mmap" function just by linking, without including headers.

This fixes such an issue reported at
mstorsjo/llvm-mingw#390.

---

Full diff: https://github.com/llvm/llvm-project/pull/78037.diff

1 Files Affected:

• (modified) compiler-rt/lib/profile/WindowsMMap.h (+6)

diff --git a/compiler-rt/lib/profile/WindowsMMap.h b/compiler-rt/lib/profile/WindowsMMap.h
index 68b8de2398d606..1df1a0be0b02bd 100644
--- a/compiler-rt/lib/profile/WindowsMMap.h
+++ b/compiler-rt/lib/profile/WindowsMMap.h
@@ -60,6 +60,12 @@
 # define DWORD_LO(x) (x)
 #endif
 
+#define mmap __llvm_profile_mmap
+#define munmap __llvm_profile_munmap
+#define msync __llvm_profile_msync
+#define madvise __llvm_profile_madvise
+#define flock __llvm_profile_flock
+
 void *mmap(void *start, size_t length, int prot, int flags, int fd,
            off_t offset);
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[mstorsjo]

Ping - can someone have a look at this one?

[MaskRay]

For Windows, the compiler-rt profile library contains a polyfill reimplementation of the mmap family of functions.

Perhaps add https://reviews.llvm.org/D15830 to the description?

[petrhosek]

While this solution works, it feels a bit hacky. I'd rather avoid using the standard POSIX names in the profile runtime implementation and instead provide our own runtime specific functions. Specifically, I'd define __llvm_profile_mmap etc. in compiler-rt/lib/profile/InstrProfilingInternal.h (or perhaps lprofMmap, we don't seem to be very consistent namewise). On Windows, we would use the existing polyfill implementation from compiler-rt/lib/profile/WindowsMMap.h. On POSIX systems we would just implement this as a simple wrapper around mmap (on ELF based systems it could be an alias).

[mstorsjo]

While this solution works, it feels a bit hacky. I'd rather avoid using the standard POSIX names in the profile runtime implementation and instead provide our own runtime specific functions. Specifically, I'd define __llvm_profile_mmap etc. in compiler-rt/lib/profile/InstrProfilingInternal.h (or perhaps lprofMmap, we don't seem to be very consistent namewise). On Windows, we would use the existing polyfill implementation from compiler-rt/lib/profile/WindowsMMap.h. On POSIX systems we would just implement this as a simple wrapper around mmap (on ELF based systems it could be an alias).

That certainly looks like a better final solution, but are you ok with merging this for now, and leaving that cleanup for later (possibly for someone else, I’m not sure I have time to take it on right now), to have the issue fixed in the 18.x branch?

[petrhosek]

While this solution works, it feels a bit hacky. I'd rather avoid using the standard POSIX names in the profile runtime implementation and instead provide our own runtime specific functions. Specifically, I'd define __llvm_profile_mmap etc. in compiler-rt/lib/profile/InstrProfilingInternal.h (or perhaps lprofMmap, we don't seem to be very consistent namewise). On Windows, we would use the existing polyfill implementation from compiler-rt/lib/profile/WindowsMMap.h. On POSIX systems we would just implement this as a simple wrapper around mmap (on ELF based systems it could be an alias).

That certainly looks like a better final solution, but are you ok with merging this for now, and leaving that cleanup for later (possibly for someone else, I’m not sure I have time to take it on right now), to have the issue fixed in the 18.x branch?

I'm fine landing this change and leaving the cleanup for later.

[mstorsjo]

For Windows, the compiler-rt profile library contains a polyfill reimplementation of the mmap family of functions.

Perhaps add https://reviews.llvm.org/D15830 to the description?

Sorry - I forgot about amending this in, I only noticed it now after merging :-(

[Andarwinux]

Hi, is it possible to backport this to llvm17 branch? Thanks.

[mstorsjo]

Hi, is it possible to backport this to llvm17 branch? Thanks.

There are no more releases planned from the 17.x branch, so I would say no. But the first 18.x release candidates are to be made next week.