[profile] Create a copy of profile filename inside of lprofCurFilename.

[Dor1s]

Summary

It seems much safer to store a copy of the filename, as environment may be
modified our moved between llvm_profile_initialize_file() and
llvm_profile_write_file() function calls.

For example, Chromium does move the environment[1], so the pointer returned by
getenv() on a program startup is not valid at the moment when the program is
closed. Due to that, LLVM profiler cannot write data to the file.

1: https://cs.chromium.org/chromium/src/services/service_manager/embedder/set_process_title_linux.cc?l=73

[Dor1s]

lib/profile/InstrProfilingFile.c

524

Is that branch really used anywhere? It seems like it doesn't. And INSTR_PROF_PROFILE_NAME_VAR is just an array of 2 chars...

I'd actually prefer to do the following:

1. remove that branch as it is not used
2. remove unsigned CopyFilenamePat argument from parseAndSetFilename and parseFilenamePattern functions
3. always use strdup(FilenamePat) and lprofCurFilename.OwnsFilenamePat = 1;, as it seems to be done only once on a program startup

But I might be missing something, so decided to start with this the least intrusive patch :)

Please let me know what you think about that.

[Dor1s]

lib/profile/InstrProfilingFile.c

524

Hm, I see, it might be specified from compiler command line. In that case, I still can either implement points 2) and 3), or pass CopyFilenamePat = 1 only for EnvFilenamePat case.

[davidxl]

I am not sure what you try to achieve here. Basically, there is no need to to use environment variable.

lib/profile/InstrProfilingFile.c

524

It is used. The compiler will generate the variable which is a weak symbol.

This is basically used when user specifies -fprofile-generate=<filename>

[Dor1s]

Thank you David for taking a look. Sorry that I didn't make my goal clear. The documentation suggests to use LLVM_PROFILE_FILE env variable if you need to specify a path for the profile. In that case, the profiler basically calls getenv("LLVM_PROFILE_FILE") and preserves the pointer returned by getenv(). Note that happens on a program startup. However, there is no warranty that the pointer still be pointing to the value of LLVM_PROFILE_FILE at the moment when __llvm_profile_write_file() is being executed (i.e. when program exits). That is a TOCTOU bug and leads to an error like the following one:

$ LLVM_PROFILE_FILE="foo.profraw" ./chrome
LLVM Profile Error: Failed to write file "oz/Projects/new/chromium/src/out/CleanCoverage/chrome": No such file or directory

[davidxl]

lgtm

Please add a comment why copy is needed.

[Dor1s]

Added a comment as requested.

[Dor1s]

David, may I ask you please to commit this patch, as I don't have Commit Access?

[Dor1s]

Landed as https://reviews.llvm.org/rL311607

lib/profile/InstrProfilingFile.c

void __llvm_profile_initialize_file(void) {
   const char *SelectedPat = NULL;
   ProfileNameSpecifier PNS = PNS_unknown;
   int hasCommandLineOverrider = (INSTR_PROF_PROFILE_NAME_VAR[0] != 0);
 
   EnvFilenamePat = getFilenamePatFromEnv();
   if (EnvFilenamePat) {
     SelectedPat = EnvFilenamePat;
     PNS = PNS_environment;
   } else if (hasCommandLineOverrider) {
     SelectedPat = INSTR_PROF_PROFILE_NAME_VAR;
     PNS = PNS_command_line;
   } else {
     SelectedPat = NULL;
     PNS = PNS_default;
   }
 
-  parseAndSetFilename(SelectedPat, PNS, 0);
+  /* Pass CopyFilenamePat = 1, to ensure that the filename would be valid at the
+   * moment when __llvm_profile_write_file() gets executed. */
+  parseAndSetFilename(SelectedPat, PNS, 1);
 }
 
 /* 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) {