Skip to content

Commit

Permalink
[mono] Reorganize zlib dependency logic (dotnet#63365)
Browse files Browse the repository at this point in the history
* [mono] Add back HAVE_SYS_ZLIB support

In particular this re-enables embedded PDB support on iOS and Android

* [mono] Reorganize zlib dependency logic

Instead of deciding whether features are enabled based on the presence or
absense of zlib, add switches to control features (embedded PDB support, and
compressed log profiler output) and based on that require zlib.

On some platforms we use the in-tree copy of zlib from
src/native/external/zlib, otherwise we want to link against the system zlib.

Previously, if zlib was missing the build would quietly succeed, but the
feature support would be missing.  Now, if the feature isn't disabled but zlib
is missing, we fail the build.

If both features are disabled, don't depend on zlib at all.

* Fix typos

Co-authored-by: Adeel Mujahid <[email protected]>

* Style nits

- reverse if() logic to avoid negation

- use cmakedefine without a value in config.h.in, same as CoreCLR

Co-authored-by: Adeel Mujahid <[email protected]>
  • Loading branch information
lambdageek and am11 authored Jan 6, 2022
1 parent d988a52 commit 0e800a6
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 27 deletions.
13 changes: 13 additions & 0 deletions src/mono/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,19 @@ if(ENABLE_MONOTOUCH)
add_definitions(-DMONOTOUCH=1)
endif()

# Decide if we need zlib, and if so whether we want the system zlib or the in-tree copy.
if(NOT DISABLE_EMBEDDED_PDB OR NOT DISABLE_LOG_PROFILER_GZ)
if(INTERNAL_ZLIB)
# defines ZLIB_SOURCES
include(${CLR_SRC_NATIVE_DIR}/external/zlib.cmake)
else()
# if we're not on a platform where we use the in-tree zlib, require system zlib
include(${CLR_SRC_NATIVE_DIR}/libs/System.IO.Compression.Native/extra_libs.cmake)
set(Z_LIBS)
append_extra_compression_libs(Z_LIBS)
endif()
endif()

######################################
# HEADER/FUNCTION CHECKS
######################################
Expand Down
10 changes: 8 additions & 2 deletions src/mono/cmake/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@
/* Use static ICU */
#cmakedefine STATIC_ICU 1

/* Use OS-provided zlib */
#cmakedefine HAVE_SYS_ZLIB 1
/* Use in-tree zlib */
#cmakedefine INTERNAL_ZLIB 1

/* Define to 1 if you have the <poll.h> header file. */
#cmakedefine HAVE_POLL_H 1
Expand Down Expand Up @@ -951,6 +951,12 @@
/* QCalls disabled */
#cmakedefine DISABLE_QCALLS 1

/* Embedded PDB support disabled */
#cmakedefine DISABLE_EMBEDDED_PDB

/* log profiler compressed output disabled */
#cmakedefine DISABLE_LOG_PROFILER_GZ

/* Have __thread keyword */
#cmakedefine MONO_KEYWORD_THREAD @MONO_KEYWORD_THREAD@

Expand Down
1 change: 0 additions & 1 deletion src/mono/cmake/defines-todo.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#option (NEED_LINK_UNLINK "Define if Unix sockets cannot be created in an anonymous namespace")
#option (HAVE_CLASSIC_WINAPI_SUPPORT "Use classic Windows API support")
#option (HAVE_UWP_WINAPI_SUPPORT "Don't use UWP Windows API support")
#option (HAVE_SYS_ZLIB "Use OS-provided zlib")
#option (MONO_XEN_OPT "Xen-specific behaviour")
#option (MONO_SMALL_CONFIG "Reduce runtime requirements (and capabilities)")
#option (AC_APPLE_UNIVERSAL_BUILD "Define if building universal (internal helper macro)")
Expand Down
2 changes: 2 additions & 0 deletions src/mono/cmake/options.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ option (DISABLE_EXECUTABLES "Disable the build of the runtime executables")
option (DISABLE_ICALL_TABLES "Enable separate icall table library")
option (DISABLE_QCALLS "Disable support for QCalls")
option (DISABLE_LOG_DEST "Disable MONO_LOG_DEST support")
option (DISABLE_EMBEDDED_PDB "Disable support for loading embedded PDBs")
option (DISABLE_LOG_PROFILER_GZ "Disable support for log profiler output compression")
option (ENABLE_ICALL_EXPORT "Export icall functions")
option (ENABLE_ICALL_SYMBOL_MAP "Generate tables which map icall functions to their C symbols")
option (ENABLE_PERFTRACING "Enables support for eventpipe library")
Expand Down
8 changes: 5 additions & 3 deletions src/mono/mono/metadata/debug-mono-ppdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@
#include <mono/utils/bsearch.h>
#include <mono/utils/mono-logger-internals.h>

#if HOST_WIN32 || HOST_WASM
#ifndef DISABLE_EMBEDDED_PDB
#ifdef INTERNAL_ZLIB
#include <external/zlib/zlib.h>
#elif HAVE_SYS_ZLIB
#else
#include <zlib.h>
#endif
#endif

#include "debug-mono-ppdb.h"

Expand Down Expand Up @@ -162,7 +164,7 @@ mono_ppdb_load_file (MonoImage *image, const guint8 *raw_contents, int size)
return NULL;
}

#if HOST_WIN32 || HOST_WASM || HAVE_SYS_ZLIB
#ifndef DISABLE_EMBEDDED_PDB
if (ppdb_data) {
/* Embedded PPDB data */
/* ppdb_size is the uncompressed size */
Expand Down
9 changes: 3 additions & 6 deletions src/mono/mono/mini/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ include(../eglib/CMakeLists.txt)
include(../utils/CMakeLists.txt)
include(../metadata/CMakeLists.txt)
include(../sgen/CMakeLists.txt)
if(INTERNAL_ZLIB) # TODO: hook up HAVE_SYS_ZLIB instead
include(${CLR_SRC_NATIVE_DIR}/external/zlib.cmake)
endif()
include(../component/CMakeLists.txt)

if(HOST_WIN32)
Expand Down Expand Up @@ -348,7 +345,7 @@ if(NOT DISABLE_SHARED_LIBS)
# to avoid a conflict we rename the import library with the .import.lib suffix
set_target_properties(monosgen-shared PROPERTIES IMPORT_SUFFIX ".import.lib")
endif()
target_link_libraries(monosgen-shared PRIVATE ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS})
target_link_libraries(monosgen-shared PRIVATE ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS} ${Z_LIBS})
if(ICU_LDFLAGS)
set_property(TARGET monosgen-shared APPEND_STRING PROPERTY LINK_FLAGS " ${ICU_LDFLAGS}")
endif()
Expand Down Expand Up @@ -398,7 +395,7 @@ if(NOT DISABLE_SHARED_LIBS)
add_library(${frameworkconfig} SHARED $<TARGET_OBJECTS:monosgen-objects>)
target_compile_definitions(${frameworkconfig} PRIVATE -DMONO_DLL_EXPORT)
target_sources(${frameworkconfig} PRIVATE $<TARGET_OBJECTS:eglib_objects>)
target_link_libraries(${frameworkconfig} PRIVATE ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS})
target_link_libraries(${frameworkconfig} PRIVATE ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS} ${Z_LIBS})
if(ICU_LDFLAGS)
set_property(TARGET ${frameworkconfig} APPEND_STRING PROPERTY LINK_FLAGS " ${ICU_LDFLAGS}")
endif()
Expand Down Expand Up @@ -472,7 +469,7 @@ if(NOT DISABLE_EXECUTABLES)
if(MONO_CROSS_COMPILE_EXECUTABLE_NAME)
set_target_properties(mono-sgen PROPERTIES OUTPUT_NAME mono-aot-cross)
endif()
target_link_libraries(mono-sgen PRIVATE monosgen-static ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS})
target_link_libraries(mono-sgen PRIVATE monosgen-static ${OS_LIBS} ${ICONV_LIB} ${LLVM_LIBS} ${ICU_LIBS} ${Z_LIBS})
if(NOT DISABLE_COMPONENTS AND STATIC_COMPONENTS AND NOT DISABLE_LINK_STATIC_COMPONENTS)
# if components are built statically, link them into runtime.
target_sources(mono-sgen PRIVATE "${mono-components-objects}")
Expand Down
6 changes: 5 additions & 1 deletion src/mono/mono/profiler/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ include_directories(
if(NOT DISABLE_LIBS)
if(HOST_ANDROID OR HOST_IOS OR HOST_TVOS)
# Build the logging profiler only for mobile platforms
add_library(mono-profiler-log SHARED helper.c log.c log-args.c)
add_library(mono-profiler-log SHARED helper.c log.c log-args.c ${ZLIB_SOURCES})
target_link_libraries(mono-profiler-log monosgen-shared eglib_objects)
if(HOST_ANDROID)
target_link_libraries(mono-profiler-log log)
Expand All @@ -22,6 +22,10 @@ if(NOT DISABLE_LIBS)
add_library(mono-profiler-log-static STATIC helper.c log.c log-args.c)
set_target_properties(mono-profiler-log-static PROPERTIES OUTPUT_NAME mono-profiler-log)
install(TARGETS mono-profiler-log-static LIBRARY)

if(NOT DISABLE_LOG_PROFILER_GZ)
target_link_libraries(mono-profiler-log ${Z_LIBS})
endif()
endif()

add_library(mono-profiler-aot-static STATIC aot.c helper.c)
Expand Down
16 changes: 10 additions & 6 deletions src/mono/mono/profiler/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@
#ifndef HOST_WIN32
#include <sys/socket.h>
#endif
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
#ifdef INTERNAL_ZLIB
#include <external/zlib/zlib.h>
#else
#include <zlib.h>
#endif
#endif

#ifdef HOST_WIN32
#include <winsock2.h>
Expand Down Expand Up @@ -316,7 +320,7 @@ struct _MonoProfiler {
MonoProfilerHandle handle;

FILE* file;
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
gzFile gzfile;
#endif

Expand Down Expand Up @@ -1065,7 +1069,7 @@ dump_header (void)
p = write_header_string (p, arch);
p = write_header_string (p, os);

#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (log_profiler.gzfile) {
gzwrite (log_profiler.gzfile, hbuf, p - hbuf);
} else
Expand Down Expand Up @@ -1156,7 +1160,7 @@ dump_buffer (LogBuffer *buf)
p = write_int64 (p, buf->thread_id);
p = write_int64 (p, buf->method_base);

#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (log_profiler.gzfile) {
gzwrite (log_profiler.gzfile, hbuf, p - hbuf);
gzwrite (log_profiler.gzfile, buf->buf, buf->cursor - buf->buf);
Expand Down Expand Up @@ -3082,7 +3086,7 @@ log_shutdown (MonoProfiler *prof)
g_assert (!(state & 0xFFFF) && "Why is the reader count still non-zero?");
g_assert (!(state >> 16) && "Why is the exclusive lock still held?");

#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (prof->gzfile)
gzclose (prof->gzfile);
#endif
Expand Down Expand Up @@ -4145,7 +4149,7 @@ create_profiler (const char *args, const char *filename, GPtrArray *filters)
exit (1);
}

#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (log_config.use_zip)
log_profiler.gzfile = gzdopen (fileno (log_profiler.file), "wb");
#endif
Expand Down
14 changes: 9 additions & 5 deletions src/mono/mono/profiler/mprof-report.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@
#include <unistd.h>
#endif
#include <stdlib.h>
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
#ifdef INTERNAL_ZLIB
#include <external/zlib/zlib.h>
#else
#include <zlib.h>
#endif
#endif
#include <glib.h>
#include <mono/metadata/profiler.h>
#include <mono/metadata/object.h>
Expand Down Expand Up @@ -1554,7 +1558,7 @@ typedef struct _RemCtxContext RemCtxContext;

typedef struct {
FILE *file;
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
gzFile gzfile;
#endif
unsigned char *buf;
Expand Down Expand Up @@ -1624,7 +1628,7 @@ static int
load_data (ProfContext *ctx, int size)
{
ensure_buffer (ctx, size);
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (ctx->gzfile) {
int r = gzread (ctx->gzfile, ctx->buf, size);
if (r == 0)
Expand Down Expand Up @@ -2227,7 +2231,7 @@ decode_buffer (ProfContext *ctx)
int len, i;
ThreadContext *thread;

#ifdef HAVE_SYS_ZLIB
#ifndef DISABLE_LOG_PROFILER_GZ
if (ctx->gzfile)
file_offset = gztell (ctx->gzfile);
else
Expand Down Expand Up @@ -3234,7 +3238,7 @@ load_file (char *name)
printf ("Cannot open file: %s\n", name);
exit (1);
}
#if defined (HAVE_SYS_ZLIB)
#ifndef DISABLE_LOG_PROFILER_GZ
if (ctx->file != stdin)
ctx->gzfile = gzdopen (fileno (ctx->file), "rb");
#endif
Expand Down
7 changes: 4 additions & 3 deletions src/native/libs/System.IO.Compression.Native/extra_libs.cmake
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@

macro(append_extra_compression_libs NativeLibsExtra)
if (CLR_CMAKE_TARGET_BROWSER)
# TODO: remove the mono-style HOST_ variable checks once Mono is using eng/native/configureplatform.cmake to define the CLR_CMAKE_TARGET_ defines
if (CLR_CMAKE_TARGET_BROWSER OR HOST_BROWSER)
# nothing special to link
elseif (CLR_CMAKE_TARGET_ANDROID)
elseif (CLR_CMAKE_TARGET_ANDROID OR HOST_ANDROID)
# need special case here since we want to link against libz.so but find_package() would resolve libz.a
set(ZLIB_LIBRARIES z)
elseif (CLR_CMAKE_TARGET_SUNOS)
elseif (CLR_CMAKE_TARGET_SUNOS OR HOST_SOLARIS)
set(ZLIB_LIBRARIES z m)
else ()
find_package(ZLIB REQUIRED)
Expand Down

0 comments on commit 0e800a6

Please sign in to comment.