[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG50830e50031b: [lldb] Add -Wl,-rpath to make tests run with 
fresh built libc++ (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Let's give this a shot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 318546.
MaskRay added a comment.

Drop fancy check in dotest.py (restore the previous behavior)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D94888#2511675 , @rupprecht wrote:

> In D94888#2506140 , @labath wrote:
>
>> I am pretty sure they were. This flag is needed to use (system) libc++ with 
>> gcc, and this was happening at a time when we were running the test suite 
>> with gcc, as it was still the default compiler for android.
>
> Is anyone running tests in this build mode (gcc + libc++) still? Should we 
> keep this support around?

It doesn't seem to work right now, so I guess the answer to the first question 
is "no". I don't mind removing it, but I also want to be careful about baking 
in clang assumptions as one of the purposes of this test suite is to be able to 
validate that we are able to debug the output of different compilers.




Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:765-770
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 _, stderr = p.communicate("#include \nint main() {}")
 if not p.returncode:
 return True, "Compiling with -stdlib=libc++ works"
 return False, "Compiling with -stdlib=libc++ fails with the error: 
%s" % stderr

rupprecht wrote:
> I don't know if this is the right place to do it, but it would be good to 
> also check somewhere that binaries built w/ libc++ actually run. That would 
> help with the error you were seeing. Maybe something like this would work?
I'm not sure what does this help with? This compile command does not contain 
the -rpath flag, so, if the executable actually depended on libc++, it would 
not run. And if it does not depend on libc++ features, running it does not tell 
us anything new.

More generally, this is probably not the best place to implement fancy checks 
like this. It's a remnant of the time when folks were running dotest directly, 
and expecting it to just work. These days, it would be better to implement this 
kind of configuration in the lit config or some place like that. Among other 
benefits, it would mean that it gets run just once, instead of once for each 
test. But that is a topic for a different discussion...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 318414.
MaskRay marked an inline comment as done.
MaskRay added a comment.

Adopt rupprecht's suggestion


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,15 +761,15 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 _, stderr = p.communicate("#include \nint main() {}")
-if not p.returncode:
-return True, "Compiling with -stdlib=libc++ works"
-return False, "Compiling with -stdlib=libc++ fails with the error: 
%s" % stderr
+if p.returncode != 0:
+return False, "Compiling with -stdlib=libc++ fails with the 
error: %s" % stderr
+if subprocess.call([f.name]) != 0:
+return False, "Compiling with -stdlib=libc++ works, but fails 
to run"
+return True, "Compiling with -stdlib=libc++ works"
 
 return False, "Don't know how to build with libc++ on %s" % platform
 


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -279,11 +279,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -393,21 +388,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,15 +761,15 @@
 return True, "libc++ always present"
 
 if platform == 

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D94888#2506140 , @labath wrote:

> In D94888#2506000 , @MaskRay wrote:
>
>> In D94888#2505992 , @labath wrote:
>>
>>> It looks like this is removing the ability to build libc++ tests with gcc 
>>> (as it does not have the `-stdlib` option). While having that ability would 
>>> be nice, I don't believe there's anyone currently using that configuration, 
>>> so it shouldn't stand in the way of other things. But we should also update 
>>> the python detection code then (in `canRunLibcxxTests` in 
>>> `packages/Python/lldbsuite/test/dotest.py` -- I guess you just need to 
>>> remove the `if os.path.isdir("/usr/include/c++/v1"):` blurb)
>>>
>>> As for testing against the system libc++ with clang, I guess that should 
>>> still work, as the extra rpath will be just ignored in that case...
>>
>> I do not know whether the following few lines D9426 
>>  were intentional.
>
> I am pretty sure they were. This flag is needed to use (system) libc++ with 
> gcc, and this was happening at a time when we were running the test suite 
> with gcc, as it was still the default compiler for android.

Is anyone running tests in this build mode (gcc + libc++) still? Should we keep 
this support around?




Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:765-770
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
 _, stderr = p.communicate("#include \nint main() {}")
 if not p.returncode:
 return True, "Compiling with -stdlib=libc++ works"
 return False, "Compiling with -stdlib=libc++ fails with the error: 
%s" % stderr

I don't know if this is the right place to do it, but it would be good to also 
check somewhere that binaries built w/ libc++ actually run. That would help 
with the error you were seeing. Maybe something like this would work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

MaskRay updated this revision to Diff 317599.
MaskRay added a comment.

Delete `if os.path.isdir("/usr/include/c++/v1"):`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -281,11 +281,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -395,21 +390,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", 
f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, 
stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -281,11 +281,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -395,21 +390,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -761,8 +761,6 @@
 return True, "libc++ always present"
 
 if platform == "linux":
-if os.path.isdir("/usr/include/c++/v1"):
-return True, "Headers found, let's hope they work"
 with tempfile.NamedTemporaryFile() as f:
 cmd = [configuration.compiler, "-xc++", "-stdlib=libc++", "-o", f.name, "-"]
 p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D94888#2506000 , @MaskRay wrote:

> In D94888#2505992 , @labath wrote:
>
>> It looks like this is removing the ability to build libc++ tests with gcc 
>> (as it does not have the `-stdlib` option). While having that ability would 
>> be nice, I don't believe there's anyone currently using that configuration, 
>> so it shouldn't stand in the way of other things. But we should also update 
>> the python detection code then (in `canRunLibcxxTests` in 
>> `packages/Python/lldbsuite/test/dotest.py` -- I guess you just need to 
>> remove the `if os.path.isdir("/usr/include/c++/v1"):` blurb)
>>
>> As for testing against the system libc++ with clang, I guess that should 
>> still work, as the extra rpath will be just ignored in that case...
>
> I do not know whether the following few lines D9426 
>  were intentional.

I am pretty sure they were. This flag is needed to use (system) libc++ with 
gcc, and this was happening at a time when we were running the test suite with 
gcc, as it was still the default compiler for android. We weren't building 
libc++ in tree so this did kind of work (it looks like it doesn't work anymore 
-- the first problem seems to be the use of `#include_next` in libc++).

> I think deleting the code until someone complains is fine? :)

Deleting it should be fine -- just finish the job and remove it from dotest.py 
too :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D94888#2505992 , @labath wrote:

> It looks like this is removing the ability to build libc++ tests with gcc (as 
> it does not have the `-stdlib` option). While having that ability would be 
> nice, I don't believe there's anyone currently using that configuration, so 
> it shouldn't stand in the way of other things. But we should also update the 
> python detection code then (in `canRunLibcxxTests` in 
> `packages/Python/lldbsuite/test/dotest.py` -- I guess you just need to remove 
> the `if os.path.isdir("/usr/include/c++/v1"):` blurb)
>
> As for testing against the system libc++ with clang, I guess that should 
> still work, as the extra rpath will be just ignored in that case...

I do not know whether the following few lines D9426 
 were intentional.

CXXFLAGS += -isystem /usr/include/c++/v1
LDFLAGS += -lc++

For a proper setup, I think more stuff is needed. `/usr/include/c++/v1` 
probably works for many Linux distributions but the choice doesn't look nice. 
When libc++ is built with lldb, it probably use the libc++ include directory 
instead. LDFLAGS will thus need a specific `-L`.

I think deleting the code until someone complains is fine? :)

> You seem to be removing more than adding there.

Can you kindly test this on NetBSD? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D94888#2505698 , @dblaikie wrote:

> Out of curiousity did you find a way to get lldb-test to print out the 
> stdout/stderr of the underlying lldb process to see what error messages it 
> was producing that demonstrated this failure? I was trying to figure out how 
> to do that, and it seems like a good thing to be able to do for this issue or 
> others down the way.

There's no underlying "lldb process" as these tests use lldb as a library 
through its python api. That said, adding `-t` (trace) to the lldb-dotest 
invocation will make it print more information about what the test is doing, 
though I am not entirely sure how much would it help in this case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It looks like this is removing the ability to build libc++ tests with gcc (as 
it does not have the `-stdlib` option). While having that ability would be 
nice, I don't believe there's anyone currently using that configuration, so it 
shouldn't stand in the way of other things. But we should also update the 
python detection code then (in `canRunLibcxxTests` in 
`packages/Python/lldbsuite/test/dotest.py` -- I guess you just need to remove 
the `if os.path.isdir("/usr/include/c++/v1"):` blurb)

As for testing against the system libc++ with clang, I guess that should still 
work, as the extra rpath will be just ignored in that case...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

(Works for me, FWIW - takes my local run down from 42 failures in 
check-lldb-api to 1 (a timeout))

Out of curiousity did you find a way to get lldb-test to print out the 
stdout/stderr of the underlying lldb process to see what error messages it was 
producing that demonstrated this failure? I was trying to figure out how to do 
that, and it seems like a good thing to be able to do for this issue or others 
down the way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

You seem to be removing more than adding there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94888/new/

https://reviews.llvm.org/D94888

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94888: [lldb] Add -Wl, -rpath to make tests run with fresh built libc++

[Fangrui Song via Phabricator via lldb-commits]

MaskRay created this revision.
MaskRay added reviewers: dblaikie, labath, mgorny, rupprecht.
Herald added a subscriber: krytarowski.
MaskRay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

On my Debian machine, system libc++/libc++abi is not installed (`libc++1-9 
libc++abi-9`),
21 check-lldb-api tests fail because -stdlib=libc++ linked executables cannot
find runtime libc++.so.1 at runtime.

Use the `-Wl,-rpath,$(LLVM_LIBS_DIR)` mechanism in
`packages/Python/lldbsuite/test/make/Makefile.rules` (D58630 
 for NetBSD) to
allow such tests compile/link with fresh libc++ built beside lldb.
(A system libc++.so.1 is not guaranteed to match fresh libc++ header files.)

Some tweaks to the existing NetBSD rule when generalizing:

- Drop `-L$(LLVM_LIBS_DIR)` since Clang driver adds it correctly.
- Add `-stdlib=libc++` only for `USE_LIBCPP`.

Also, drop `-isystem /usr/include/c++/v1` introduced in D9426 
. It is not needed
by Clang driver. GCC using libc++ requires more setup.

I don't find any test needing `-Wl,-rpath` in 
`test/Shell/helper/{build,toolchain}.py` (D58630 
 for NetBSD added them).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94888

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -281,11 +281,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-   ifeq ($(OS),NetBSD)
-   LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-   endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread
@@ -395,21 +390,18 @@
 
 ifeq (1,$(USE_LIBCPP))
CXXFLAGS += -DLLDB_USING_LIBCPP
-   ifeq "$(OS)" "Linux"
-   ifneq (,$(findstring clang,$(CC)))
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   else
-   CXXFLAGS += -isystem /usr/include/c++/v1
-   LDFLAGS += -lc++
-   endif
-   else ifeq "$(OS)" "Android"
+   ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
LDFLAGS += -stdlib=libc++
endif
+   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+   ifneq (,$(LLVM_LIBS_DIR))
+   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   endif
+   endif
 endif
 
 #--


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -281,11 +281,6 @@
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
-ifneq (,$(LLVM_LIBS_DIR))
-	ifeq ($(OS),NetBSD)
-		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR)
-	endif
-endif
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
 	ifneq (,$(filter YES,$(ENABLE_THREADS)))
 		LDFLAGS += -pthread
@@ -395,21 +390,18 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Linux"
-		ifneq (,$(findstring clang,$(CC)))
-			CXXFLAGS += -stdlib=libc++
-			LDFLAGS += -stdlib=libc++
-		else
-			CXXFLAGS += -isystem /usr/include/c++/v1
-			LDFLAGS += -lc++
-		endif
-	else ifeq "$(OS)" "Android"
+	ifeq "$(OS)" "Android"
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
 		CXXFLAGS += -stdlib=libc++
 		LDFLAGS += -stdlib=libc++
 	endif
+	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+		ifneq (,$(LLVM_LIBS_DIR))
+			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		endif
+	endif
 endif
 
 #--
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits