Merge pull request #3541 from citusdata/jelte_fix

We got some errors for safestringlib builds on OSX. The fixes are as follows:

1.  Change name of memset_s to memset8_s
2.  Disable some linker flags on OSX
3.  Reorder warning flags in so -Wall does not override an ignore for clang

Also adds clean-full target to clean our compiled code and also vendored artifacts. Usually it's not needed to clean vendored artifacts once they are built correctly, so they are not cleaned during regular clean to keep full recompiles of our code faster.

Makefile

@@ -26,7 +26,9 @@ install-headers: extension
 	$(INSTALL_DATA) $(citus_abs_srcdir)/src/include/distributed/*.h '$(DESTDIR)$(includedir_server)/distributed/'
 clean-extension:
 	$(MAKE) -C src/backend/distributed/ clean
-.PHONY: extension install-extension clean-extension
+clean-full:
+	$(MAKE) -C src/backend/distributed/ clean-full
+.PHONY: extension install-extension clean-extension clean-full
 # Add to generic targets
 install: install-extension install-headers
 clean: clean-extension

src/backend/distributed/Makefile

@@ -52,7 +52,11 @@ SQL_BUILDDIR=build/sql
 $(safestringlib_a): $(safestringlib_sources)
 	rm -rf $(safestringlib_builddir)
 	mkdir -p $(safestringlib_builddir)
-	cd $(safestringlib_builddir) && cmake $(safestringlib_srcdir) && make -j5
+	@# exports of LDFLAGS and CPPFLAGS are to make sure the ones from this
+	@# Makefile are not used
+	+cd $(safestringlib_builddir) && \
+		export LDFLAGS='' && export CPPFLAGS='' && \
+		cmake $(safestringlib_srcdir) && make
 
 citus.so: $(safestringlib_a)
 
@@ -65,7 +69,7 @@ ifneq (,$(SQL_Po_files))
 include $(SQL_Po_files)
 endif
 
-.PHONY: check-sql-snapshots
+.PHONY: check-sql-snapshots clean-full
 
 check-sql-snapshots:
 	bash -c '\
@@ -79,3 +83,7 @@ cleanup-before-install:
 	rm -f $(DESTDIR)$(datadir)/$(datamoduledir)/citus*
 
 install: cleanup-before-install
+
+clean-full:
+	make clean
+	rm -rf $(safestringlib_builddir)

vendor/README.md

@@ -5,4 +5,6 @@ rm -rf safestringlib
 git clone https://github.com/intel/safestringlib
 rm -rf safestringlib/{.git,unittests}
 git add safestringlib/
+git commit -m "Update safestringlib"
+git cherry-pick -x 92d7a40d1de472d23d05967be9b77eda30af85cb
 ```

vendor/safestringlib/CMakeLists.txt

@@ -145,14 +145,14 @@ set_target_properties(${PROJECT_NAME}_objlib
 
 target_compile_definitions(${PROJECT_NAME}_objlib PRIVATE -DSTDC_HEADERS)
 
+target_compile_options(${PROJECT_NAME}_objlib
+                       PRIVATE -Wall -Wextra -Wsign-compare -Wformat-security)
 target_compile_options(${PROJECT_NAME}_objlib
                        PRIVATE -Wno-unknown-pragmas -Wno-unused-parameter)
 if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_C_COMPILER_VERSION VERSION_GREATER 7)
   target_compile_options(${PROJECT_NAME}_objlib
                          PRIVATE -Wno-implicit-fallthrough)
 endif()
-target_compile_options(${PROJECT_NAME}_objlib
-                       PRIVATE -Wall -Wextra -Wsign-compare -Wformat-security)
 target_compile_options(${PROJECT_NAME}_objlib
                        PRIVATE  -Wstack-protector -Winit-self)
 target_compile_options(${PROJECT_NAME}_objlib
@@ -166,7 +166,10 @@ target_compile_options(${PROJECT_NAME}_objlib PRIVATE -fPIE -fPIC)
 if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_C_COMPILER_VERSION VERSION_GREATER 6)
     target_compile_options(${PROJECT_NAME}_objlib PRIVATE -mmitigate-rop)
 endif()
-set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -z noexecstack -z relro -z now")
+if(APPLE)
+elseif(UNIX)
+    set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -z noexecstack -z relro -z now")
+endif()
 
 option(BUILD_ERROR_ON_WARNING "Fail compilation on warning" OFF)
 

vendor/safestringlib/include/safe_mem_lib.h

@@ -97,7 +97,12 @@ extern errno_t wmemmove_s(wchar_t *dest, rsize_t dmax,
 
 
 /* set bytes */
-extern errno_t memset_s(void *dest, rsize_t dmax, uint8_t value);
+/* NOTE: This name is changed from memset_s to memset8_s out because it does
+ * not match with the C11 declaration of memset_s on OSX. The upstream issue
+ * can be found here:
+ * https://github.com/intel/safestringlib/issues/14
+ */
+extern errno_t memset8_s(void *dest, rsize_t dmax, uint8_t value);
 
 /* set uint16_t */
 extern errno_t memset16_s(uint16_t *dest, rsize_t dmax, uint16_t value);

vendor/safestringlib/safeclib/memset_s.c

@@ -1,5 +1,5 @@
 /*------------------------------------------------------------------
- * memset_s
+ * memset8_s
  *
  * October 2008, Bo Berry
  *
@@ -37,12 +37,12 @@
 
 /**
  * NAME
- *    memset_s
+ *    memset8_s
  *
  * SYNOPSIS
  *    #include "safe_mem_lib.h"
  *    errno_t
- *    memset_s(void *dest, rsize_t len, uint8_t value)
+ *    memset8_s(void *dest, rsize_t len, uint8_t value)
  *
  * DESCRIPTION
  *    Sets len bytes starting at dest to the specified value.
@@ -78,22 +78,22 @@
  *
  */
 errno_t
-memset_s (void *dest, rsize_t len, uint8_t value)
+memset8_s (void *dest, rsize_t len, uint8_t value)
 {
     if (dest == NULL) {
-        invoke_safe_mem_constraint_handler("memset_s: dest is null",
+        invoke_safe_mem_constraint_handler("memset8_s: dest is null",
                    NULL, ESNULLP);
         return (RCNEGATE(ESNULLP));
     }
 
     if (len == 0) {
-        invoke_safe_mem_constraint_handler("memset_s: len is 0",
+        invoke_safe_mem_constraint_handler("memset8_s: len is 0",
                    NULL, ESZEROL);
         return (RCNEGATE(ESZEROL));
     }
 
     if (len > RSIZE_MAX_MEM) {
-        invoke_safe_mem_constraint_handler("memset_s: len exceeds max",
+        invoke_safe_mem_constraint_handler("memset8_s: len exceeds max",
                    NULL, ESLEMAX);
         return (RCNEGATE(ESLEMAX));
     }
@@ -102,4 +102,4 @@ memset_s (void *dest, rsize_t len, uint8_t value)
 
     return (RCNEGATE(EOK));
 }
-EXPORT_SYMBOL(memset_s)
+EXPORT_SYMBOL(memset8_s)