Skip to content

Commit

Permalink
PFS_events_statements cleanup: Use offsetof
Browse files Browse the repository at this point in the history 
The macro my_offsetof() performs pointer arithmetics that may be
undefined behavior. As reported in MDEV-26272, it may cause
clang -fsanitize=undefined to generate invalid memory references.

struct PFS_events_statements: Convert to std::is_standard_layout
by encapsulating the standard-layout struct PFS_events instead of
deriving from it, so that the standard macro offsetof() can be used.

PFS_events_statements::copy(): Renamed from copy_events_statements().
A cast to void* is now needed in memcpy() to avoid GCC -Wclass-memaccess
"writing to an object ... leaves 64 bytes unchanged".
  • Loading branch information
@dr-m
dr-m committed Nov 24, 2021
1 parent 8d39871 commit 6b2b510
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 55 deletions.
50 changes: 25 additions & 25 deletions storage/perfschema/pfs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4841,8 +4841,8 @@ pfs_start_stage_v1(PSI_stage_key key, const char *src_file, int src_line)
pfs->m_class= NULL;

/* New waits will now be attached directly to the parent statement. */
child_wait->m_event_id= parent_statement->m_event_id;
child_wait->m_event_type= parent_statement->m_event_type;
child_wait->m_event_id= parent_statement->m_event.m_event_id;
child_wait->m_event_type= parent_statement->m_event.m_event_type;
/* See below for new stages, that may overwrite this. */
}

Expand Down Expand Up @@ -4957,8 +4957,8 @@ void pfs_end_stage_v1()
/* New waits will now be attached directly to the parent statement. */
PFS_events_waits *child_wait= & pfs_thread->m_events_waits_stack[0];
PFS_events_statements *parent_statement= & pfs_thread->m_statement_stack[0];
child_wait->m_event_id= parent_statement->m_event_id;
child_wait->m_event_type= parent_statement->m_event_type;
child_wait->m_event_id= parent_statement->m_event.m_event_id;
child_wait->m_event_type= parent_statement->m_event.m_event_type;

/* This stage is completed */
pfs->m_class= NULL;
Expand Down Expand Up @@ -5009,13 +5009,13 @@ pfs_get_thread_statement_locker_v1(PSI_statement_locker_state *state,
pfs_dirty_state dirty_state;
pfs_thread->m_stmt_lock.allocated_to_dirty(& dirty_state);
PFS_events_statements *pfs= & pfs_thread->m_statement_stack[pfs_thread->m_events_statements_count];
pfs->m_thread_internal_id= pfs_thread->m_thread_internal_id;
pfs->m_event_id= event_id;
pfs->m_event_type= EVENT_TYPE_STATEMENT;
pfs->m_end_event_id= 0;
pfs->m_class= klass;
pfs->m_timer_start= 0;
pfs->m_timer_end= 0;
pfs->m_event.m_thread_internal_id= pfs_thread->m_thread_internal_id;
pfs->m_event.m_event_id= event_id;
pfs->m_event.m_event_type= EVENT_TYPE_STATEMENT;
pfs->m_event.m_end_event_id= 0;
pfs->m_event.m_class= klass;
pfs->m_event.m_timer_start= 0;
pfs->m_event.m_timer_end= 0;
pfs->m_lock_time= 0;
pfs->m_current_schema_name_length= 0;
pfs->m_sqltext_length= 0;
Expand Down Expand Up @@ -5065,9 +5065,9 @@ pfs_get_thread_statement_locker_v1(PSI_statement_locker_state *state,
if (pfs_thread->m_events_statements_count > 0)
{
parent_statement= pfs - 1;
parent_event= parent_statement->m_event_id;
parent_type= parent_statement->m_event_type;
parent_level= parent_statement->m_nesting_event_level + 1;
parent_event= parent_statement->m_event.m_event_id;
parent_type= parent_statement->m_event.m_event_type;
parent_level= parent_statement->m_event.m_nesting_event_level + 1;
}

if (parent_transaction->m_state == TRANS_STATE_ACTIVE &&
Expand All @@ -5077,9 +5077,9 @@ pfs_get_thread_statement_locker_v1(PSI_statement_locker_state *state,
parent_type= parent_transaction->m_event_type;
}

pfs->m_nesting_event_id= parent_event;
pfs->m_nesting_event_type= parent_type;
pfs->m_nesting_event_level= parent_level;
pfs->m_event.m_nesting_event_id= parent_event;
pfs->m_event.m_nesting_event_type= parent_type;
pfs->m_event.m_nesting_event_level= parent_level;

/* Set parent Stored Procedure information for this statement. */
if(sp_share)
Expand Down Expand Up @@ -5197,7 +5197,7 @@ pfs_refine_statement_v1(PSI_statement_locker *locker,
DBUG_ASSERT(pfs != NULL);

/* mutate EVENTS_STATEMENTS_CURRENT.EVENT_NAME */
pfs->m_class= klass;
pfs->m_event.m_class= klass;
}

state->m_class= klass;
Expand Down Expand Up @@ -5233,9 +5233,9 @@ void pfs_start_statement_v1(PSI_statement_locker *locker,
PFS_events_statements *pfs= reinterpret_cast<PFS_events_statements*> (state->m_statement);
DBUG_ASSERT(pfs != NULL);

pfs->m_timer_start= timer_start;
pfs->m_source_file= src_file;
pfs->m_source_line= src_line;
pfs->m_event.m_timer_start= timer_start;
pfs->m_event.m_source_file= src_file;
pfs->m_event.m_source_line= src_line;

DBUG_ASSERT(db_len <= sizeof(pfs->m_current_schema_name));
if (db_len > 0)
Expand Down Expand Up @@ -5492,8 +5492,8 @@ void pfs_end_statement_v1(PSI_statement_locker *locker, void *stmt_da)
break;
}

pfs->m_timer_end= timer_end;
pfs->m_end_event_id= thread->m_event_id;
pfs->m_event.m_timer_end= timer_end;
pfs->m_event.m_end_event_id= thread->m_event_id;

if (digest_storage != NULL)
{
Expand Down Expand Up @@ -5970,8 +5970,8 @@ pfs_get_thread_transaction_locker_v1(PSI_transaction_locker_state *state,
{
PFS_events_statements *pfs_statement=
&pfs_thread->m_statement_stack[statements_count - 1];
pfs->m_nesting_event_id= pfs_statement->m_event_id;
pfs->m_nesting_event_type= pfs_statement->m_event_type;
pfs->m_nesting_event_id= pfs_statement->m_event.m_event_id;
pfs->m_nesting_event_type= pfs_statement->m_event.m_event_type;
}
else
{
Expand Down
25 changes: 12 additions & 13 deletions storage/perfschema/pfs_events_statements.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,27 +147,26 @@ void cleanup_events_statements_history_long(void)
h_long_stmts_text_array= NULL;
}

static inline void copy_events_statements(PFS_events_statements *dest,
const PFS_events_statements *source)
inline void PFS_events_statements::copy(const PFS_events_statements &source)
{
/* Copy all attributes except SQL TEXT and DIGEST */
memcpy(dest, source, my_offsetof(PFS_events_statements, m_sqltext));
memcpy((void*) this, &source, offsetof(PFS_events_statements, m_sqltext));

/* Copy SQL TEXT */
int sqltext_length= source->m_sqltext_length;
int sqltext_length= source.m_sqltext_length;

if (sqltext_length > 0)
{
memcpy(dest->m_sqltext, source->m_sqltext, sqltext_length);
dest->m_sqltext_length= sqltext_length;
memcpy(m_sqltext, source.m_sqltext, sqltext_length);
m_sqltext_length= sqltext_length;
}
else
{
dest->m_sqltext_length= 0;
m_sqltext_length= 0;
}

/* Copy DIGEST */
dest->m_digest_storage.copy(& source->m_digest_storage);
m_digest_storage.copy(&source.m_digest_storage);
}

/**
Expand All @@ -192,7 +191,7 @@ void insert_events_statements_history(PFS_thread *thread, PFS_events_statements
to make this thread (the writer) faster.
This is ok, the readers of m_statements_history will filter this out.
*/
copy_events_statements(&thread->m_statements_history[index], statement);
thread->m_statements_history[index].copy(*statement);

index++;
if (index >= events_statements_history_per_thread)
Expand Down Expand Up @@ -221,7 +220,7 @@ void insert_events_statements_history_long(PFS_events_statements *statement)
events_statements_history_long_full= true;

/* See related comment in insert_events_statements_history. */
copy_events_statements(&events_statements_history_long_array[index], statement);
events_statements_history_long_array[index].copy(*statement);
}

static void fct_reset_events_statements_current(PFS_thread *pfs_thread)
Expand All @@ -230,7 +229,7 @@ static void fct_reset_events_statements_current(PFS_thread *pfs_thread)
PFS_events_statements *pfs_stmt_last= pfs_stmt + statement_stack_max;

for ( ; pfs_stmt < pfs_stmt_last; pfs_stmt++)
pfs_stmt->m_class= NULL;
pfs_stmt->m_event.m_class= nullptr;
}

/** Reset table EVENTS_STATEMENTS_CURRENT data. */
Expand All @@ -247,7 +246,7 @@ static void fct_reset_events_statements_history(PFS_thread *pfs_thread)
pfs_thread->m_statements_history_index= 0;
pfs_thread->m_statements_history_full= false;
for ( ; pfs < pfs_last; pfs++)
pfs->m_class= NULL;
pfs->m_event.m_class= nullptr;
}

/** Reset table EVENTS_STATEMENTS_HISTORY data. */
Expand All @@ -265,7 +264,7 @@ void reset_events_statements_history_long(void)
PFS_events_statements *pfs= events_statements_history_long_array;
PFS_events_statements *pfs_last= pfs + events_statements_history_long_size;
for ( ; pfs < pfs_last; pfs++)
pfs->m_class= NULL;
pfs->m_event.m_class= nullptr;
}

static void fct_reset_events_statements_by_thread(PFS_thread *thread)
Expand Down
5 changes: 4 additions & 1 deletion storage/perfschema/pfs_events_statements.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ struct PFS_user;
struct PFS_host;

/** A statement record. */
struct PFS_events_statements : public PFS_events
struct PFS_events_statements
{
PFS_events m_event;
enum_object_type m_sp_type;
char m_schema_name[NAME_LEN];
uint m_schema_name_length;
Expand Down Expand Up @@ -117,6 +118,8 @@ struct PFS_events_statements : public PFS_events
and always point to pre allocated memory.
*/
sql_digest_storage m_digest_storage;

inline void copy(const PFS_events_statements &source);
};

void insert_events_statements_history(PFS_thread *thread, PFS_events_statements *statement);
Expand Down
4 changes: 2 additions & 2 deletions storage/perfschema/pfs_prepared_stmt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ create_prepared_stmt(void *identity,
if (pfs_stmt)
{
if (pfs_program)
pfs->m_owner_event_id= pfs_stmt->m_nesting_event_id;
pfs->m_owner_event_id= pfs_stmt->m_event.m_nesting_event_id;
else
pfs->m_owner_event_id= pfs_stmt->m_event_id;
pfs->m_owner_event_id= pfs_stmt->m_event.m_event_id;
}

/* Insert this record. */
Expand Down
29 changes: 15 additions & 14 deletions storage/perfschema/table_events_statements.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,28 +228,29 @@ void table_events_statements_common::make_row_part_1(PFS_events_statements *stat

m_row_exists= false;

PFS_statement_class *unsafe= (PFS_statement_class*) statement->m_class;
PFS_statement_class *unsafe= (PFS_statement_class*)
statement->m_event.m_class;
PFS_statement_class *klass= sanitize_statement_class(unsafe);
if (unlikely(klass == NULL))
return;

m_row.m_thread_internal_id= statement->m_thread_internal_id;
m_row.m_event_id= statement->m_event_id;
m_row.m_end_event_id= statement->m_end_event_id;
m_row.m_nesting_event_id= statement->m_nesting_event_id;
m_row.m_nesting_event_type= statement->m_nesting_event_type;
m_row.m_nesting_event_level= statement->m_nesting_event_level;
m_row.m_thread_internal_id= statement->m_event.m_thread_internal_id;
m_row.m_event_id= statement->m_event.m_event_id;
m_row.m_end_event_id= statement->m_event.m_end_event_id;
m_row.m_nesting_event_id= statement->m_event.m_nesting_event_id;
m_row.m_nesting_event_type= statement->m_event.m_nesting_event_type;
m_row.m_nesting_event_level= statement->m_event.m_nesting_event_level;

if (m_row.m_end_event_id == 0)
{
timer_end= get_timer_raw_value(statement_timer);
}
else
{
timer_end= statement->m_timer_end;
timer_end= statement->m_event.m_timer_end;
}

m_normalizer->to_pico(statement->m_timer_start, timer_end,
m_normalizer->to_pico(statement->m_event.m_timer_start, timer_end,
& m_row.m_timer_start, & m_row.m_timer_end, & m_row.m_timer_wait);
m_row.m_lock_time= statement->m_lock_time * MICROSEC_TO_PICOSEC;

Expand Down Expand Up @@ -662,7 +663,7 @@ int table_events_statements_current::rnd_pos(const void *pos)

statement= &pfs_thread->m_statement_stack[m_pos.m_index_2];

if (statement->m_class != NULL)
if (statement->m_event.m_class)
{
make_row(pfs_thread, statement);
return 0;
Expand Down Expand Up @@ -762,7 +763,7 @@ int table_events_statements_history::rnd_next(void)

statement= &pfs_thread->m_statements_history[m_pos.m_index_2];

if (statement->m_class != NULL)
if (statement->m_event.m_class)
{
make_row(pfs_thread, statement);
/* Next iteration, look for the next history in this thread */
Expand Down Expand Up @@ -793,7 +794,7 @@ int table_events_statements_history::rnd_pos(const void *pos)
return HA_ERR_RECORD_DELETED;

statement= &pfs_thread->m_statements_history[m_pos.m_index_2];
if (statement->m_class != NULL)
if (statement->m_event.m_class)
{
make_row(pfs_thread, statement);
return 0;
Expand Down Expand Up @@ -876,7 +877,7 @@ int table_events_statements_history_long::rnd_next(void)
{
statement= &events_statements_history_long_array[m_pos.m_index];

if (statement->m_class != NULL)
if (statement->m_event.m_class)
{
make_row(statement);
/* Next iteration, look for the next entry */
Expand Down Expand Up @@ -908,7 +909,7 @@ int table_events_statements_history_long::rnd_pos(const void *pos)

statement= &events_statements_history_long_array[m_pos.m_index];

if (statement->m_class == NULL)
if (!statement->m_event.m_class)
return HA_ERR_RECORD_DELETED;

make_row(statement);
Expand Down

0 comments on commit 6b2b510

Please sign in to comment.