Fix driver-related things for FreeBSD

[kutuzov.viktor.84]

Summary

This patch adds libcxxrt when clang is invoked with -stdlib=libc++ on FreeBSD (which is necessary to build llvm against libc++ on FreeBSD 9.2). It also adds the default path to the libc++ headers (which is /usr/local/include/c++/v1 and not /usr/include/c++/v1 as clang currently expect). Furthermore, it fixes the path to the clang's headers (which is /usr/include/clang/X.Y.Z and not <resource-dir>/include).

[kutuzov.viktor.84]

Ed, this patch refers to /usr/include/clang/X.Y.Z while the current practice is /usr/include/clang/X.Y . Do you think we can fix FreeBSD so there is no clash if there are several versions of clang installed that only differ in the minor number?

[• tpoechtrager]

Hi Viktor,

• {F53971, layout=link}

[dim]

Since the mailing list reply seems to have eaten most of my comments, I'll repost them here.

Index: lib/Driver/ToolChains.cpp
===================================================================
--- lib/Driver/ToolChains.cpp
+++ lib/Driver/ToolChains.cpp
@@ -2490,6 +2490,9 @@
 case ToolChain::CST_Libcxx:
   addSystemInclude(DriverArgs, CC1Args,
                    getDriver().SysRoot + "/usr/include/c++/v1");
+    // The libc++ port installs headers to /usr/local by default.
+    addSystemInclude(DriverArgs, CC1Args,
+                     getDriver().SysRoot + "/usr/local/include/c++/v1");
   break;
 case ToolChain::CST_Libstdcxx:
   addSystemInclude(DriverArgs, CC1Args,
@@ -2530,6 +2533,19 @@
 return getSanitizerArgs().hasZeroBaseShadow();
}

Paths to /usr/local should never be in the default compiler include path on FreeBSD. This location is reserved for ports usage, and ports are supposed to add the correct paths using -I by themselves.

There is already a devel/libc++ port for the (older) FreeBSD releases which do not include libc++, and this also arranges for other dependent port to know where it installs the libc++ headers. See http://svnweb.freebsd.org/ports/head/devel/libc%2B%2B/Makefile?view=markup

+void FreeBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+                                 ArgStringList &CmdArgs) const {
+  switch (GetCXXStdlibType(Args)) {
+  case ToolChain::CST_Libcxx:
+    CmdArgs.push_back("-lc++");
+    CmdArgs.push_back("-lcxxrt");
+    break;
+  case ToolChain::CST_Libstdcxx:
+    CmdArgs.push_back("-lstdc++");
+    break;
+  }
+}

This part is not necessary, it is already taken care of via FreeBSD::GetCXXStdlibType() in lib/Driver/ToolChains.cpp, and ToolChain::AddCXXStdlibLibArgs() in lib/Driver/ToolChain.cpp.

In FreeBSD, we provide libc++.so as a linker script, which automatically pulls in the required runtime support, by default that is /usr/lib/libcxxrt.so:

$ cat /usr/lib/libc++.so
/* $FreeBSD: stable/9/lib/libc++/libc++.ldscript 258060 2013-11-12 18:43:35Z dim $ */
GROUP ( /usr/lib/libc++.so.1 /usr/lib/libcxxrt.so )

This makes it easy for the end user to switch to another C++ runtime library, if desired. If you would hardcode it in the compiler, this becomes much more difficult.

NetBSD::NetBSD(const Driver &D, const llvm::Triple& Triple, const ArgList &Args)
Index: lib/Driver/ToolChains.h
===================================================================
--- lib/Driver/ToolChains.h
+++ lib/Driver/ToolChains.h
@@ -584,6 +584,8 @@
 void
 AddClangCXXStdlibIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                              llvm::opt::ArgStringList &CC1Args) const override;
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+                           llvm::opt::ArgStringList &CmdArgs) const override;
 bool IsIntegratedAssemblerDefault() const override {
   if (getTriple().getArch() == llvm::Triple::ppc ||
       getTriple().getArch() == llvm::Triple::ppc64)

Similar to the above hunk, this is already handled.

Index: lib/Frontend/InitHeaderSearch.cpp
===================================================================
--- lib/Frontend/InitHeaderSearch.cpp
+++ lib/Frontend/InitHeaderSearch.cpp
@@ -14,6 +14,7 @@
#include "clang/Frontend/Utils.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/Version.h"
#include "clang/Config/config.h" // C_INCLUDE_DIRS
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/HeaderSearchOptions.h"
@@ -242,11 +243,16 @@
 // Builtin includes use #include_next directives and should be positioned
 // just prior C include dirs.
 if (HSOpts.UseBuiltinIncludes) {
-    // Ignore the sys root, we *always* look for clang headers relative to
-    // supplied path.
-    SmallString<128> P = StringRef(HSOpts.ResourceDir);
-    llvm::sys::path::append(P, "include");
-    AddUnmappedPath(P.str(), ExternCSystem, false);
+    // On FreeBSD the include path is not resource-dir-relative.
+    if (os == llvm::Triple::FreeBSD) {
+      AddPath("/usr/include/clang/" CLANG_VERSION_STRING, ExternCSystem, false);
+    } else {
+      // Ignore the sys root, we *always* look for clang headers relative to
+      // supplied path.
+      SmallString<128> P = StringRef(HSOpts.ResourceDir);
+      llvm::sys::path::append(P, "include");
+      AddUnmappedPath(P.str(), ExternCSystem, false);
+    }

For this specific part, we have been using the following patch for years now:

http://svnweb.freebsd.org/base/head/contrib/llvm/patches/patch-r208961-clang-version-include.diff?view=co&revision=263320&content-type=text%2Fplain

Please use that instead. If newer versions of clang use x.y.z as version numbering scheme, we should probably chop off the .z part (the patch version), if the internal headers are supposed to be compatible across patch versions.

[kutuzov.viktor.84]

Updated to address concerns mentioned by the FreeBSD community:

• no need for the /usr/local/include include path;
• no need to add -lcxxrt as newcoming FreeBSD versions (9.3+ and 10.0+) take care of this by providing libc++.so in the form of a linker script that refers to the corresponding run-time library; the note is that there's still a way to pull the -lcxxrt option with the CMAKE_EXE_LINKER_FLAGS variable on FreeBSD 9.2, so not a problem;
• switch the path for internal clang headers from /usr/include/clang/X.Y.Z to /usr/include/clang/X.Y.

Also, I propose that we do not add the resource-dir-related include path on FreeBSD unless the FreeBSD's directory structure is reworked respectively. (Currently, we add an include path that should never exist on FreeBSD.)

[kutuzov.viktor.84]

Ping.

[compnerd]

include/clang/Basic/Version.h

26–27

Rather than using the non-descript XYZ, could you change this to MAJOR_MINOR_PATCH please?

30–37

Similariy, MAJOR_MINOR.

[kutuzov.viktor.84]

Two changes:

• the clang version macros are renamed as suggested and
• the resource-dir-relative include path reverted back as LLVM rely on it when build the sanitizers tests.

Thanks for reviewing.

[compnerd]

LGTM

lib/Frontend/InitHeaderSearch.cpp

247

The braces aren't necessary.

[Eugene.Zelenko]

Looks like patch was not committed.

[dim]

In D3381#560100, @Eugene.Zelenko wrote:

Looks like patch was not committed.

Indeed, please don't commit it at all. In FreeBSD, we've now switched to the default layout for internal includes, e.g. we now install them in /usr/lib/clang/x.y.z/include, like on every other platform. (Note that this caused quite some grumbling because people like to find headers easily, but we pointed out the gcc does something similar.)

include/clang/Basic/Version.h

 #define LLVM_CLANG_BASIC_VERSION_H
 
 #include "clang/Basic/Version.inc"
 #include "llvm/ADT/StringRef.h"
 
 /// \brief Helper macro for CLANG_VERSION_STRING.
 #define CLANG_MAKE_VERSION_STRING2(X) #X
 
-#ifdef CLANG_VERSION_PATCHLEVEL
 /// \brief Helper macro for CLANG_VERSION_STRING.
-#define CLANG_MAKE_VERSION_STRING(X,Y,Z) CLANG_MAKE_VERSION_STRING2(X.Y.Z)
+#define CLANG_MAKE_VERSION_STRING_MAJOR_MINOR_PATCH(X,Y,Z) \
+  CLANG_MAKE_VERSION_STRING2(X.Y.Z)
 
-/// \brief A string that describes the Clang version number, e.g., "1.0".
-#define CLANG_VERSION_STRING \
-  CLANG_MAKE_VERSION_STRING(CLANG_VERSION_MAJOR,CLANG_VERSION_MINOR, \
-                            CLANG_VERSION_PATCHLEVEL)
-#else
 /// \brief Helper macro for CLANG_VERSION_STRING.
-#define CLANG_MAKE_VERSION_STRING(X,Y) CLANG_MAKE_VERSION_STRING2(X.Y)
+#define CLANG_MAKE_VERSION_STRING_MAJOR_MINOR(X,Y) \
+  CLANG_MAKE_VERSION_STRING2(X.Y)
+
+/// \brief A string that describes the Clang version number, e.g., "1.0.0".
+#define CLANG_VERSION_STRING_MAJOR_MINOR_PATCH \
+  CLANG_MAKE_VERSION_STRING_MAJOR_MINOR_PATCH(CLANG_VERSION_MAJOR, \
+                                              CLANG_VERSION_MINOR, \
+                                              CLANG_VERSION_PATCHLEVEL)
 
 /// \brief A string that describes the Clang version number, e.g., "1.0".
-#define CLANG_VERSION_STRING \
-  CLANG_MAKE_VERSION_STRING(CLANG_VERSION_MAJOR,CLANG_VERSION_MINOR)
+#define CLANG_VERSION_STRING_MAJOR_MINOR \
+  CLANG_MAKE_VERSION_STRING_MAJOR_MINOR(CLANG_VERSION_MAJOR,CLANG_VERSION_MINOR)
+
+#ifdef CLANG_VERSION_PATCHLEVEL
+#define CLANG_VERSION_STRING CLANG_VERSION_STRING_MAJOR_MINOR_PATCH
+#else
+#define CLANG_VERSION_STRING CLANG_VERSION_STRING_MAJOR_MINOR
 #endif
 
 namespace clang {
   /// \brief Retrieves the repository path (e.g., Subversion path) that
   /// identifies the particular Clang branch, tag, or trunk from which this
   /// Clang was built.
   std::string getClangRepositoryPath();
 

lib/Frontend/InitHeaderSearch.cpp

 //===--- InitHeaderSearch.cpp - Initialize header search paths ------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the InitHeaderSearch class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/Utils.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h" // C_INCLUDE_DIRS
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Triple.h"
default:
       AddPath("/usr/local/include", System, false);
       break;
     }
   }
 
   // Builtin includes use #include_next directives and should be positioned
   // just prior C include dirs.
   if (HSOpts.UseBuiltinIncludes) {
+    // On FreeBSD the include path is not resource-dir-relative.
+    if (os == llvm::Triple::FreeBSD) {
+      AddPath("/usr/include/clang/" CLANG_VERSION_STRING_MAJOR_MINOR,
+              ExternCSystem, false);
+    }
+
     // Ignore the sys root, we *always* look for clang headers relative to
     // supplied path.
     SmallString<128> P = StringRef(HSOpts.ResourceDir);
     llvm::sys::path::append(P, "include");
     AddUnmappedPath(P.str(), ExternCSystem, false);
   }
 
   // All remaining additions are for system include directories, early exit if