[PATCH] D40944: [profile] Enable on Solaris

[Rainer Orth via Phabricator via llvm-commits]

ro marked an inline comment as done.
ro added inline comments.


================
Comment at: lib/profile/GCDAProfiling.c:39
 #define O_BINARY 0
 #endif
 #endif
----------------
vsk wrote:
> Why not migrate O_BINARY to the common header as well?
Looking closer, I see that both MAP_FILE and O_BINARY
are problematic: InstrProfilingPort.h and/or InstrProfiling.h
are included before system headers, but such fallback definitions
need to come after those had a chance to provide the real
definitions.  To fix this, either the system header includes
need to be moved to the InstProfiling{Port,}.h files or those
need to be included after system headers.  WDYT?


================
Comment at: lib/profile/InstrProfilingUtil.c:116
+#else
+  flock(fd, LOCK_EX);
+  return 0;
----------------
vsk wrote:
> Is flock available on Windows? If not, please handle that case by reporting a warning, and document the limitation near the header declarations of lprof(Un)lockFd.
I guess so: even before my patch, it was used unconditionally
in GCDAProfiling.c which is common code.


================
Comment at: lib/profile/InstrProfilingUtil.c:134
+      if (errno == ENOLCK) {
+	return -1;
+      }
----------------
vsk wrote:
> Please clang-format your changes.
Will do: I missed this because formatting errors in may
sanitizer patches were pointed out during the build, so I assumed
this applied to all of compiler-rt (or even llvm).


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40944