[compiler-rt] r272081 - [profile] Hide a few external symbols (NFCI)

[Xinliang David Li via llvm-commits]

yes -- this is a good point.

David

On Tue, Jun 7, 2016 at 9:33 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> On Tue, Jun 7, 2016 at 6:50 PM, Vedant Kumar via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>>
>> > On Jun 7, 2016, at 6:29 PM, Xinliang David Li <xinliangli at gmail.com>
>> wrote:
>> >
>> >
>> >
>> > On Tue, Jun 7, 2016 at 5:44 PM, Vedant Kumar via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>> > Author: vedantk
>> > Date: Tue Jun  7 19:44:38 2016
>> > New Revision: 272081
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=272081&view=rev
>> > Log:
>> > [profile] Hide a few external symbols (NFCI)
>> >
>> > There are still a few external symbols visible from InstrProfData.inc.
>> > The plan for dealing with those isn't as straightforward, so I'll try it
>> > in a separate commit.
>> >
>> > Modified:
>> >     compiler-rt/trunk/lib/profile/InstrProfiling.c
>> >     compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>> >     compiler-rt/trunk/lib/profile/InstrProfilingUtil.c
>> >
>> > Modified: compiler-rt/trunk/lib/profile/InstrProfiling.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfiling.c?rev=272081&r1=272080&r2=272081&view=diff
>> >
>> ==============================================================================
>> > --- compiler-rt/trunk/lib/profile/InstrProfiling.c (original)
>> > +++ compiler-rt/trunk/lib/profile/InstrProfiling.c Tue Jun  7 19:44:38
>> 2016
>> > @@ -18,7 +18,8 @@
>> >
>> >  COMPILER_RT_VISIBILITY char *(*GetEnvHook)(const char *) = 0;
>> >
>> > -COMPILER_RT_WEAK uint64_t __llvm_profile_raw_version =
>> INSTR_PROF_RAW_VERSION;
>> > +COMPILER_RT_VISIBILITY COMPILER_RT_WEAK uint64_t
>> __llvm_profile_raw_version =
>> > +    INSTR_PROF_RAW_VERSION;
>> >
>> > There is no need for this one to be hidden (see comments in
>> InstrProfiling.h).
>> >
>> >
>> >  COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_magic(void) {
>> >    return sizeof(void *) == sizeof(uint64_t) ? (INSTR_PROF_RAW_MAGIC_64)
>> >
>> > Modified: compiler-rt/trunk/lib/profile/InstrProfilingFile.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingFile.c?rev=272081&r1=272080&r2=272081&view=diff
>> >
>> ==============================================================================
>> > --- compiler-rt/trunk/lib/profile/InstrProfilingFile.c (original)
>> > +++ compiler-rt/trunk/lib/profile/InstrProfilingFile.c Tue Jun  7
>> 19:44:38 2016
>> > @@ -30,7 +30,7 @@ typedef struct lprofFilename {
>> >    unsigned NumHosts;
>> >  } lprofFilename;
>> >
>> > -lprofFilename lprofCurFilename = {0, {0}, {0}, 0, 0};
>> > +COMPILER_RT_VISIBILITY lprofFilename lprofCurFilename = {0, {0}, {0},
>> 0, 0};
>> >
>> > This changes the behavior -- the original
>> __llvm_profile_CurrentFilename was intended to be non-hidden -- so that
>> share libraries and main program can share the same file name. It is
>> debatable whether this is a good behavior, but it may require a separate
>> discussion.
>>
>> Rolled back in r272089.
>>
>> I'm surprised that this change didn't break check-profile on my machine.
>
>
> We probably don't have any tests that use __llvm_profile_set_filename in
> conjunction with shared libraries. Note that LLVM_PROFILE_FILE implicitly
> has the "all shared libraries pick up the same file name" behavior due to
> global coupling in getenv.
>
> -- Sean Silva
>
>
>> Could you explain how we're relying on lprofCurFilename having default
>> visibility -- i.e how does the sharing happen exactly? I was under the
>> impression that instrumented shared libraries received their own copies of
>> lprofCurFilename.
>>
>> vedant
>>
>> >
>> > David
>> >
>> >
>> >  int getpid(void);
>> >  static int getCurFilenameLength();
>> >
>> > Modified: compiler-rt/trunk/lib/profile/InstrProfilingUtil.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingUtil.c?rev=272081&r1=272080&r2=272081&view=diff
>> >
>> ==============================================================================
>> > --- compiler-rt/trunk/lib/profile/InstrProfilingUtil.c (original)
>> > +++ compiler-rt/trunk/lib/profile/InstrProfilingUtil.c Tue Jun  7
>> 19:44:38 2016
>> > @@ -67,7 +67,7 @@ void *lprofPtrFetchAdd(void **Mem, long
>> >  #endif
>> >
>> >  #ifdef COMPILER_RT_HAS_UNAME
>> > -int lprofGetHostName(char *Name, int Len) {
>> > +COMPILER_RT_VISIBILITY int lprofGetHostName(char *Name, int Len) {
>> >    struct utsname N;
>> >    int R;
>> >    if (!(R = uname(&N)))
>> > @@ -76,7 +76,7 @@ int lprofGetHostName(char *Name, int Len
>> >  }
>> >  #endif
>> >
>> > -FILE *lprofOpenFileEx(const char *ProfileName) {
>> > +COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
>> >    FILE *f;
>> >    int fd;
>> >  #ifdef COMPILER_RT_HAS_FCNTL_LCK
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160607/b4929a05/attachment.html>