Hi Rafael,
See attached the patches with the changes that you suggested.
Tested on amd64-linux. Ok to commit?
Thanks,
Sebastian
--
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum
>>> + Â Â TranslatedArgs(_TranslatedArgs), Redirects(0),
>>> SysRoot(D.SysRoot)
>>>
>>> Is this value of SysRoot ever used? It should not, right? It is
>>> probably better to initialize SysRoot with an invalid value instead of
>>> D.SysRoot.
>>
>> I'm not sure I understand your comment here.
>>
>> We can also implement getSysRoot by returning the sysroot of the driver.
>
> I am suggesting initializing with something like
>
> ... SysRoot(".....")
>
> to make sure we find any code using the SysRoot before the following code
> runs.
>
>>> Â if (SysRoot != "") {
>>> + Â Â llvm::SmallString<128> Prefix(SysRoot);
>>> + Â Â if (Prefix.back() == '/') {
>>> + Â Â Â llvm::sys::path::remove_filename(Prefix); // remove the /
>>> + Â Â Â SysRoot = Prefix.str();
>>> + Â Â }
>>> + Â }
>>>
>>> This points a StringRef to a value that is destroyed at the closing
>>> brace.
>>
>> Right, I have not understood your remark last time.
>> As the original code was not removing the trailing /,
>> I removed that code.
>
> I think you have another use after free bug in
>
> + CmdArgs.push_back(sysroot.str().c_str());
>
> since this creates a std::string and takes a pointer to the inner c
> string. You probably have to use C.getArgs().MakeArgString.
>
>> Sebastian
>> --
>> Qualcomm Innovation Center, Inc is a member of Code Aurora Forum
>
> Cheers,
> Rafael
>
> p.s.: you dropped cfe-commits in the reply.
>
>From a28cd8ed98978a8ca4ce7baa8b1debfcdec5e3ba Mon Sep 17 00:00:00 2001
From: Sebastian Pop <[email protected]>
Date: Thu, 9 Feb 2012 13:02:01 -0600
Subject: [PATCH] add configure flag --with-default-sysroot
---
autoconf/configure.ac | 7 +++++++
configure | 19 +++++++++++++++++--
include/llvm/Config/config.h.cmake | 3 ---
include/llvm/Config/config.h.in | 3 +++
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/autoconf/configure.ac b/autoconf/configure.ac
index 884a21b..baf440e 100644
--- a/autoconf/configure.ac
+++ b/autoconf/configure.ac
@@ -838,6 +838,13 @@ AC_ARG_WITH(gcc-toolchain,
AC_DEFINE_UNQUOTED(GCC_INSTALL_PREFIX,"$withval",
[Directory where gcc is installed.])
+AC_ARG_WITH(sysroot,
+ AS_HELP_STRING([--with-default-sysroot],
+ [Add --sysroot=<path> to all compiler invocations.]),,
+ withval="")
+AC_DEFINE_UNQUOTED(DEFAULT_SYSROOT,"$withval",
+ [Default <path> to all compiler invocations for --sysroot=<path>.])
+
dnl Allow linking of LLVM with GPLv3 binutils code.
AC_ARG_WITH(binutils-include,
AS_HELP_STRING([--with-binutils-include],
diff --git a/configure b/configure
index 8ff4e62..be2cb72 100755
--- a/configure
+++ b/configure
@@ -1442,6 +1442,7 @@ Optional Packages:
--with-c-include-dirs Colon separated list of directories clang will
search for headers
--with-gcc-toolchain Directory where gcc is installed.
+ --with-default-sysroot Add --sysroot=<path> to all compiler invocations.
--with-binutils-include Specify path to binutils/include/ containing
plugin-api.h file for gold plugin.
--with-bug-report-url Specify the URL where bug reports should be
@@ -3802,7 +3803,7 @@ else
llvm_cv_target_os_type="Darwin" ;;
*-*-minix*)
llvm_cv_target_os_type="Minix" ;;
- *-*-freebsd*| *-*-kfreebsd-gnu)
+ *-*-freebsd* | *-*-kfreebsd-gnu)
llvm_cv_target_os_type="FreeBSD" ;;
*-*-openbsd*)
llvm_cv_target_os_type="OpenBSD" ;;
@@ -5583,6 +5584,20 @@ _ACEOF
+# Check whether --with-sysroot was given.
+if test "${with_sysroot+set}" = set; then
+ withval=$with_sysroot;
+else
+ withval=""
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define DEFAULT_SYSROOT "$withval"
+_ACEOF
+
+
+
# Check whether --with-binutils-include was given.
if test "${with_binutils_include+set}" = set; then
withval=$with_binutils_include;
@@ -10386,7 +10401,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<EOF
-#line 10387 "configure"
+#line 10404 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
diff --git a/include/llvm/Config/config.h.cmake b/include/llvm/Config/config.h.cmake
index 69e3580..95c4d6c 100644
--- a/include/llvm/Config/config.h.cmake
+++ b/include/llvm/Config/config.h.cmake
@@ -11,9 +11,6 @@
/* Relative directory for resource files */
#define CLANG_RESOURCE_DIR "${CLANG_RESOURCE_DIR}"
-/* Directory wherelibstdc++ is installed. */
-#define GCC_INSTALL_PREFIX "${GCC_INSTALL_PREFIX}"
-
/* Directories clang will search for headers */
#define C_INCLUDE_DIRS "${C_INCLUDE_DIRS}"
diff --git a/include/llvm/Config/config.h.in b/include/llvm/Config/config.h.in
index ccff7da..677bf2e 100644
--- a/include/llvm/Config/config.h.in
+++ b/include/llvm/Config/config.h.in
@@ -12,6 +12,9 @@
/* Directories clang will search for headers */
#undef C_INCLUDE_DIRS
+/* Default <path> to all compiler invocations for --sysroot=<path>. */
+#undef DEFAULT_SYSROOT
+
/* Define if position independent code is enabled */
#undef ENABLE_PIC
--
1.7.5.4
>From 0c41bf9b2f62e47aed1036d598615391a3601e85 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <[email protected]>
Date: Thu, 9 Feb 2012 13:06:08 -0600
Subject: [PATCH] use DEFAULT_SYSROOT
---
CMakeLists.txt | 4 ++++
include/clang/Config/config.h.cmake | 9 ++++++---
include/clang/Config/config.h.in | 9 ++++++---
include/clang/Driver/Compilation.h | 3 +++
lib/Driver/Compilation.cpp | 4 ++++
lib/Driver/Driver.cpp | 8 +++-----
lib/Driver/Tools.cpp | 10 ++++++----
7 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1197610..c828482 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -66,6 +66,10 @@ set(CLANG_RESOURCE_DIR "" CACHE STRING
set(C_INCLUDE_DIRS "" CACHE STRING
"Colon separated list of directories clang will search for headers.")
+set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
+set(DEFAULT_SYSROOT "" CACHE PATH
+ "Default <path> to all compiler invocations for --sysroot=<path>." )
+
set(CLANG_VENDOR "" CACHE STRING
"Vendor-specific text for showing with version information.")
diff --git a/include/clang/Config/config.h.cmake b/include/clang/Config/config.h.cmake
index bd5dc31..c18c4cc 100644
--- a/include/clang/Config/config.h.cmake
+++ b/include/clang/Config/config.h.cmake
@@ -4,8 +4,11 @@
/* Relative directory for resource files */
#define CLANG_RESOURCE_DIR "${CLANG_RESOURCE_DIR}"
-/* Directory where gcc is installed. */
-#define GCC_INSTALL_PREFIX "${GCC_INSTALL_PREFIX}"
-
/* Directories clang will search for headers */
#define C_INCLUDE_DIRS "${C_INCLUDE_DIRS}"
+
+/* Default <path> to all compiler invocations for --sysroot=<path>. */
+#define DEFAULT_SYSROOT "${DEFAULT_SYSROOT}"
+
+/* Directory where gcc is installed. */
+#define GCC_INSTALL_PREFIX "${GCC_INSTALL_PREFIX}"
diff --git a/include/clang/Config/config.h.in b/include/clang/Config/config.h.in
index 3f5d503..24ed6bd 100644
--- a/include/clang/Config/config.h.in
+++ b/include/clang/Config/config.h.in
@@ -9,10 +9,13 @@
/* Relative directory for resource files */
#undef CLANG_RESOURCE_DIR
-/* Directory where gcc is installed. */
-#undef GCC_INSTALL_PREFIX
-
/* Directories clang will search for headers */
#undef C_INCLUDE_DIRS
+/* Default <path> to all compiler invocations for --sysroot=<path>. */
+#undef DEFAULT_SYSROOT
+
+/* Directory where gcc is installed. */
+#undef GCC_INSTALL_PREFIX
+
#endif
diff --git a/include/clang/Driver/Compilation.h b/include/clang/Driver/Compilation.h
index fd88c3a..6f1a221 100644
--- a/include/clang/Driver/Compilation.h
+++ b/include/clang/Driver/Compilation.h
@@ -92,6 +92,9 @@ public:
return FailureResultFiles;
}
+ /// Returns the sysroot path.
+ StringRef getSysRoot() const;
+
/// getArgsForToolChain - Return the derived argument list for the
/// tool chain \arg TC (or the default tool chain, if TC is not
/// specified).
diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp
index 42c8449..5553fc9 100644
--- a/lib/Driver/Compilation.cpp
+++ b/lib/Driver/Compilation.cpp
@@ -230,3 +230,7 @@ void Compilation::initCompilationForDiagnostics(void) {
Redirects[1] = new const llvm::sys::Path();
Redirects[2] = new const llvm::sys::Path();
}
+
+StringRef Compilation::getSysRoot(void) const {
+ return getDriver().SysRoot;
+}
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
index 35e6398..57cb153 100644
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -49,8 +49,8 @@ Driver::Driver(StringRef ClangExecutable,
bool IsProduction,
DiagnosticsEngine &Diags)
: Opts(createDriverOptTable()), Diags(Diags),
- ClangExecutable(ClangExecutable), UseStdLib(true),
- DefaultTargetTriple(DefaultTargetTriple),
+ ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
+ UseStdLib(true), DefaultTargetTriple(DefaultTargetTriple),
DefaultImageName(DefaultImageName),
DriverTitle("clang \"gcc-compatible\" driver"),
CCPrintOptionsFilename(0), CCPrintHeadersFilename(0),
@@ -660,9 +660,7 @@ bool Driver::HandleImmediateArgs(const Compilation &C) {
llvm::outs() << "\n";
llvm::outs() << "libraries: =" << ResourceDir;
- std::string sysroot;
- if (Arg *A = C.getArgs().getLastArg(options::OPT__sysroot_EQ))
- sysroot = A->getValue(C.getArgs());
+ StringRef sysroot = C.getSysRoot();
for (ToolChain::path_list::const_iterator it = TC.getFilePaths().begin(),
ie = TC.getFilePaths().end(); it != ie; ++it) {
diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
index 4c4ab7b..fc3a988 100644
--- a/lib/Driver/Tools.cpp
+++ b/lib/Driver/Tools.cpp
@@ -377,10 +377,11 @@ void Clang::AddPreprocessingOptions(Compilation &C,
// If we have a --sysroot, and don't have an explicit -isysroot flag, add an
// -isysroot to the CC1 invocation.
- if (Arg *A = Args.getLastArg(options::OPT__sysroot_EQ)) {
+ StringRef sysroot = C.getSysRoot();
+ if (sysroot != "") {
if (!Args.hasArg(options::OPT_isysroot)) {
CmdArgs.push_back("-isysroot");
- CmdArgs.push_back(A->getValue(Args));
+ CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
}
}
@@ -3971,9 +3972,10 @@ void darwin::Link::AddLinkArgs(Compilation &C,
// Give --sysroot= preference, over the Apple specific behavior to also use
// --isysroot as the syslibroot.
- if (const Arg *A = Args.getLastArg(options::OPT__sysroot_EQ)) {
+ StringRef sysroot = C.getSysRoot();
+ if (sysroot != "") {
CmdArgs.push_back("-syslibroot");
- CmdArgs.push_back(A->getValue(Args));
+ CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
} else if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
CmdArgs.push_back("-syslibroot");
CmdArgs.push_back(A->getValue(Args));
--
1.7.5.4
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
