Fixed two GCC-only issues around upb's generated extension registry.

This change refactors the generated `upb_MiniTableExtension` variables to be `const upb_MiniTableExtension*` instead of `const upb_MiniTableExtension`. The linker array now stores pointers to these extension objects.  This fixes an issue related to position-independent executables on GCC (see the attached bug for more information).

We also unconditionally retain all extensions for the generated registry when compiling under GCC.  This effectively disables tree shaking of extensions when compiling with GCC, but fixes a GCC-only compiler error.  There may be a way to re-enable linker GC with GCC at some point, but the priority for the moment is just fixing the issue.

A new test is added to verify registry loading with no linked extensions.

Fixes: #26385
PiperOrigin-RevId: 906412231

Author: haberman

hpb/backend/upb/extension.h

@@ -184,19 +184,19 @@ class ExtensionIdentifier {
 
   // Placeholder for extant legacy callers, avoid use if possible
   const upb_MiniTableExtension* mini_table_ext() const {
-    return mini_table_ext_;
+    return *mini_table_ext_;
   }
 
  private:
   constexpr explicit ExtensionIdentifier(
-      const upb_MiniTableExtension* mte,
+      const upb_MiniTableExtension* const* mte,
       typename UpbExtensionTrait<ExtensionType>::DefaultType val,
       uint32_t number)
       : mini_table_ext_(mte), default_val_(val), number_(number) {}
 
   constexpr uint32_t number() const { return number_; }
 
-  const upb_MiniTableExtension* mini_table_ext_;
+  const upb_MiniTableExtension* const* mini_table_ext_;
 
   typename UpbExtensionTrait<ExtensionType>::ReturnType default_value() const {
     if constexpr (IsHpbClass<ExtensionType>) {

upb/BUILD

@@ -303,6 +303,7 @@ filegroup(
         "//src/google/protobuf:well_known_type_protos",
         "//upb/json:test_protos",
         "//upb/message:test_protos",
+        "//upb/mini_table:test_protos",
         "//upb/test:test_protos",
         "//upb/util:test_protos",
     ],

upb/message/promote_test.cc

@@ -81,14 +81,12 @@ TEST(GeneratedCode, FindUnknown) {
 
   upb_FindUnknownRet result = upb_Message_FindUnknown(
       UPB_UPCAST(base_msg),
-      upb_MiniTableExtension_Number(&upb_test_ModelExtension1_model_ext_ext),
-      0);
+      upb_MiniTableExtension_Number(upb_test_ModelExtension1_model_ext_ext), 0);
   EXPECT_EQ(kUpb_FindUnknown_Ok, result.status);
 
   result = upb_Message_FindUnknown(
       UPB_UPCAST(base_msg),
-      upb_MiniTableExtension_Number(&upb_test_ModelExtension2_model_ext_ext),
-      0);
+      upb_MiniTableExtension_Number(upb_test_ModelExtension2_model_ext_ext), 0);
   EXPECT_EQ(kUpb_FindUnknown_NotPresent, result.status);
 
   upb_Arena_Free(arena);
@@ -125,7 +123,7 @@ TEST(GeneratedCode, PromoteFromMultiple) {
 
   upb_MessageValue value;
   upb_GetExtension_Status result = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(parsed), &upb_test_ModelExtension1_model_ext_ext, options,
+      UPB_UPCAST(parsed), upb_test_ModelExtension1_model_ext_ext, options,
       arena, &value);
   ASSERT_EQ(result, kUpb_GetExtension_Ok);
   upb_test_ModelExtension1* parsed_ex =
@@ -135,8 +133,7 @@ TEST(GeneratedCode, PromoteFromMultiple) {
 
   upb_FindUnknownRet found = upb_Message_FindUnknown(
       UPB_UPCAST(parsed),
-      upb_MiniTableExtension_Number(&upb_test_ModelExtension1_model_ext_ext),
-      0);
+      upb_MiniTableExtension_Number(upb_test_ModelExtension1_model_ext_ext), 0);
   EXPECT_EQ(kUpb_FindUnknown_NotPresent, found.status);
 
   upb_Arena_Free(arena);
@@ -188,7 +185,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test known GetExtension 1
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension1_model_ext_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension1_model_ext_ext, 0, arena,
       &value);
   ext1 = (upb_test_ModelExtension1*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -197,39 +194,39 @@ TEST(GeneratedCode, Extensions) {
 
   // Test known GetExtension 2
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension2_model_ext_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension2_model_ext_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
   EXPECT_EQ(5, upb_test_ModelExtension2_i(ext2));
 
   // Test known GetExtension 3
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension2_model_ext_2_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension2_model_ext_2_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
   EXPECT_EQ(6, upb_test_ModelExtension2_i(ext2));
 
   // Test known GetExtension 4
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension2_model_ext_3_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension2_model_ext_3_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
   EXPECT_EQ(7, upb_test_ModelExtension2_i(ext2));
 
   // Test known GetExtension 5
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension2_model_ext_4_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension2_model_ext_4_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
   EXPECT_EQ(8, upb_test_ModelExtension2_i(ext2));
 
   // Test known GetExtension 6
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(msg), &upb_test_ModelExtension2_model_ext_5_ext, 0, arena,
+      UPB_UPCAST(msg), upb_test_ModelExtension2_model_ext_5_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -246,7 +243,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension1_model_ext_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension1_model_ext_ext, 0, arena,
       &value);
   ext1 = (upb_test_ModelExtension1*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -256,7 +253,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension2_model_ext_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension2_model_ext_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -265,7 +262,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension2_model_ext_2_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension2_model_ext_2_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -274,7 +271,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension2_model_ext_3_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension2_model_ext_3_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -283,7 +280,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension2_model_ext_4_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension2_model_ext_4_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);
@@ -292,7 +289,7 @@ TEST(GeneratedCode, Extensions) {
 
   // Test unknown GetExtension.
   promote_status = upb_Message_GetOrPromoteExtension(
-      UPB_UPCAST(base_msg), &upb_test_ModelExtension2_model_ext_5_ext, 0, arena,
+      UPB_UPCAST(base_msg), upb_test_ModelExtension2_model_ext_5_ext, 0, arena,
       &value);
   ext2 = (upb_test_ModelExtension2*)value.msg_val;
   EXPECT_EQ(kUpb_GetExtension_Ok, promote_status);

upb/mini_table/BUILD

@@ -148,6 +148,28 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "generated_registry_empty_test",
+    srcs = ["generated_registry_empty_test.cc"],
+    linkstatic = 1,
+    deps = [
+        ":generated_registry_empty_test_upb_minitable_proto",
+        ":mini_table",
+        "@googletest//:gtest",
+        "@googletest//:gtest_main",
+    ],
+)
+
+proto_library(
+    name = "generated_registry_empty_test_proto",
+    srcs = ["generated_registry_empty_test.proto"],
+)
+
+upb_minitable_proto_library(
+    name = "generated_registry_empty_test_upb_minitable_proto",
+    deps = [":generated_registry_empty_test_proto"],
+)
+
 proto_library(
     name = "message_benchmark_proto",
     testonly = 1,
@@ -232,3 +254,13 @@ filegroup(
     ),
     visibility = ["//upb:__pkg__"],
 )
+
+filegroup(
+    name = "test_protos",
+    srcs = glob(
+        [
+            "**/*test.proto",
+        ],
+    ),
+    visibility = ["//upb:__pkg__"],
+)

upb/mini_table/extension_registry.c

@@ -100,3 +100,7 @@ const upb_MiniTableExtension* upb_ExtensionRegistry_Lookup(
     return NULL;
   }
 }
+
+size_t upb_ExtensionRegistry_Size(const upb_ExtensionRegistry* r) {
+  return upb_strtable_count(&r->exts);
+}

upb/mini_table/extension_registry.h

@@ -84,6 +84,9 @@ upb_ExtensionRegistryStatus upb_ExtensionRegistry_AddArray(
 UPB_API const upb_MiniTableExtension* upb_ExtensionRegistry_Lookup(
     const upb_ExtensionRegistry* r, const upb_MiniTable* t, uint32_t num);
 
+// Returns the number of extensions in the registry. For testing/debugging only.
+UPB_API size_t upb_ExtensionRegistry_Size(const upb_ExtensionRegistry* r);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

upb/mini_table/generated_registry.c

@@ -42,26 +42,24 @@ static bool _upb_GeneratedRegistry_AddAllLinkedExtensions(
   const UPB_PRIVATE(upb_GeneratedExtensionListEntry)* entry =
       UPB_PRIVATE(upb_generated_extension_list);
   while (entry != NULL) {
-    const char* current = (const char*)entry->start;
-    const char* end = (const char*)entry->stop;
-    while ((size_t)(end - current) >= sizeof(upb_MiniTableExtension)) {
-      const upb_MiniTableExtension* ext =
-          (const upb_MiniTableExtension*)current;
+    const upb_MiniTableExtension** current = entry->start;
+    for (current = entry->start; current != entry->stop; ++current) {
+      const upb_MiniTableExtension* ext = *current;
       // Sentinels and padding introduced by the linker can result in zeroed
       // entries, so simply skip them.
-      if (upb_MiniTableExtension_Number(ext) == 0) {
+      if (*current == NULL) {
         // MSVC introduces padding that might not be sized exactly the same as
-        // upb_MiniTableExtension, so we can't iterate by sizeof.  This is a
-        // valid thing for any linker to do, so it's safer to just always do it.
-        current += UPB_ALIGN_OF(upb_MiniTableExtension);
+        // the linker array element, but it should be properly aligned, so just
+        // skipping empty elements should be safe.  (If the size and align of
+        // the array elements was different, we'd have to do something more
+        // complicated).
         continue;
       }
 
       if (upb_ExtensionRegistry_Add(r, ext) !=
           kUpb_ExtensionRegistryStatus_Ok) {
         return false;
       }
-      current += sizeof(upb_MiniTableExtension);
     }
     entry = entry->next;
   }

upb/mini_table/generated_registry_empty_test.cc

@@ -0,0 +1,45 @@
+
+#include <gtest/gtest.h>
+#include "upb/mini_table/extension_registry.h"
+#include "upb/mini_table/generated_registry.h"
+#include "upb/mini_table/generated_registry_empty_test.upb_minitable.h"
+#include "upb/mini_table/message.h"
+
+namespace {
+
+// Tests that we can instantiate the registry even if no extensions are linked.
+// This ensures that the sentinel object in the linker array properly ensures
+// that the encapsulation symbols are defined (eg. __start_linkarr_exts,
+// __stop_linkarr_exts) even when no extensions were linked.
+TEST(GeneratedRegistryEmptyTest, Instantiate) {
+  // Strongly reference the generated MiniTable to ensure that the TU for
+  // generated_registry_empty_test.upb_minitable.c is pulled in.
+  const upb_MiniTable* volatile table =
+      &upb_0test_0empty_0registry__EmptyRegistryTestMessage_msg_init;
+  (void)table;
+
+  // Test that the registry can be loaded, even if no extensions are linked.
+  // If we did not have a sentinel in the linker array, we would get a linker
+  // error here like:
+  //
+  // ld: error: undefined symbol: __start_linkarr_upb_AllExts
+  // >>> referenced by generated_registry_empty_test.upb_minitable.c
+  // >>>
+  // generated_registry_empty_test.upb_minitable.pic.o:(upb_GeneratedRegistry_Constructor_dont_copy_me__upb_internal_use_only.entry)
+  // >>> the encapsulation symbol needs to be retained under --gc-sections
+  // properly; consider -z nostart-stop-gc (see
+  // https://lld.llvm.org/ELF/start-stop-gc)
+  //
+  // ld: error: undefined symbol: __stop_linkarr_upb_AllExts
+  // >>> referenced by generated_registry_empty_test.upb_minitable.c
+  // >>>
+  // generated_registry_empty_test.upb_minitable.pic.o:(upb_GeneratedRegistry_Constructor_dont_copy_me__upb_internal_use_only.entry)
+  const upb_GeneratedRegistryRef* ref = upb_GeneratedRegistry_Load();
+  EXPECT_NE(ref, nullptr);
+  const upb_ExtensionRegistry* reg = upb_GeneratedRegistry_Get(ref);
+  EXPECT_NE(reg, nullptr);
+
+  upb_GeneratedRegistry_Release(ref);
+}
+
+}  // namespace

upb/mini_table/generated_registry_empty_test.proto

@@ -0,0 +1,15 @@
+edition = "2024";
+
+package upb_test_empty_registry;
+
+// A message that is in the same file as the unused extension.
+message EmptyRegistryTestMessage {
+  extensions 1 to max;
+}
+
+// An extension that we will *not* reference. The existence of this extension
+// will cause us to emit the code to register all extensions, but since this
+// extension is never referenced, that list of extensions will be empty.
+extend EmptyRegistryTestMessage {
+  int32 unused_extension = 1;
+}

upb/mini_table/generated_registry_test.cc

@@ -43,7 +43,7 @@ TEST_F(GeneratedRegistryTest, LocalExtensionExists) {
   // to be linked in.
 
   // Force linkage and sanity check extension numbers.
-  ASSERT_EQ(upb_MiniTableExtension_Number(&upb_test_2023_ext_ext), 100);
+  ASSERT_EQ(upb_MiniTableExtension_Number(upb_test_2023_ext_ext), 100);
 
   EXPECT_NE(upb_ExtensionRegistry_Lookup(
                 reg_, &upb__test_02023__EditionsMessage_msg_init, 100),
@@ -55,7 +55,7 @@ TEST_F(GeneratedRegistryTest, ForeignExtensionExists) {
   // linked in.
 
   // Force linkage and sanity check extension numbers.
-  ASSERT_EQ(upb_MiniTableExtension_Number(&upb_message_opt_ext), 7739036);
+  ASSERT_EQ(upb_MiniTableExtension_Number(upb_message_opt_ext), 7739036);
 
   EXPECT_NE(upb_ExtensionRegistry_Lookup(reg_, &google__protobuf__MessageOptions_msg_init,
                                          7739036),
@@ -77,8 +77,8 @@ TEST_F(GeneratedRegistryTest, MultipleFilesExtensionExists) {
   // in and registered.
 
   // Force linkage and sanity check extension numbers.
-  ASSERT_EQ(upb_MiniTableExtension_Number(&upb_multiple_files_ext1_ext), 100);
-  ASSERT_EQ(upb_MiniTableExtension_Number(&upb_multiple_files_ext2_ext), 100);
+  ASSERT_EQ(upb_MiniTableExtension_Number(upb_multiple_files_ext1_ext), 100);
+  ASSERT_EQ(upb_MiniTableExtension_Number(upb_multiple_files_ext2_ext), 100);
 
   EXPECT_NE(upb_ExtensionRegistry_Lookup(
                 reg_, &upb__MultipleFilesMessage1_msg_init, 100),