This is an archive of the discontinued LLVM Phabricator instance.

[Support] Reimplement getMainExecutable() using sysctl on NetBSD
AbandonedPublic

Authored by mgorny on Jan 20 2019, 5:13 AM.

Details

Summary

Use sysctl() to implement getMainExecutable() on NetBSD, rather than
trying to guess the correct path from argv[0]. This is one
of the fixes to recent clang-check-mac-libcxx-fixed-compilation-db.cpp
test failure on NetBSD.

This has been historically done on both FreeBSD and NetBSD in r303015,
and reverted in r303285 due to buggy implementation on FreeBSD.
However, FWIK the NetBSD implementation does not suffer from the same
bugs and is more reliable than playing with argv[0].

Diff Detail

Event Timeline

mgorny created this revision.Jan 20 2019, 5:13 AM
krytarowski accepted this revision.Jan 20 2019, 5:16 AM
krytarowski added inline comments.
lib/Support/Unix/Path.inc
178

Please include a comment with a rationale (hardlink issue) that this cannot be used on FreeBSD.

This revision is now accepted and ready to land.Jan 20 2019, 5:16 AM
mgorny updated this revision to Diff 182699.Jan 20 2019, 5:25 AM

Added FreeBSD warning comment.

mgorny marked an inline comment as done.Jan 20 2019, 5:25 AM
dim accepted this revision.Jan 20 2019, 6:07 AM

I can't vouch for the NetBSD part, but this definitely seems like a good idea to me. I'm unsure if the sysctl is already fixed on the FreeBSD side. @emaste any idea?

joerg added a comment.Jan 21 2019, 1:25 AM

I'd really prefer to keep the argv[0] code as is. I'm not sure what that test case is supposed to do, but it seems quite questionable as "check" is not a valid language frontend nor a version suffix. It should not work.

I'd really prefer to keep the argv[0] code as is. I'm not sure what that test case is supposed to do, but it seems quite questionable as "check" is not a valid language frontend nor a version suffix. It should not work.

Related D56976. A broken program/test does not pass argv0.

In D56975#1364533, @dim wrote:

I can't vouch for the NetBSD part, but this definitely seems like a good idea to me. I'm unsure if the sysctl is already fixed on the FreeBSD side. @emaste any idea?

Does not seem to be the case IMHO.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2019, 2:06 AM
Herald added a subscriber: jdoerfert. · View Herald Transcript
mgorny reopened this revision.Mar 3 2019, 8:53 PM

Reverted in r355302. It seems that it is unreliable and sometimes evaluates to empty path, breaking clang.

This revision is now accepted and ready to land.Mar 3 2019, 8:53 PM
mgorny abandoned this revision.Mar 3 2019, 8:54 PM

I'm not going to pursue this further.