[profile] Cleanup profile file path set up code

[davidxl]

Summary

Consolidate the logic (scattered in multiple different places) into one function. Also added note messages to make it more friendly to the user (e.g. debugging problem that the profile is not dumped in a place expected ..)

Regression tests were added to ensure no behavioral change.

[silvas]

Nice! This really clarifies the code! LGTM

[MatzeB]

compiler-rt/trunk/lib/profile/InstrProfilingFile.c

339–346 ↗

(On Diff #60395)

So this is breaking our test-suite. Adding extra output to stdout/stderr based on profile settings messes up tests that look for a certain output of a process. Can we remove those notes again?

lib/profile/InstrProfilingFile.c

 #include <sys/file.h>
 #include <sys/mman.h>
 #include <unistd.h>
 #if defined(__linux__)
 #include <sys/types.h>
 #endif
 #endif
 
+/* From where is profile name specified.  */
+typedef enum ProfileNameSpecifier {
+  PNS_unknown = 0,
+  PNS_default,
+  PNS_command_line,
+  PNS_environment,
+  PNS_runtime_api
+} ProfileNameSpecifier;
+
+static const char *getPNSStr(ProfileNameSpecifier PNS) {
+  switch (PNS) {
+  case PNS_default:
+    return "default setting";
+  case PNS_command_line:
+    return "command line";
+  case PNS_environment:
+    return "environment variable";
+  case PNS_runtime_api:
+    return "runtime API";
+  default:
+  case PNS_unknown:
+    return "Unknown";
+  }
+}
+
 #define MAX_PID_SIZE 16
 /* Data structure holding the result of parsed filename pattern.  */
 typedef struct lprofFilename {
   /* File name string possibly with %p or %h specifiers. */
   const char *FilenamePat;
   char PidChars[MAX_PID_SIZE];
   char Hostname[COMPILER_RT_MAX_HOSTLEN];
   unsigned NumPids;
   unsigned NumHosts;
   /* When in-process merging is enabled, this parameter specifies
    * the total number of profile data files shared by all the processes
    * spawned from the same binary. By default the value is 1. If merging
    * is not enabled, its value should be 0. This parameter is specified
    * by the %[0-9]m specifier. For instance %2m enables merging using
    * 2 profile data files. %1m is equivalent to %m. Also %m specifier
    * can only appear once at the end of the name pattern. */
   unsigned MergePoolSize;
+  ProfileNameSpecifier PNS;
 } lprofFilename;
 
-lprofFilename lprofCurFilename = {0, {0}, {0}, 0, 0, 0};
+lprofFilename lprofCurFilename = {0, {0}, {0}, 0, 0, 0, PNS_unknown};
 
 int getpid(void);
 static int getCurFilenameLength();
 static const char *getCurFilename(char *FilenameBuf);
 static unsigned doMerging() { return lprofCurFilename.MergePoolSize; }
 
 /* Return 1 if there is an error, otherwise return  0.  */
 static uint32_t fileWriter(ProfDataIOVec *IOVecs, uint32_t NumIOVecs,
static void truncateCurrentFile(void) {
 
   /* Truncate the file.  Later we'll reopen and append. */
   File = fopen(Filename, "w");
   if (!File)
     return;
   fclose(File);
 }
 
+static const char *DefaultProfileName = "default.profraw";
 static void resetFilenameToDefault(void) {
   memset(&lprofCurFilename, 0, sizeof(lprofCurFilename));
-  lprofCurFilename.FilenamePat = "default.profraw";
+  lprofCurFilename.FilenamePat = DefaultProfileName;
+  lprofCurFilename.PNS = PNS_default;
 }
 
 static int containsMergeSpecifier(const char *FilenamePat, int I) {
   return (FilenamePat[I] == 'm' ||
           (FilenamePat[I] >= '1' && FilenamePat[I] <= '9' &&
            /* If FilenamePat[I] is not '\0', the next byte is guaranteed
             * to be in-bound as the string is null terminated. */
            FilenamePat[I + 1] == 'm'));
if (FilenamePat[I] == '%') {
       }
     }
 
   lprofCurFilename.NumPids = NumPids;
   lprofCurFilename.NumHosts = NumHosts;
   return 0;
 }
 
-static void parseAndSetFilename(const char *FilenamePat) {
-  int NewFile;
+static void parseAndSetFilename(const char *FilenamePat,
+                                ProfileNameSpecifier PNS) {
+
   const char *OldFilenamePat = lprofCurFilename.FilenamePat;
+  ProfileNameSpecifier OldPNS = lprofCurFilename.PNS;
 
+  if (PNS < OldPNS)
+    return;
+
+  if (!FilenamePat)
+    FilenamePat = DefaultProfileName;
+
+  /* When -fprofile-instr-generate=<path> is specified on the
+   * command line, each module will be instrumented with runtime
+   * init call to __llvm_profile_init function which calls
+   * __llvm_profile_override_default_filename. In most of the cases,
+   * the path will be identical, so bypass the parsing completely.
+   */
+  if (OldFilenamePat && !strcmp(OldFilenamePat, FilenamePat)) {
+    lprofCurFilename.PNS = PNS;
+    return;
+  }
+
+  /* When PNS >= OldPNS, the last one wins. */
   if (!FilenamePat || parseFilenamePattern(FilenamePat))
     resetFilenameToDefault();
+  lprofCurFilename.PNS = PNS;
 
-  NewFile =
-      !OldFilenamePat || (strcmp(OldFilenamePat, lprofCurFilename.FilenamePat));
+  if (!OldFilenamePat) {
+    PROF_NOTE("Set profile file path to \"%s\" via %s.\n",
+              lprofCurFilename.FilenamePat, getPNSStr(PNS));
+  } else {
+    PROF_NOTE("Override old profile path \"%s\" via %s to \"%s\" via %s.\n",
+              OldFilenamePat, getPNSStr(OldPNS), lprofCurFilename.FilenamePat,
+              getPNSStr(PNS));
+  }
 
-  if (NewFile && !lprofCurFilename.MergePoolSize)
+  if (!lprofCurFilename.MergePoolSize)
     truncateCurrentFile();
 }
 
 /* Return buffer length that is required to store the current profile
  * filename with PID and hostname substitutions. */
 /* The length to hold uint64_t followed by 2 digit pool id including '_' */
 #define SIGLEN 24
 static int getCurFilenameLength() {

 
 /* This method is invoked by the runtime initialization hook
  * InstrProfilingRuntime.o if it is linked in. Both user specified
  * profile path via -fprofile-instr-generate= and LLVM_PROFILE_FILE
  * environment variable can override this default value. */
 COMPILER_RT_VISIBILITY
 void __llvm_profile_initialize_file(void) {
   const char *FilenamePat;
-  /* Check if the filename has been initialized. */
-  if (lprofCurFilename.FilenamePat)
-    return;
 
-  /* Detect the filename and truncate. */
   FilenamePat = getFilenamePatFromEnv();
-  parseAndSetFilename(FilenamePat);
+  parseAndSetFilename(FilenamePat, FilenamePat ? PNS_environment : PNS_default);
 }
 
 /* This API is directly called by the user application code. It has the
  * highest precedence compared with LLVM_PROFILE_FILE environment variable
  * and command line option -fprofile-instr-generate=<profile_name>.
  */
 COMPILER_RT_VISIBILITY
 void __llvm_profile_set_filename(const char *FilenamePat) {
-  parseAndSetFilename(FilenamePat);
+  parseAndSetFilename(FilenamePat, PNS_runtime_api);
 }
 
 /*
  * This API is invoked by the global initializers emitted by Clang/LLVM when
  * -fprofile-instr-generate=<..> is specified (vs -fprofile-instr-generate
  *  without an argument). This option has lower precedence than the
  *  LLVM_PROFILE_FILE environment variable.
  */
 COMPILER_RT_VISIBILITY
 void __llvm_profile_override_default_filename(const char *FilenamePat) {
-  /* If the env var is set, skip setting filename from argument. */
-  const char *Env_Filename = getFilenamePatFromEnv();
-  if (Env_Filename)
-    return;
-
-  parseAndSetFilename(FilenamePat);
+  parseAndSetFilename(FilenamePat, PNS_command_line);
 }
 
 /* The public API for writing profile data into the file with name
  * set by previous calls to __llvm_profile_set_filename or
  * __llvm_profile_override_default_filename or
  * __llvm_profile_initialize_file. */
 COMPILER_RT_VISIBILITY
 int __llvm_profile_write_file(void) {

lib/profile/InstrProfilingPort.h

 #endif
 
 #define PROF_ERR(Format, ...)                                                  \
   fprintf(stderr, "LLVM Profile Error: " Format, __VA_ARGS__);
 
 #define PROF_WARN(Format, ...)                                                 \
   fprintf(stderr, "LLVM Profile Warning: " Format, __VA_ARGS__);
 
+#define PROF_NOTE(Format, ...)                                                 \
+  fprintf(stderr, "LLVM Profile Note: " Format, __VA_ARGS__);
+
 #if defined(__FreeBSD__)
 
 #include <inttypes.h>
 #include <sys/types.h>
 
 #else /* defined(__FreeBSD__) */
 
 #include <inttypes.h>
 #include <stdint.h>
 
 #endif /* defined(__FreeBSD__) && defined(__i386__) */
 
 #endif /* PROFILE_INSTRPROFILING_PORT_H_ */