Skip to content

Commit

Permalink
[trace] Remove default definitions of weak symbols
Browse files Browse the repository at this point in the history
Instead of providing a default no-op implementation, check the symbols
for `NULL` before accessing them. Providing a default implementation
doesn't reliably work with dynamic linking. Depending on link order the
default implementations may not be overridden. By skipping the default
implementation, all link order issues are resolved. If the symbols
aren't provided the weak function will be `NULL`.
  • Loading branch information
terrelln committed Apr 26, 2021
1 parent 3e2fbfd commit 6cee3c2
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 52 deletions.
1 change: 0 additions & 1 deletion build/meson/lib/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ libzstd_sources = [join_paths(zstd_rootdir, 'lib/common/entropy_common.c'),
join_paths(zstd_rootdir, 'lib/common/threading.c'),
join_paths(zstd_rootdir, 'lib/common/pool.c'),
join_paths(zstd_rootdir, 'lib/common/zstd_common.c'),
join_paths(zstd_rootdir, 'lib/common/zstd_trace.c'),
join_paths(zstd_rootdir, 'lib/common/error_private.c'),
join_paths(zstd_rootdir, 'lib/common/xxhash.c'),
join_paths(zstd_rootdir, 'lib/compress/hist.c'),
Expand Down
42 changes: 0 additions & 42 deletions lib/common/zstd_trace.c

This file was deleted.

10 changes: 6 additions & 4 deletions lib/common/zstd_trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,15 @@ typedef unsigned long long ZSTD_TraceCtx;
* @returns Non-zero if tracing is enabled. The return value is
* passed to ZSTD_trace_compress_end().
*/
ZSTD_TraceCtx ZSTD_trace_compress_begin(struct ZSTD_CCtx_s const* cctx);
ZSTD_WEAK_ATTR ZSTD_TraceCtx ZSTD_trace_compress_begin(
struct ZSTD_CCtx_s const* cctx);

/**
* Trace the end of a compression call.
* @param ctx The return value of ZSTD_trace_compress_begin().
* @param trace The zstd tracing info.
*/
void ZSTD_trace_compress_end(
ZSTD_WEAK_ATTR void ZSTD_trace_compress_end(
ZSTD_TraceCtx ctx,
ZSTD_Trace const* trace);

Expand All @@ -132,14 +133,15 @@ void ZSTD_trace_compress_end(
* @returns Non-zero if tracing is enabled. The return value is
* passed to ZSTD_trace_compress_end().
*/
ZSTD_TraceCtx ZSTD_trace_decompress_begin(struct ZSTD_DCtx_s const* dctx);
ZSTD_WEAK_ATTR ZSTD_TraceCtx ZSTD_trace_decompress_begin(
struct ZSTD_DCtx_s const* dctx);

/**
* Trace the end of a decompression call.
* @param ctx The return value of ZSTD_trace_decompress_begin().
* @param trace The zstd tracing info.
*/
void ZSTD_trace_decompress_end(
ZSTD_WEAK_ATTR void ZSTD_trace_decompress_end(
ZSTD_TraceCtx ctx,
ZSTD_Trace const* trace);

Expand Down
6 changes: 3 additions & 3 deletions lib/compress/zstd_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -4379,7 +4379,7 @@ static size_t ZSTD_compressBegin_internal(ZSTD_CCtx* cctx,
ZSTD_buffered_policy_e zbuff)
{
#if ZSTD_TRACE
cctx->traceCtx = ZSTD_trace_compress_begin(cctx);
cctx->traceCtx = (ZSTD_trace_compress_begin != NULL) ? ZSTD_trace_compress_begin(cctx) : 0;
#endif
DEBUGLOG(4, "ZSTD_compressBegin_internal: wlog=%u", params->cParams.windowLog);
/* params are supposed to be fully validated at this point */
Expand Down Expand Up @@ -4510,7 +4510,7 @@ static size_t ZSTD_writeEpilogue(ZSTD_CCtx* cctx, void* dst, size_t dstCapacity)
void ZSTD_CCtx_trace(ZSTD_CCtx* cctx, size_t extraCSize)
{
#if ZSTD_TRACE
if (cctx->traceCtx) {
if (cctx->traceCtx && ZSTD_trace_compress_end != NULL) {
int const streaming = cctx->inBuffSize > 0 || cctx->outBuffSize > 0 || cctx->appliedParams.nbWorkers > 0;
ZSTD_Trace trace;
ZSTD_memset(&trace, 0, sizeof(trace));
Expand Down Expand Up @@ -5456,7 +5456,7 @@ static size_t ZSTD_CCtx_init_compressStream2(ZSTD_CCtx* cctx,
}
if (params.nbWorkers > 0) {
#if ZSTD_TRACE
cctx->traceCtx = ZSTD_trace_compress_begin(cctx);
cctx->traceCtx = (ZSTD_trace_compress_begin != NULL) ? ZSTD_trace_compress_begin(cctx) : 0;
#endif
/* mt context creation */
if (cctx->mtctx == NULL) {
Expand Down
4 changes: 2 additions & 2 deletions lib/decompress/zstd_decompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ static size_t ZSTD_setRleBlock(void* dst, size_t dstCapacity,
static void ZSTD_DCtx_trace_end(ZSTD_DCtx const* dctx, U64 uncompressedSize, U64 compressedSize, unsigned streaming)
{
#if ZSTD_TRACE
if (dctx->traceCtx) {
if (dctx->traceCtx && ZSTD_trace_decompress_end != NULL) {
ZSTD_Trace trace;
ZSTD_memset(&trace, 0, sizeof(trace));
trace.version = ZSTD_VERSION_NUMBER;
Expand Down Expand Up @@ -1383,7 +1383,7 @@ size_t ZSTD_decompressBegin(ZSTD_DCtx* dctx)
{
assert(dctx != NULL);
#if ZSTD_TRACE
dctx->traceCtx = ZSTD_trace_decompress_begin(dctx);
dctx->traceCtx = (ZSTD_trace_decompress_begin != NULL) ? ZSTD_trace_decompress_begin(dctx) : 0;
#endif
dctx->expected = ZSTD_startingInputLength(dctx->format); /* dctx->format must be properly set */
dctx->stage = ZSTDds_getFrameHeaderSize;
Expand Down

1 comment on commit 6cee3c2

@MaskRay
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange. A weak definition is more portable than a weak reference. Weak references traditionally had some GNU toolchain bugs for some archaic architectures. https://maskray.me/blog/2021-04-25-weak-symbol

Please sign in to comment.