Skip to content

Commit c937939

Browse files
committed
[#28913] YSQL: Fix dangling std::string::c_str() setting up YbcFlushDebugContext
Summary: cbbb323 adds a few places where we do something like the following: ``` debug_context.strarg2 = row_id.ybctid().ToDebugHexString().c_str(); ``` where `ToDebugHexString()` returns a new `std::string`. The backing `std::string` is destroyed at the end of the assignment, so `debug_context.strarg2` now contains a dangling pointer. This is caught in newer clang versions (clang21) with the following error: ``` ../../src/yb/yql/pggate/pg_operation_buffer.cc:382:35: error: object backing the pointer 'debug_context.strarg2' will be destroyed at the end of the full-expression [-Werror,-Wdangling-assignment-gsl] 382 | debug_context.strarg2 = row_id.ybctid().ToDebugHexString().c_str(); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` Fix this by storing the temporary std::string in a separate variable. Jira: DB-18636 Test Plan: Jenkins. Also compiled locally with clang21 where this fails to compile. Reviewers: kramanathan, jason Reviewed By: kramanathan, jason Subscribers: yql, ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D47365
1 parent a95900b commit c937939

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

src/yb/yql/pggate/pg_operation_buffer.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,12 +374,14 @@ class PgOperationBuffer::Impl {
374374
if (PREDICT_FALSE(!keys_.insert(row_id).second)) {
375375
RETURN_NOT_OK(CheckDuplicateInsertForFastPathCopy(table, stmt_type, need_transaction));
376376

377+
std::string ybctid_hex;
377378
YbcFlushDebugContext debug_context {};
378379
debug_context.reason = YB_CONFLICTING_KEY_WRITE;
379380
debug_context.oidarg = table.pg_table_id().object_oid;
380381
if (need_flush_context) {
381382
debug_context.strarg1 = table.table_name().table_name().c_str();
382-
debug_context.strarg2 = row_id.ybctid().ToDebugHexString().c_str();
383+
ybctid_hex = row_id.ybctid().ToDebugHexString();
384+
debug_context.strarg2 = ybctid_hex.c_str();
383385
}
384386
RETURN_NOT_OK(Flush(debug_context));
385387
keys_.insert(row_id);

src/yb/yql/pggate/pg_session.cc

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,9 +1324,13 @@ Result<int64_t> PgSession::GetCronLastMinute() { return pg_client_.GetCronLastMi
13241324

13251325
Status PgSession::AcquireAdvisoryLock(
13261326
const YbcAdvisoryLockId& lock_id, YbcAdvisoryLockMode mode, bool wait, bool session) {
1327+
std::string lock_id_str;
13271328
YbcFlushDebugContext debug_context {};
13281329
debug_context.reason = YbcFlushReason::YB_ACQUIRE_ADVISORY_LOCK;
1329-
debug_context.strarg1 = yb_debug_log_docdb_requests ? ToString(lock_id).c_str() : nullptr;
1330+
if (yb_debug_log_docdb_requests) {
1331+
lock_id_str = ToString(lock_id);
1332+
debug_context.strarg1 = lock_id_str.c_str();
1333+
}
13301334
RETURN_NOT_OK(FlushBufferedOperations(debug_context));
13311335
tserver::PgAcquireAdvisoryLockRequestPB req;
13321336
AdvisoryLockRequestInitCommon(req, pg_client_.SessionID(), lock_id, mode);
@@ -1379,9 +1383,13 @@ Status PgSession::AcquireObjectLock(const YbcObjectLockId& lock_id, YbcObjectLoc
13791383
return Status::OK();
13801384
}
13811385
VLOG(1) << "Lock acquisition via shared memory not available";
1386+
std::string lock_id_str;
13821387
YbcFlushDebugContext debug_context {};
13831388
debug_context.reason = YbcFlushReason::YB_ACQUIRE_OBJECT_LOCK;
1384-
debug_context.strarg1 = yb_debug_log_docdb_requests ? ToString(lock_id).c_str() : nullptr;
1389+
if (yb_debug_log_docdb_requests) {
1390+
lock_id_str = ToString(lock_id);
1391+
debug_context.strarg1 = lock_id_str.c_str();
1392+
}
13851393

13861394
return pg_txn_manager_->AcquireObjectLock(
13871395
VERIFY_RESULT(FlushBufferedOperations(debug_context)), lock_id, mode);

0 commit comments

Comments
 (0)