Skip to content

Commit ad17a80

Browse files
committed
[yugabyte#27992] YSQL: Use __lsan_ignore_object instead of lsan-supressions.txt for postgres code
Summary: Sometimes (possible after update to clang 19) address sanitizer fails to symbolize leak stack trace: ``` ==85768==WARNING: Can't read from symbolizer at fd 5 ==85768==WARNING: Can't read from symbolizer at fd 5 ==85768==WARNING: Can't read from symbolizer at fd 5 ==85768==WARNING: Can't read from symbolizer at fd 5 ==85768==WARNING: Can't read from symbolizer at fd 5 #0 0x5580d4be8bba (${BUILD_ROOT}/postgres/bin/postgres+0xef0bba) #1 0x5580d587cc4c (${BUILD_ROOT}/postgres/bin/postgres+0x1b84c4c) #2 0x5580d55be5cc (${BUILD_ROOT}/postgres/bin/postgres+0x18c65cc) #3 0x5580d55be171 (${BUILD_ROOT}/postgres/bin/postgres+0x18c6171) #4 0x7fc9991e57e4 (/lib64/libc.so.6+0x3a7e4) (BuildId: 889235a2805b8308b2d0274921bbe1890e9a1986) ``` While actually this stack trace contains supresseed function (postmaster_strdup in this particular scenario), it cannot obtain function name, so detected leak is propagated and test fails. Replaced lsan-suppressions.txt entries with code annotations using `__lsan_ignore_object` to address this issue. Also cleaned up lsan-supressions.txt and resolved issue yugabyte#27496 by adding missing call to YBCFreeStatus. Jira: DB-17615 Test Plan: ./yb_build.sh asan --cxx-test pgwrapper_pg_wait_on_conflict-test --gtest_filter PgTabletSplittingWaitQueuesTest.SplitTablet -n 200 -- -p 8 Reviewers: dmitry, esheng, rthallam Reviewed By: dmitry, esheng, rthallam Subscribers: rthallam, ybase, yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D45426
1 parent 01e90be commit ad17a80

File tree

10 files changed

+68
-32
lines changed

10 files changed

+68
-32
lines changed

build-support/lsan-suppressions.txt

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,7 @@ leak:__res_vinit
2727
# In general, PostgreSQL has not yet been adapted to run correctly with LSAN
2828
# as of 09/2018.
2929

30-
leak:save_ps_display_args
31-
leak:pg_malloc_internal
32-
leak:escape_single_quotes_ascii
3330
leak:gaih_inet
34-
leak:pg_strdup
3531

3632
# When we push down PG/YSQL expression execution we prepare long-lived, thread-local memory
3733
# contexts in DocDB to use for evaluating the expressions. These contexts are never deleted, only
@@ -41,13 +37,6 @@ leak:YbgPrepareMemoryContext
4137
# https://gist.githubusercontent.com/mbautin/015cb594c8281e1afc7ee7b3b5230fce/raw
4238
leak:__strdup
4339

44-
# https://gist.githubusercontent.com/mbautin/864e341c578f30b478da93f59cf098cc/raw
45-
leak:guc_strdup
46-
leak:postmaster_strdup
47-
48-
# https://github.com/yugabyte/yugabyte-db/issues/27496
49-
leak:CloneAndAddErrorCode
50-
5140
# https://github.com/yugabyte/yugabyte-db/issues/14745
5241
leak:ThreadSafeObjectPool
5342

src/postgres/src/backend/access/yb_access/yb_scan.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4064,12 +4064,16 @@ HandleExplicitRowLockStatus(YbcPgExplicitRowLockStatus status)
40644064
if (status.error_info.is_initialized &&
40654065
YBCIsExplicitRowLockConflictStatus(status.ybc_status))
40664066
{
4067+
YBCFreeStatus(status.ybc_status);
40674068
YBCHandleConflictError((OidIsValid(status.error_info.conflicting_table_id) ?
40684069
RelationIdGetRelation(status.error_info.conflicting_table_id) :
40694070
NULL),
40704071
status.error_info.pg_wait_policy);
40714072
}
4072-
HandleYBStatus(status.ybc_status);
4073+
else
4074+
{
4075+
HandleYBStatus(status.ybc_status);
4076+
}
40734077
}
40744078

40754079
/*

src/postgres/src/backend/postmaster/postmaster.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@
149149
#include "storage/procarray.h"
150150
#include "storage/procsignal.h"
151151
#include "storage/sinvaladt.h"
152+
#include "yb/util/debug/leak_annotations.h"
152153
#include "yb/yql/pggate/ybc_pg_shared_mem.h"
153154
#include "yb_ash.h"
154155
#include "yb_query_diagnostics.h"
@@ -598,7 +599,9 @@ HANDLE PostmasterHandle;
598599
char *
599600
postmaster_strdup(const char *in)
600601
{
601-
return strdup(in);
602+
char *result = strdup(in);
603+
__lsan_ignore_object(result);
604+
return result;
602605
}
603606

604607
/*

src/postgres/src/backend/utils/misc/guc.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@
123123
#include "pg_yb_utils.h"
124124
#include "tcop/pquery.h"
125125
#include "utils/syscache.h"
126+
#include "yb/util/debug/leak_annotations.h"
126127
#include "yb_ash.h"
127128
#include "yb_query_diagnostics.h"
128129
#include "yb_tcmalloc_utils.h"
@@ -7332,6 +7333,7 @@ guc_strdup(int elevel, const char *src)
73327333
ereport(elevel,
73337334
(errcode(ERRCODE_OUT_OF_MEMORY),
73347335
errmsg("out of memory")));
7336+
__lsan_ignore_object(data);
73357337
return data;
73367338
}
73377339

src/postgres/src/backend/utils/misc/ps_status.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232
#include "utils/guc.h"
3333
#include "utils/ps_status.h"
3434

35+
/* YB includes */
36+
#include "yb/util/debug/leak_annotations.h"
37+
3538
extern char **environ;
3639
bool update_process_title = true;
3740

@@ -113,6 +116,15 @@ static size_t ps_buffer_fixed_size; /* size of the constant prefix */
113116
static int save_argc;
114117
static char **save_argv;
115118

119+
#if defined(PS_USE_CLOBBER_ARGV)
120+
static char **
121+
allocate_new_environ(int i)
122+
{
123+
char **result = (char **) malloc((i + 1) * sizeof(char *));
124+
__lsan_ignore_object(result);
125+
return result;
126+
}
127+
#endif
116128

117129
/*
118130
* Call this early in startup to save the original argc/argv values.
@@ -202,7 +214,7 @@ save_ps_display_args(int argc, char **argv)
202214
/*
203215
* move the environment out of the way
204216
*/
205-
new_environ = (char **) malloc((i + 1) * sizeof(char *));
217+
new_environ = allocate_new_environ(i);;
206218
if (!new_environ)
207219
{
208220
write_stderr("out of memory\n");

src/postgres/src/common/fe_memutils.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
#include "postgres_fe.h"
2121

22+
/* YB includes */
23+
#include "yb/util/debug/leak_annotations.h"
24+
2225
static inline void *
2326
pg_malloc_internal(size_t size, int flags)
2427
{
@@ -40,6 +43,8 @@ pg_malloc_internal(size_t size, int flags)
4043

4144
if ((flags & MCXT_ALLOC_ZERO) != 0)
4245
MemSet(tmp, 0, size);
46+
47+
__lsan_ignore_object(tmp);
4348
return tmp;
4449
}
4550

@@ -98,6 +103,7 @@ pg_strdup(const char *in)
98103
fprintf(stderr, _("out of memory\n"));
99104
exit(EXIT_FAILURE);
100105
}
106+
__lsan_ignore_object(tmp);
101107
return tmp;
102108
}
103109

src/postgres/src/port/quotes.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
#include "c.h"
1717

18+
/* YB includes */
19+
#include "yb/util/debug/leak_annotations.h"
20+
1821
/*
1922
* Escape (by doubling) any single quotes or backslashes in given string
2023
*
@@ -47,5 +50,6 @@ escape_single_quotes_ascii(const char *src)
4750
result[j++] = src[i];
4851
}
4952
result[j] = '\0';
53+
__lsan_ignore_object(result);
5054
return result;
5155
}

src/yb/util/debug/leak_annotations.h

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,12 @@
3232
#pragma once
3333

3434
// API definitions from LLVM lsan_interface.h
35-
35+
#ifdef __cplusplus
3636
extern "C" {
37-
// Allocations made between calls to __lsan_disable() and __lsan_enable() will
37+
#endif
38+
39+
#if defined(ADDRESS_SANITIZER)
40+
// Allocations made between calls to __lsan_disable() and __lsan_enable() will
3841
// be treated as non-leaks. Disable/enable pairs may be nested.
3942
void __lsan_disable();
4043
void __lsan_enable();
@@ -52,10 +55,23 @@ extern "C" {
5255
// most once per process. This function will terminate the process if there
5356
// are memory leaks and the exit_code flag is non-zero.
5457
void __lsan_do_leak_check();
58+
#elif defined(__cplusplus)
59+
inline void __lsan_disable() {}
60+
inline void __lsan_enable() {}
61+
inline void __lsan_ignore_object(const void *p) {}
62+
#else
63+
static void __lsan_disable() {}
64+
static void __lsan_enable() {}
65+
static void __lsan_ignore_object(const void *p) {}
66+
#endif
67+
68+
#ifdef __cplusplus
5569
} // extern "C"
70+
#endif
71+
72+
#ifdef __cplusplus
5673

57-
namespace yb {
58-
namespace debug {
74+
namespace yb::debug {
5975

6076
class ScopedLSANDisabler {
6177
public:
@@ -76,5 +92,6 @@ class ScopedLSANDisabler {
7692
}
7793
};
7894

79-
} // namespace debug
80-
} // namespace yb
95+
} // namespace yb::debug
96+
97+
#endif

src/yb/util/debug/leakcheck_disabler.h

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,12 @@
3131
//
3232
#pragma once
3333

34-
#include "yb/gutil/macros.h"
3534
#include "yb/util/debug/leak_annotations.h"
3635

37-
namespace yb {
38-
namespace debug {
36+
#ifdef __cplusplus
37+
#include "yb/gutil/macros.h"
38+
39+
namespace yb::debug {
3940

4041
// Scoped object that generically disables LSAN leak checking in a given scope.
4142
// While this object is alive, calls to "new" will not be checked for leaks.
@@ -49,23 +50,21 @@ class ScopedLeakCheckDisabler {
4950
DISALLOW_COPY_AND_ASSIGN(ScopedLeakCheckDisabler);
5051
};
5152

53+
} // namespace yb::debug
54+
55+
#endif
56+
5257
#if defined(__has_feature)
5358
#if __has_feature(address_sanitizer)
5459
#define DISABLE_ASAN __attribute__((no_sanitize("address")))
60+
#define DISABLE_UBSAN __attribute__((no_sanitize("undefined")))
5561
#endif
5662
#endif
63+
5764
#ifndef DISABLE_ASAN
5865
#define DISABLE_ASAN
5966
#endif
6067

61-
#if defined(__has_feature)
62-
#if __has_feature(address_sanitizer)
63-
#define DISABLE_UBSAN __attribute__((no_sanitize("undefined")))
64-
#endif
65-
#endif
6668
#ifndef DISABLE_UBSAN
6769
#define DISABLE_UBSAN
6870
#endif
69-
70-
} // namespace debug
71-
} // namespace yb

src/yb/yql/pggate/pg_perform_future.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Status PatchStatus(const Status& status, const PgObjectIds& relations) {
3636
const auto& actual_status =
3737
PgsqlRequestStatus(status) == PgsqlResponsePB::PGSQL_STATUS_DUPLICATE_KEY_ERROR
3838
? duplicate_key_status : status;
39-
return actual_status.CloneAndAddErrorCode(RelationOid(relations[op_index].object_oid));
39+
return actual_status.CloneAndAddErrorCode(RelationOid(relations[op_index].object_oid));
4040
}
4141
return status;
4242
}

0 commit comments

Comments
 (0)