Fixed a bug with tree shaking: use a separate MiniTable for statically tree shaken messages.

Since statically tree shaken messages can never later become linked, we should not need to use any of the special code in the decoder.  By using a distinct "empty" message type, we avoid triggering any of this special behavior. This avoids bugs around hazzers and other presence checks.

Also fixed a bug in the cmake staleness test that was causing test failures.

PiperOrigin-RevId: 643036818
pull/17123/head
Joshua Haberman 9 months ago committed by Copybara-Service
parent bf306b3bea
commit d5bd5b90da
  1. 22
      upb/mini_table/internal/message.c
  2. 9
      upb_generator/bootstrap_compiler.bzl
  3. 6
      upb_generator/protoc-gen-upb_minitable.cc

@ -14,7 +14,12 @@
// Must be last.
#include "upb/port/def.inc"
// A MiniTable for an empty message, used for unlinked sub-messages.
// A MiniTable for an empty message, used for unlinked sub-messages that are
// built via MiniDescriptors. Messages that use this MiniTable may possibly
// be linked later, in which case this MiniTable will be replaced with a real
// one. This pattern is known as "dynamic tree shaking", and it introduces
// complication because sub-messages may either be the "empty" type or the
// "real" type. A tagged bit indicates the difference.
const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = {
.UPB_PRIVATE(subs) = NULL,
.UPB_PRIVATE(fields) = NULL,
@ -25,3 +30,18 @@ const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = {
.UPB_PRIVATE(table_mask) = -1,
.UPB_PRIVATE(required_count) = 0,
};
// A MiniTable for a statically tree shaken message. Messages that use this
// MiniTable are guaranteed to remain unlinked; unlike the empty message, this
// MiniTable is never replaced, which greatly simplifies everything, because the
// type of a sub-message is always known, without consulting a tagged bit.
const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken) = {
.UPB_PRIVATE(subs) = NULL,
.UPB_PRIVATE(fields) = NULL,
.UPB_PRIVATE(size) = sizeof(struct upb_Message),
.UPB_PRIVATE(field_count) = 0,
.UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable,
.UPB_PRIVATE(dense_below) = 0,
.UPB_PRIVATE(table_mask) = -1,
.UPB_PRIVATE(required_count) = 0,
};

@ -126,7 +126,14 @@ def _cmake_staleness_test(name, base_dir, src_files, proto_lib_deps, **kwargs):
name = name + "_copy_gencode_%d" % genrule,
outs = ["generated_sources/" + src],
srcs = [name, name + "_minitable"],
cmd = "mkdir -p $(@D); for src in $(SRCS); do cp -f $$src $(@D) || echo 'copy failed!'; done",
cmd = """
mkdir -p $(@D)
for src in $(SRCS); do
if [[ $$src == *%s ]]; then
cp -f $$src $(@D) || echo 'copy failed!'
fi
done
""" % src[src.rfind("/"):],
)
# Keep bazel gencode in sync with our checked-in sources needed for cmake builds.

@ -367,8 +367,8 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools,
IsCrossFile(field)) {
if (seen.insert(pools.GetMiniTable64(field.message_type())).second) {
output(
"__attribute__((weak)) const upb_MiniTable* $0 = "
"&UPB_PRIVATE(_kUpb_MiniTable_Empty);\n",
"__attribute__((weak)) const upb_MiniTable* $0 ="
" &UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken);\n",
MessagePtrName(field.message_type()));
}
}
@ -571,7 +571,7 @@ void WriteMiniTableSourceIncludes(upb::FileDefPtr file, Output& output) {
output(
"extern const struct upb_MiniTable "
"UPB_PRIVATE(_kUpb_MiniTable_Empty);\n");
"UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken);\n");
}
void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file,

Loading…
Cancel
Save