Skip to content

Commit

Permalink
[#16412] docdb: fixed linker flags to detect undefined symbols during…
Browse files Browse the repository at this point in the history
… build rather than in runtime

Summary:
Commit ac72f22 removed linker flags `-Wl,--no-undefined` and `-Wl,--no-allow-shlib-undefined` because some of the shared libraries we build rely e.g. on tcmalloc hooks, which will be only provided at runtime, but not at linking time of the shared library.
As a side effect that change made it impossible for linker to catch undefined symbols and when some symbol is undefined, we only will get failure in runtime.

This revision adds back these linker flags only for executable files, so liker can still catch undefined symbols during build phase, but allow libraries to be linked without errors.

Other changes:
- Split `common/wire_protocol.*` into `common/wire_protocol.*` (common lib) and `common/ql_wire_protocol.*` (ql_common lib) to clean up dependencies and fix some tests and tools linker errors that haven't been caught due to removed flags.
- Added missing `YbgStatusCreateError` to `src/postgres/src/backend/exports-common.txt`

Test Plan: Jenkins

Reviewers: mihnea, smishra, mbautin

Reviewed By: mbautin

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D23480
  • Loading branch information
ttyusupov committed Mar 14, 2023
1 parent 032310b commit 81bf3da
Show file tree
Hide file tree
Showing 52 changed files with 356 additions and 241 deletions.
18 changes: 11 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -693,12 +693,6 @@ if ("${YB_TCMALLOC_ENABLED}" STREQUAL "1" OR
"${YB_BUILD_TYPE}" MATCHES "^(asan|tsan)$")
message("Allowing undefined symbols "
"(YB_TCMALLOC_ENABLED=${YB_TCMALLOC_ENABLED}, build type is ${YB_BUILD_TYPE})")
if(IS_GCC)
# Without this, yb_util cannot get linked with GCC 11 because it references some undefined
# symbols belonging to tcmalloc, e.g. HeapProfilerStart.
# https://gist.githubusercontent.com/mbautin/53e72fbc8c45b4b0d759673fba102ddf/raw
ADD_LINKER_FLAGS("-Wl,--unresolved-symbols=ignore-all")
endif()
if(APPLE)
# See https://bit.ly/3DrOibC on this Apple Clang specific flag.
ADD_LINKER_FLAGS("-undefined dynamic_lookup")
Expand All @@ -708,8 +702,18 @@ if ("${YB_TCMALLOC_ENABLED}" STREQUAL "1" OR
#
# See https://github.com/bazelbuild/bazel/issues/16413 for discussion.
ADD_LINKER_FLAGS("-Wl,-no_fixup_chains")
elseif(IS_GCC)
# Without this, yb_util cannot get linked with GCC 11 because it references some undefined
# symbols belonging to tcmalloc, e.g. HeapProfilerStart.
# https://gist.githubusercontent.com/mbautin/53e72fbc8c45b4b0d759673fba102ddf/raw
ADD_LINKER_FLAGS("-Wl,--unresolved-symbols=ignore-all")
else()
ADD_LINKER_FLAGS("-Wl,--allow-shlib-undefined")
# Allow undefined symbols when linking libs, so not resolved references to tcmalloc symbols
# does not break libs linking.
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--allow-shlib-undefined")
# Don't allow undefined symbols when linking executables, so we can catch undefined symbols
# during build phase rather than later in runtime.
ADD_EXE_LINKER_FLAGS("-Wl,--no-undefined -Wl,--no-allow-shlib-undefined")
endif()

if(APPLE)
Expand Down
3 changes: 2 additions & 1 deletion src/yb/cdc/cdcsdk_producer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@

#include "yb/client/client.h"
#include "yb/client/yb_table_name.h"
#include "yb/common/wire_protocol.h"

#include "yb/common/ql_expr.h"
#include "yb/common/ql_wire_protocol.h"

#include "yb/consensus/consensus.messages.h"

Expand Down
2 changes: 1 addition & 1 deletion src/yb/client/client-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
#include "yb/common/redis_constants_common.h"
#include "yb/common/placement_info.h"
#include "yb/common/schema.h"
#include "yb/common/wire_protocol.h"
#include "yb/common/ql_wire_protocol.h"

#include "yb/gutil/bind.h"
#include "yb/gutil/map-util.h"
Expand Down
1 change: 0 additions & 1 deletion src/yb/client/client-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
#include "yb/common/entity_ids.h"
#include "yb/common/index.h"
#include "yb/common/transaction.h"
#include "yb/common/wire_protocol.h"

#include "yb/master/master_fwd.h"
#include "yb/master/master_admin.fwd.h"
Expand Down
2 changes: 1 addition & 1 deletion src/yb/client/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
#include "yb/common/roles_permissions.h"
#include "yb/common/schema.h"
#include "yb/common/transaction.h"
#include "yb/common/wire_protocol.h"
#include "yb/common/ql_wire_protocol.h"

#include "yb/gutil/bind.h"
#include "yb/gutil/map-util.h"
Expand Down
1 change: 0 additions & 1 deletion src/yb/client/client_builder-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@

#include "yb/common/common_net.pb.h"
#include "yb/common/entity_ids.h"
#include "yb/common/wire_protocol.h"

#include "yb/gutil/ref_counted.h"

Expand Down
2 changes: 2 additions & 0 deletions src/yb/client/client_master_rpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "yb/client/client.h"
#include "yb/client/client-internal.h"

#include "yb/common/wire_protocol.h"

#include "yb/master/master_fwd.h"
#include "yb/master/master_error.h"

Expand Down
1 change: 0 additions & 1 deletion src/yb/client/meta_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@

#include "yb/common/partition.h"
#include "yb/common/placement_info.h"
#include "yb/common/wire_protocol.h"
#include "yb/consensus/metadata.pb.h"

#include "yb/gutil/macros.h"
Expand Down
2 changes: 1 addition & 1 deletion src/yb/client/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#include "yb/common/partial_row.h"
#include "yb/common/ql_type.h"
#include "yb/common/schema.h"
#include "yb/common/wire_protocol.h"
#include "yb/common/ql_wire_protocol.h"

#include "yb/gutil/map-util.h"
#include "yb/gutil/strings/substitute.h"
Expand Down
1 change: 1 addition & 0 deletions src/yb/client/table_alterer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "yb/client/client-internal.h"
#include "yb/client/schema-internal.h"

#include "yb/common/ql_wire_protocol.h"
#include "yb/common/schema.h"
#include "yb/common/transaction.h"

Expand Down
2 changes: 1 addition & 1 deletion src/yb/client/table_creator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
#include "yb/client/client.h"
#include "yb/client/table_info.h"

#include "yb/common/ql_wire_protocol.h"
#include "yb/common/schema.h"
#include "yb/common/transaction.h"
#include "yb/common/wire_protocol.h"

#include "yb/master/master_ddl.pb.h"

Expand Down
3 changes: 2 additions & 1 deletion src/yb/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ set(COMMON_SRCS
termination_monitor.cc
transaction.cc
transaction_error.cc
wire_protocol.cc
ybc_util.cc
ybc-internal.cc
llvm_profile_dumper.cc
Expand Down Expand Up @@ -129,7 +130,7 @@ set(QL_SRCS
ql_scanspec.cc
row.cc
schema.cc
wire_protocol.cc
ql_wire_protocol.cc
)

set(QL_LIBS
Expand Down
184 changes: 184 additions & 0 deletions src/yb/common/ql_wire_protocol.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//
// The following only applies to changes made to this file as part of YugaByte development.
//
// Portions Copyright (c) YugaByte, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
// in compliance with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software distributed under the License
// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
// or implied. See the License for the specific language governing permissions and limitations
// under the License.
//
#include "yb/common/ql_wire_protocol.h"

#include <string>
#include <vector>

#include "yb/common/common.pb.h"
#include "yb/common/ql_type.h"
#include "yb/common/schema.h"
#include "yb/common/wire_protocol.messages.h"

namespace yb {

void SchemaToColocatedTableIdentifierPB(
const Schema& schema, ColocatedTableIdentifierPB* colocated_pb) {
if (schema.has_colocation_id()) {
colocated_pb->set_colocation_id(schema.colocation_id());
} else if (schema.has_cotable_id()) {
colocated_pb->set_cotable_id(schema.cotable_id().ToString());
}
}

void SchemaToPB(const Schema& schema, SchemaPB *pb, int flags) {
pb->Clear();
SchemaToColocatedTableIdentifierPB(schema, pb->mutable_colocated_table_id());
SchemaToColumnPBs(schema, pb->mutable_columns(), flags);
schema.table_properties().ToTablePropertiesPB(pb->mutable_table_properties());
pb->set_pgschema_name(schema.SchemaName());
}

void SchemaToPBWithoutIds(const Schema& schema, SchemaPB *pb) {
pb->Clear();
SchemaToColumnPBs(schema, pb->mutable_columns(), SCHEMA_PB_WITHOUT_IDS);
}

Status SchemaFromPB(const SchemaPB& pb, Schema *schema) {
// Conver the columns.
std::vector<ColumnSchema> columns;
std::vector<ColumnId> column_ids;
int num_key_columns = 0;
RETURN_NOT_OK(ColumnPBsToColumnTuple(pb.columns(), &columns, &column_ids, &num_key_columns));

// Convert the table properties.
TableProperties table_properties = TableProperties::FromTablePropertiesPB(pb.table_properties());
RETURN_NOT_OK(schema->Reset(columns, column_ids, num_key_columns, table_properties));

if(pb.has_pgschema_name()) {
schema->SetSchemaName(pb.pgschema_name());
}

if (pb.has_colocated_table_id()) {
switch (pb.colocated_table_id().value_case()) {
case ColocatedTableIdentifierPB::kCotableId: {
schema->set_cotable_id(
VERIFY_RESULT(Uuid::FromString(pb.colocated_table_id().cotable_id())));
break;
}
case ColocatedTableIdentifierPB::kColocationId:
schema->set_colocation_id(pb.colocated_table_id().colocation_id());
break;
case ColocatedTableIdentifierPB::VALUE_NOT_SET:
break;
}
}
return Status::OK();
}

void ColumnSchemaToPB(const ColumnSchema& col_schema, ColumnSchemaPB *pb, int flags) {
pb->Clear();
pb->set_name(col_schema.name());
col_schema.type()->ToQLTypePB(pb->mutable_type());
pb->set_is_nullable(col_schema.is_nullable());
pb->set_is_static(col_schema.is_static());
pb->set_is_counter(col_schema.is_counter());
pb->set_order(col_schema.order());
pb->set_sorting_type(col_schema.sorting_type());
pb->set_pg_type_oid(col_schema.pg_type_oid());
// We only need to process the *hash* primary key here. The regular primary key is set by the
// conversion for SchemaPB. The reason is that ColumnSchema and ColumnSchemaPB are not matching
// 1 to 1 as ColumnSchema doesn't have "is_key" field. That was Kudu's code, and we keep it that
// way for now.
if (col_schema.is_hash_key()) {
pb->set_is_key(true);
pb->set_is_hash_key(true);
}
}


ColumnSchema ColumnSchemaFromPB(const ColumnSchemaPB& pb) {
// Only "is_hash_key" is used to construct ColumnSchema. The field "is_key" will be read when
// processing SchemaPB.
return ColumnSchema(pb.name(), QLType::FromQLTypePB(pb.type()), pb.is_nullable(),
pb.is_hash_key(), pb.is_static(), pb.is_counter(), pb.order(),
SortingType(pb.sorting_type()), pb.pg_type_oid());
}

Status ColumnPBsToColumnTuple(
const google::protobuf::RepeatedPtrField<ColumnSchemaPB>& column_pbs,
std::vector<ColumnSchema>* columns , std::vector<ColumnId>* column_ids, int* num_key_columns) {
columns->reserve(column_pbs.size());
bool is_handling_key = true;
for (const ColumnSchemaPB& pb : column_pbs) {
columns->push_back(ColumnSchemaFromPB(pb));
if (pb.is_key()) {
if (!is_handling_key) {
return STATUS(InvalidArgument,
"Got out-of-order key column", pb.ShortDebugString());
}
(*num_key_columns)++;
} else {
is_handling_key = false;
}
if (pb.has_id()) {
column_ids->push_back(ColumnId(pb.id()));
}
}

DCHECK_LE((*num_key_columns), columns->size());
return Status::OK();
}

Status ColumnPBsToSchema(const google::protobuf::RepeatedPtrField<ColumnSchemaPB>& column_pbs,
Schema* schema) {

std::vector<ColumnSchema> columns;
std::vector<ColumnId> column_ids;
int num_key_columns = 0;
RETURN_NOT_OK(ColumnPBsToColumnTuple(column_pbs, &columns, &column_ids, &num_key_columns));

// TODO(perf): could make the following faster by adding a
// Reset() variant which actually takes ownership of the column
// vector.
return schema->Reset(columns, column_ids, num_key_columns);
}

void SchemaToColumnPBs(const Schema& schema,
google::protobuf::RepeatedPtrField<ColumnSchemaPB>* cols,
int flags) {
cols->Clear();
size_t idx = 0;
for (const ColumnSchema& col : schema.columns()) {
ColumnSchemaPB* col_pb = cols->Add();
ColumnSchemaToPB(col, col_pb);
col_pb->set_is_key(idx < schema.num_key_columns());

if (schema.has_column_ids() && !(flags & SCHEMA_PB_WITHOUT_IDS)) {
col_pb->set_id(schema.column_id(idx));
}

idx++;
}
}

} // namespace yb

0 comments on commit 81bf3da

Please sign in to comment.