[llvm] r241012 - Reapply "Use gethostuuid() on Mac to identify hosts for LockFileManager"
Ben Langmuir
blangmuir at apple.com
Tue Jul 7 18:22:59 PDT 2015
> On Jul 2, 2015, at 6:08 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> On Thu, Jul 2, 2015 at 9:00 AM Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>> On Jun 30, 2015, at 11:50 AM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>>
>>
>>
>> On Mon, Jun 29, 2015 at 3:50 PM Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>>> On Jun 29, 2015, at 3:28 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>>>
>>>
>>>
>>> On Mon, Jun 29, 2015 at 3:26 PM Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>>>> On Jun 29, 2015, at 3:20 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>>>>
>>>> Uh...
>>>>
>>>> Why not just check for the header and use that?
>>>
>>> gethostuuid() is defined in unistd.h on recent versions of Mac. The presence of uuid.h is necessary but not sufficient.
>>>
>>>
>>> IIRC I put in some other checks for uuid.h at one point, you might want to audit it.
>>
>> I can’t find any other use of uuid.h in llvm or clang. Do you recall where this was?
>>
>>
>> IIRC it was support, but it's been a few years.
>>
>>>
>>> That said, you can still check for the function in the available headers...
>>>
>>>> Or check for the functions you need etc at configure/cmake time?
>>>
>>> Sadly, on some platforms and/or versions where this function exists, it returns a bogus value.
>>>
>>>
>>> Released versions?
>>
>> Yes.
>>
>>
>> Whee. That said...
>>
>>>
>>> If so, this is just bogus all over the place. :\ That said you could restrict the use by known good versions or write checks to make sure that it's valid to use rather than #ifdef'ing them.
>>
>> I’m not sure I understand - are you suggesting moving the platform-specific #ifdef I’m using into the configuration stage, or are you suggesting checking the validity of the return values directly? If the latter, then unfortunately the “bad” values aren’t readily distinguishable, they’re just not “unique” enough.
>>
>>
>> Since you've got a version of it you can still make it a configure time check rather than a compile one to keep the amount of version specific ifdefs out of the file. If you need help with the autoconf side I can help, the cmake side... I'll punt to Chris B or someone. :)
>
> I took a look this morning, but it wasn’t obvious to me how I should set up such a check in either the autoconf or cmake builds. I see some checks for *tool* versions, but not OS versions. I guess one way would be to just use my existing platform-specific #if, but in a configure-time test program that #errors for unsupported cases… Is there a better way?
>
>
> Nope. Barring any way of knowing whether or not the API works other than version numbers that's the best way.
Okay, I got this working in CMake, but I’m not sure if it is ever going to work with autoconf. It appears that when autoconf invokes the compiler for a test program, it isn’t providing the needed -mmacosx-version-min flag, so the test for HAVE_GETHOSTUUID that I was trying to add always fails. And indeed I’m not sure how I would even tell the configure script about the deployment target. I don’t see any mention of deployment target in the configure scripts, or in the way we invoke configure in our builds. Looking about in the source, I see that the deployment target is instead determined by a make variable (MACOSX_DEPLOYMENT_TARGET in Makefile.rules).
Based on that, I’m inclined to leave this version check in the source file. But I’m much more familiar with the CMake build system than the autoconf one, so let me know if I’m missing something here!
Ben
>
> -eric
>
> Thanks,
>
> Ben
>
>>
>> -eric
>>
>> Ben
>>
>>>
>>> -eric
>>>
>>>>
>>>> -eric
>>>>
>>>> On Mon, Jun 29, 2015 at 3:19 PM Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
>>>> Author: benlangmuir
>>>> Date: Mon Jun 29 17:16:39 2015
>>>> New Revision: 241012
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=241012&view=rev <http://llvm.org/viewvc/llvm-project?rev=241012&view=rev>
>>>> Log:
>>>> Reapply "Use gethostuuid() on Mac to identify hosts for LockFileManager"
>>>>
>>>> Reapplies r241005 after fixing the build on non-Mac platforms. Original
>>>> commit message below.
>>>>
>>>> The hostname can be very unstable when there are many machines on the
>>>> network competing for the same name. Using the hardware UUID makes it
>>>> less likely to have collisions or to consider files written by the
>>>> current host to be owned by a different one at a later time.
>>>>
>>>> rdar://problem/21512307 <>
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/Support/LockFileManager.cpp
>>>>
>>>> Modified: llvm/trunk/lib/Support/LockFileManager.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=241012&r1=241011&r2=241012&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=241012&r1=241011&r2=241012&view=diff>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Support/LockFileManager.cpp (original)
>>>> +++ llvm/trunk/lib/Support/LockFileManager.cpp Mon Jun 29 17:16:39 2015
>>>> @@ -21,6 +21,16 @@
>>>> #if LLVM_ON_UNIX
>>>> #include <unistd.h>
>>>> #endif
>>>> +
>>>> +#if defined(__APPLE__) && defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && (__MAC_OS_X_VERSION_MIN_REQUIRED > 1050)
>>>> +#define USE_OSX_GETHOSTUUID 1
>>>> +#else
>>>> +#define USE_OSX_GETHOSTUUID 0
>>>> +#endif
>>>> +
>>>> +#if USE_OSX_GETHOSTUUID
>>>> +#include <uuid/uuid.h>
>>>> +#endif
>>>> using namespace llvm;
>>>>
>>>> /// \brief Attempt to read the lock file with the given name, if it exists.
>>>> @@ -56,14 +66,45 @@ LockFileManager::readLockFile(StringRef
>>>> return None;
>>>> }
>>>>
>>>> -bool LockFileManager::processStillExecuting(StringRef Hostname, int PID) {
>>>> +static std::error_code getHostID(SmallVectorImpl<char> &HostID) {
>>>> + HostID.clear();
>>>> +
>>>> +#if USE_OSX_GETHOSTUUID
>>>> + // On OS X, use the more stable hardware UUID instead of hostname.
>>>> + struct timespec wait = {1, 0}; // 1 second.
>>>> + uuid_t uuid;
>>>> + if (gethostuuid(uuid, &wait) != 0)
>>>> + return std::error_code(errno, std::system_category());
>>>> +
>>>> + uuid_string_t UUIDStr;
>>>> + uuid_unparse(uuid, UUIDStr);
>>>> + StringRef UUIDRef(UUIDStr);
>>>> + HostID.append(UUIDRef.begin(), UUIDRef.end());
>>>> +
>>>> +#elif LLVM_ON_UNIX
>>>> + char HostName[256];
>>>> + HostName[255] = 0;
>>>> + HostName[0] = 0;
>>>> + gethostname(HostName, 255);
>>>> + StringRef HostNameRef(HostName);
>>>> + HostID.append(HostNameRef.begin(), HostNameRef.end());
>>>> +
>>>> +#else
>>>> + StringRef Dummy("localhost");
>>>> + HostID.append(Dummy.begin(), Dummy.end());
>>>> +#endif
>>>> +
>>>> + return std::error_code();
>>>> +}
>>>> +
>>>> +bool LockFileManager::processStillExecuting(StringRef HostID, int PID) {
>>>> #if LLVM_ON_UNIX && !defined(__ANDROID__)
>>>> - char MyHostname[256];
>>>> - MyHostname[255] = 0;
>>>> - MyHostname[0] = 0;
>>>> - gethostname(MyHostname, 255);
>>>> + SmallString<256> StoredHostID;
>>>> + if (getHostID(StoredHostID))
>>>> + return true; // Conservatively assume it's executing on error.
>>>> +
>>>> // Check whether the process is dead. If so, we're done.
>>>> - if (MyHostname == Hostname && getsid(PID) == -1 && errno == ESRCH)
>>>> + if (StoredHostID == HostID && getsid(PID) == -1 && errno == ESRCH)
>>>> return false;
>>>> #endif
>>>>
>>>> @@ -126,17 +167,18 @@ LockFileManager::LockFileManager(StringR
>>>>
>>>> // Write our process ID to our unique lock file.
>>>> {
>>>> - raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
>>>> + SmallString<256> HostID;
>>>> + if (auto EC = getHostID(HostID)) {
>>>> + Error = EC;
>>>> + return;
>>>> + }
>>>>
>>>> + raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
>>>> + Out << HostID << ' ';
>>>> #if LLVM_ON_UNIX
>>>> - // FIXME: move getpid() call into LLVM
>>>> - char hostname[256];
>>>> - hostname[255] = 0;
>>>> - hostname[0] = 0;
>>>> - gethostname(hostname, 255);
>>>> - Out << hostname << ' ' << getpid();
>>>> + Out << getpid();
>>>> #else
>>>> - Out << "localhost 1";
>>>> + Out << "1";
>>>> #endif
>>>> Out.close();
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150707/8117b6d6/attachment.html>
More information about the llvm-commits
mailing list