Only throw -fPIC when building a shared library

[asmith]

Summary

Update makefiles to specify -fPIC in Makefile.rules and only throw -fPIC when building a shared library. This change is necessary to allow building the lldb tests on Windows where -fPIC is not a valid option.

Update a few places to Python 3.x syntax

[zturner]

In the future when you upload diffs can you include context? (i.e. git diff -U999999). It's nice to be able to see the surrounding code when I'm looking at a diff.

Is there ever a case where you would want to build a shared library without -fPIC? I'm wondering if we should just update the common Makefile.rules and if DYLIB_NAME is set (or something else indicating that this is a shared library), then we put the logic about -fPIC in that common file. Then people wouldn't have to remember to get this right in the future.

[labath]

In D42994#999891, @zturner wrote:

In the future when you upload diffs can you include context? (i.e. git diff -U999999). It's nice to be able to see the surrounding code when I'm looking at a diff.

Is there ever a case where you would want to build a shared library without -fPIC? I'm wondering if we should just update the common Makefile.rules and if DYLIB_NAME is set (or something else indicating that this is a shared library), then we put the logic about -fPIC in that common file. Then people wouldn't have to remember to get this right in the future.

Adding -fPIC by default sounds like a great idea. Building a shared library without it will probably break compilation, but if some test still wants to try it, it can be turned off with -fno-PIC.

[labath]

packages/Python/lldbsuite/test/make/Makefile.rules

550

I think Zachary meant only when building the shared library, but you're doing it unconditionally. This would be a nice place to set the flag for shared library only.

[zturner]

This lgtm. If this causes some tests that were previously skipped or xfailed to start passing, you can unskip / unxfail them at the same time.

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

48–49

Weird, does that comma syntax even do anything or was that just a bug?

[stella.stamenova]

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

48–49

In python 2.5 or earlier it meant the same as "as" but "as" was not supported. Then for a while (until 3.x) both the comma and "as" meant the same thing and then since 3.x "as" is the only option allowed.

packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/Makefile

 LEVEL = ../../../make
 
 DYLIB_NAME := foo
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
-CFLAGS_EXTRAS := -fPIC
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile

 LEVEL = ../../../make
 
 DYLIB_NAME := foo
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
-CFLAGS_EXTRAS += -fPIC
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/load_unload/a.mk

 LEVEL := ../../make
 
 LIB_PREFIX := loadunload_
 
-CFLAGS_EXTRAS := -fPIC
 LD_EXTRAS := -L. -l$(LIB_PREFIX)b
 
 DYLIB_NAME := $(LIB_PREFIX)a
 DYLIB_CXX_SOURCES := a.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules
 
 $(DYLIB_FILENAME): lib_b
 
 .PHONY lib_b:
   $(MAKE) VPATH=$(SRCDIR) -I $(SRCDIR) -f $(SRCDIR)/b.mk
 
 clean::
   $(MAKE) -I $(SRCDIR) -f $(SRCDIR)/b.mk clean

packages/Python/lldbsuite/test/functionalities/load_unload/b.mk

 LEVEL := ../../make
 
 LIB_PREFIX := loadunload_
 
 DYLIB_NAME := $(LIB_PREFIX)b
 DYLIB_CXX_SOURCES := b.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/load_unload/c.mk

 LEVEL := ../../make
 
 LIB_PREFIX := loadunload_
 
 DYLIB_NAME := $(LIB_PREFIX)c
 DYLIB_CXX_SOURCES := c.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/load_unload/d.mk

 LEVEL := ../../make
 
 LIB_PREFIX := loadunload_
 
 DYLIB_EXECUTABLE_PATH := $(CURDIR)
 
 DYLIB_NAME := $(LIB_PREFIX)d
 DYLIB_CXX_SOURCES := d.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/load_unload/hidden/Makefile

 LEVEL := ../../../make
 
 LIB_PREFIX := loadunload_
 
 DYLIB_NAME := $(LIB_PREFIX)d
 DYLIB_CXX_SOURCES := d.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/pre_run_dylibs/Makefile

 LEVEL = ../../make
 
 DYLIB_NAME := unlikely_name
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
-CFLAGS_EXTRAS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

def test_attach_to_process_by_id(self):
         process = target.GetProcess()
         self.assertTrue(process, PROCESS_IS_VALID)
 
     @expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], bugnumber="<rdar://problem/34538611>") # old lldb-server has race condition, launching an inferior and then launching debugserver in quick succession sometimes fails
     def test_attach_to_process_from_different_dir_by_id(self):
         """Test attach by process id"""
         newdir = self.getBuildArtifact("newdir")
         try:
-            os.mkdir(newdir)
-        except OSError, e:
+            os.mkdir(os.path.join(os.getcwd(),'newdir'))
+        except OSError as e:
             if e.errno != os.errno.EEXIST:
                 raise
         testdir = self.getBuildDir()
         exe = os.path.join(newdir, 'proc_attach')
         self.buildProgram('main.cpp', exe)
         self.addTearDownHook(lambda: shutil.rmtree(newdir))
 
         # Spawn a new process

packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk

 LEVEL := ../../../make
 
 DYLIB_NAME := One
 DYLIB_C_SOURCES := One.c OneConstant.c
 DYLIB_ONLY := YES
 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS_EXTRAS += -fPIC
-
-OneConstant.o: OneConstant.c
+One/OneConstant.o: One/OneConstant.c
   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@

packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk

 LEVEL := ../../../make
 
 DYLIB_NAME := Two
 DYLIB_C_SOURCES := Two.c TwoConstant.c
 DYLIB_ONLY := YES
 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS_EXTRAS += -fPIC
-
-TwoConstant.o: TwoConstant.c
+Two/TwoConstant.o: Two/TwoConstant.c
   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@

packages/Python/lldbsuite/test/lang/c/shared_lib/Makefile

 LEVEL = ../../../make
 
 DYLIB_NAME := foo
 DYLIB_C_SOURCES := foo.c
 C_SOURCES := main.c
-CFLAGS_EXTRAS += -fPIC
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/c/shared_lib_stripped_symbols/Makefile

 LEVEL = ../../../make
 
 DYLIB_NAME := foo
 DYLIB_C_SOURCES := foo.c
 C_SOURCES := main.c
-CFLAGS_EXTRAS += -fPIC
 
 SPLIT_DEBUG_SYMBOLS = YES
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/c/tls_globals/Makefile

 LEVEL = ../../../make
 
 C_SOURCES := main.c
-CFLAGS_EXTRAS += -fPIC
 
 DYLIB_NAME := a
 DYLIB_C_SOURCES := a.c
 
 ENABLE_THREADS := YES
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/namespace_definitions/a.mk

 LEVEL := ../../../make
 
 DYLIB_NAME := a
 DYLIB_CXX_SOURCES := a.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/namespace_definitions/b.mk

 LEVEL := ../../../make
 
 DYLIB_NAME := b
 DYLIB_CXX_SOURCES := b.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
-
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lldbtest.py

def buildLibrary(self, sources, lib_name):
                                                     os.path.join(
                                                         os.environ["LLDB_SRC"],
                                                         "include")),
                 'LD_EXTRAS': "-shared -L%s/../lib -llldb -Wl,-rpath,%s/../lib" % (lib_dir, lib_dir)}
         elif self.getPlatform() == 'windows':
             d = {
                 'DYLIB_CXX_SOURCES': sources,
                 'DYLIB_NAME': lib_name,
-                'CFLAGS_EXTRAS': "%s -I%s -fPIC" % (stdflag,
-                                                    os.path.join(
-                                                        os.environ["LLDB_SRC"],
-                                                        "include")),
+                'CFLAGS_EXTRAS': "%s -I%s " % (stdflag,
                 'LD_EXTRAS': "-shared -l%s\liblldb.lib" % self.os.environ["LLDB_IMPLIB_DIR"]}
         if self.TraceOn():
             print(
                 "Building LLDB Library (%s) from sources %s" %
                 (lib_name, sources))
 
         self.buildDefault(dictionary=d)
 

packages/Python/lldbsuite/test/make/Makefile.rules

 
 # Use this one if you want to build one part of the result without debug information:
 ifeq "$(OS)" "Darwin"
   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 else
   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 endif
 
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS += -fPIC
+  CXXFLAGS += -fPIC
+endif
+
 ifeq "$(MAKE_DWO)" "YES"
   CFLAGS += -gsplit-dwarf
 endif
 
 CLANG_MODULE_CACHE_DIR := module-cache
 
 MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
 

 else
 $(ARCHIVE_NAME) : $(foreach ar_obj,$(ARCHIVE_OBJECTS),$(ARCHIVE_NAME)($(ar_obj)))
 endif
 endif
 
 #----------------------------------------------------------------------
 # Make the dylib
 #----------------------------------------------------------------------
 $(DYLIB_OBJECTS) : CFLAGS += -DCOMPILING_LLDB_TEST_DLL
 
 $(DYLIB_FILENAME) : $(DYLIB_OBJECTS)
 ifeq "$(OS)" "Darwin"
   $(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -install_name "$(DYLIB_EXECUTABLE_PATH)/$(DYLIB_FILENAME)" -dynamiclib -o "$(DYLIB_FILENAME)"
 ifneq "$(MAKE_DSYM)" "NO"
 ifneq "$(DS)" ""
   "$(DS)" $(DSFLAGS) "$(DYLIB_FILENAME)"
 endif

packages/Python/lldbsuite/test/python_api/process/read-mem-cstring/TestReadMemCString.py

 class TestReadMemCString(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
     def test_read_memory_c_string(self):
         """Test corner case behavior of SBProcess::ReadCStringFromMemory"""
         self.build()
-  self.dbg.SetAsync(False)
+        self.dbg.SetAsync(False)
 
         self.main_source = "main.c"
-        self.main_source_path = os.path.join(self.getSourceDir(),
-                                             self.main_source)
-  self.main_source_spec = lldb.SBFileSpec(self.main_source_path)
-  self.exe = self.getBuildArtifact("read-mem-cstring")
+        self.main_source_spec = lldb.SBFileSpec(self.main_source)
+        self.exe = os.path.join(os.getcwd(), "read-mem-cstring")
 
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
             self, 'breakpoint here', self.main_source_spec, None, self.exe)
 
-  frame = thread.GetFrameAtIndex(0)
-        
+        frame = thread.GetFrameAtIndex(0)
         err = lldb.SBError()
 
         empty_str_addr = frame.FindVariable("empty_string").GetValueAsUnsigned(err)
         self.assertTrue(err.Success())
         self.assertTrue(empty_str_addr != lldb.LLDB_INVALID_ADDRESS)
 
         one_letter_str_addr = frame.FindVariable("one_letter_string").GetValueAsUnsigned(err)
         self.assertTrue(err.Success())