Skip to content
This repository was archived by the owner on Apr 23, 2020. It is now read-only.

Commit b19457c

Browse files
committed
[profile] Do not cache __llvm_profile_get_filename result
When the %m filename pattern is used, the filename is unique to each image, so the cached value is wrong. It struck me that the full filename isn't something that's recomputed often, so perhaps it doesn't need to be cached at all. David Li pointed out we can go further and just hide lprofCurFilename. This may regress workflows that depend on using the set-filename API to change filenames across all loaded DSOs, but this is expected to be very rare. rdar://55137071 Differential Revision: https://reviews.llvm.org/D69137 git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@375301 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 4aae16c commit b19457c

File tree

5 files changed

+39
-18
lines changed

5 files changed

+39
-18
lines changed

lib/profile/InstrProfiling.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ int __llvm_orderfile_dump(void);
155155
*
156156
* \c Name is not copied, so it must remain valid. Passing NULL resets the
157157
* filename logic to the default behaviour.
158+
*
159+
* Note: There may be multiple copies of the profile runtime (one for each
160+
* instrumented image/DSO). This API only modifies the filename within the
161+
* copy of the runtime available to the calling image.
158162
*/
159163
void __llvm_profile_set_filename(const char *Name);
160164

@@ -173,6 +177,10 @@ void __llvm_profile_set_filename(const char *Name);
173177
* with the contents of the profiling file. If EnableMerge is zero, the runtime
174178
* may still merge the data if it would have merged for another reason (for
175179
* example, because of a %m specifier in the file name).
180+
*
181+
* Note: There may be multiple copies of the profile runtime (one for each
182+
* instrumented image/DSO). This API only modifies the file object within the
183+
* copy of the runtime available to the calling image.
176184
*/
177185
void __llvm_profile_set_file_object(FILE *File, int EnableMerge);
178186

@@ -196,7 +204,12 @@ const char *__llvm_profile_get_path_prefix();
196204
* \brief Return filename (including path) of the profile data. Note that if the
197205
* user calls __llvm_profile_set_filename later after invoking this interface,
198206
* the actual file name may differ from what is returned here.
199-
* Side-effect: this API call will invoke malloc with dynamic memory allocation.
207+
* Side-effect: this API call will invoke malloc with dynamic memory allocation
208+
* (the returned pointer must be passed to `free` to avoid a leak).
209+
*
210+
* Note: There may be multiple copies of the profile runtime (one for each
211+
* instrumented image/DSO). This API only retrieves the filename from the copy
212+
* of the runtime available to the calling image.
200213
*/
201214
const char *__llvm_profile_get_filename();
202215

lib/profile/InstrProfilingFile.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ typedef struct lprofFilename {
7070
* by runtime. */
7171
unsigned OwnsFilenamePat;
7272
const char *ProfilePathPrefix;
73-
const char *Filename;
7473
char PidChars[MAX_PID_SIZE];
7574
char Hostname[COMPILER_RT_MAX_HOSTLEN];
7675
unsigned NumPids;
@@ -86,8 +85,8 @@ typedef struct lprofFilename {
8685
ProfileNameSpecifier PNS;
8786
} lprofFilename;
8887

89-
COMPILER_RT_WEAK lprofFilename lprofCurFilename = {0, 0, 0, 0, {0},
90-
{0}, 0, 0, 0, PNS_unknown};
88+
static lprofFilename lprofCurFilename = {0, 0, 0, {0}, {0},
89+
0, 0, 0, PNS_unknown};
9190

9291
static int ProfileMergeRequested = 0;
9392
static int isProfileMergeRequested() { return ProfileMergeRequested; }
@@ -387,8 +386,6 @@ static int parseFilenamePattern(const char *FilenamePat,
387386
/* Clean up cached prefix and filename. */
388387
if (lprofCurFilename.ProfilePathPrefix)
389388
free((void *)lprofCurFilename.ProfilePathPrefix);
390-
if (lprofCurFilename.Filename)
391-
free((void *)lprofCurFilename.Filename);
392389

393390
if (lprofCurFilename.FilenamePat && lprofCurFilename.OwnsFilenamePat) {
394391
free((void *)lprofCurFilename.FilenamePat);
@@ -602,9 +599,6 @@ const char *__llvm_profile_get_filename(void) {
602599
char *FilenameBuf;
603600
const char *Filename;
604601

605-
if (lprofCurFilename.Filename)
606-
return lprofCurFilename.Filename;
607-
608602
Length = getCurFilenameLength();
609603
FilenameBuf = (char *)malloc(Length + 1);
610604
if (!FilenameBuf) {
@@ -615,7 +609,6 @@ const char *__llvm_profile_get_filename(void) {
615609
if (!Filename)
616610
return "\0";
617611

618-
lprofCurFilename.Filename = FilenameBuf;
619612
return FilenameBuf;
620613
}
621614

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const char *__llvm_profile_get_filename(void);
2+
3+
const char *get_filename_from_DSO(void) {
4+
return __llvm_profile_get_filename();
5+
}

test/profile/Posix/instrprof-set-filename-shared.test

Lines changed: 0 additions & 8 deletions
This file was deleted.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Test __llvm_profile_get_filename when the on-line merging mode is enabled.
2+
//
3+
// RUN: %clang_pgogen -dynamiclib -o %t.dso %p/Inputs/instrprof-get-filename-dso.c
4+
// RUN: %clang_pgogen -o %t %s %t.dso
5+
// RUN: env LLVM_PROFILE_FILE="%t-%m.profraw" %run %t
6+
7+
#include <string.h>
8+
9+
const char *__llvm_profile_get_filename(void);
10+
extern const char *get_filename_from_DSO(void);
11+
12+
int main(int argc, const char *argv[]) {
13+
const char *filename1 = __llvm_profile_get_filename();
14+
const char *filename2 = get_filename_from_DSO();
15+
16+
// Exit with code 1 if the two filenames are the same.
17+
return strcmp(filename1, filename2) == 0;
18+
}

0 commit comments

Comments
 (0)