1991378 - Crash in [@ google::protobuf::EncodedDescriptorDatabase::Add]

Status: RESOLVED DUPLICATE of bug 1816848

(Firefox :: General, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/7896d409-0aed-4237-8ea7-3a2eb0250927

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  xul.dll  std::unique_ptr<google::protobuf::EncodedDescriptorDatabase::DescriptorIndex,...  /builds/worker/fetches/vs/VC/Tools/MSVC/14.39.33519/include/memory:3266
0  xul.dll  google::protobuf::EncodedDescriptorDatabase::Add(void const*, int)  toolkit/components/protobuf/src/google/protobuf/descriptor_database.cc:558
0  xul.dll  google::protobuf::DescriptorPool::InternalAddGeneratedFile(void const*, int)  toolkit/components/protobuf/src/google/protobuf/descriptor.cc:1986
0  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptorsImpl(google::protobuf:...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3049
0  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptors(google::protobuf::int...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3060
1  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptorsImpl(google::protobuf:...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3045
1  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptors(google::protobuf::int...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3060
2  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptorsImpl(google::protobuf:...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3045
2  xul.dll  google::protobuf::(anonymous namespace)::AddDescriptors(google::protobuf::int...  toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc:3060
3  ucrtbase.dll  initterm

This is a brand new crash but I don't really understand how we get from loading a library in the startup path to ending up with a NULL pointer access in protobuf. The stack traces are completed and consistent so this is definitely happening. Greg, Yannis, Bob, since you have more Windows experience than ma can one of you have a look and figure out what's going on? Unfortunately the oldest crashes we have started on beta and not nightly, so it's hard to pin down the change that might have caused this.

Comment 1

[Yannis Juglaret [:yannis]]

This is code that runs during the loading of xul.dll. We internally call _initterm with an array of function pointers to call:

0:000> .ecxr
0:000> k
  *** Stack trace for last set context - .thread/.cxr resets it
 # Child-SP          RetAddr               Call Site
[...]
08 000000d4`5cffd700 00007ffe`39bc526d     xul!google::protobuf::`anonymous namespace'::AddDescriptors+0x66 [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3060] 
09 000000d4`5cffd960 00000001`825c7486     ucrtbase!initterm+0x2d
0a 000000d4`5cffd990 00000001`825c7654     xul!dllmain_crt_process_attach+0x9a [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\dll_dllmain.cpp @ 66] 
[...]

Based on the stack above and the code below, we are running the function pointers found from 1888e9858 to 1888e94c0 (excluded):

0:000> u xul!dllmain_crt_process_attach L30
xul!dllmain_crt_process_attach [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\dll_dllmain.cpp @ 34]:
[...]
00000001`825c7473 488d15de233206  lea     rdx,[xul!__xc_z (00000001`888e9858)]
00000001`825c747a 488d0d3f203206  lea     rcx,[xul!__xc_a (00000001`888e94c0)]
00000001`825c7481 e87abfce04      call    xul!initterm (00000001`872b3400)
00000001`825c7486

Within these 115 function pointers, I see five obvious suspicious ones that directly forward execution to xul!google::protobuf::`anonymous namespace'::AddDescriptors and could result in the crashes we are seeing (the 110 others don't seem related to protobuf):

0:000> ? (1888e9858-1888e94c0)/8
Evaluate expression: 115 = 00000000`00000073
0:000> dps 1888e94c0 L73
[...]
00000001`888e94d0  00000001`80006300 xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service.pb.cc @ 0]
00000001`888e94d8  00000001`800112c0 xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fcommon_2fv1_2fcommon_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.pb.cc @ 0]
00000001`888e94e0  00000001`800112d0 xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fresource_2fv1_2fresource_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.pb.cc @ 0]
00000001`888e94e8  00000001`800112e0 xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2ftrace_2fv1_2ftrace_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.pb.cc @ 0]
00000001`888e94f0  00000001`800112f0 xul!`dynamic initializer for 'dynamic_init_dummy_google_2fprotobuf_2fdescriptor_2eproto' [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/descriptor.pb.cc @ 0]
[...]

0:000> u 00000001`80006300
xul!google::protobuf::internal::AddDescriptorsRunner::AddDescriptorsRunner [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3095] [inlined in xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/collector/trace/v1/trace_service.pb.cc @ 0]]:
00000001`80006300 488d0de13c9a08  lea     rcx,[xul!descriptor_table_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto (00000001`889a9fe8)]
00000001`80006307 e904000000      jmp     xul!google::protobuf::`anonymous namespace'::AddDescriptors (00000001`80006310)

0:000> u 00000001`800112c0
xul!google::protobuf::internal::AddDescriptorsRunner::AddDescriptorsRunner [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3095] [inlined in xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fcommon_2fv1_2fcommon_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/common/v1/common.pb.cc @ 0]]:
00000001`800112c0 488d0df9939908  lea     rcx,[xul!descriptor_table_opentelemetry_2fproto_2fcommon_2fv1_2fcommon_2eproto (00000001`889aa6c0)]
00000001`800112c7 e94450ffff      jmp     xul!google::protobuf::`anonymous namespace'::AddDescriptors (00000001`80006310)

0:000> u 00000001`800112d0
xul!google::protobuf::internal::AddDescriptorsRunner::AddDescriptorsRunner [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3095] [inlined in xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2fresource_2fv1_2fresource_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/resource/v1/resource.pb.cc @ 0]]:
00000001`800112d0 488d0d49949908  lea     rcx,[xul!descriptor_table_opentelemetry_2fproto_2fresource_2fv1_2fresource_2eproto (00000001`889aa720)]
00000001`800112d7 e93450ffff      jmp     xul!google::protobuf::`anonymous namespace'::AddDescriptors (00000001`80006310)

0:000> u 00000001`800112e0
xul!google::protobuf::internal::AddDescriptorsRunner::AddDescriptorsRunner [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3095] [inlined in xul!`dynamic initializer for 'dynamic_init_dummy_opentelemetry_2fproto_2ftrace_2fv1_2ftrace_2eproto' [/builds/worker/checkouts/gecko/third_party/opentelemetry-cpp/third_party/opentelemetry-proto/opentelemetry/proto/trace/v1/trace.pb.cc @ 0]]:
00000001`800112e0 488d0d79939908  lea     rcx,[xul!descriptor_table_opentelemetry_2fproto_2ftrace_2fv1_2ftrace_2eproto (00000001`889aa660)]
00000001`800112e7 e92450ffff      jmp     xul!google::protobuf::`anonymous namespace'::AddDescriptors (00000001`80006310)

0:000> u 00000001`800112f0
xul!google::protobuf::internal::AddDescriptorsRunner::AddDescriptorsRunner [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/generated_message_reflection.cc @ 3095] [inlined in xul!`dynamic initializer for 'dynamic_init_dummy_google_2fprotobuf_2fdescriptor_2eproto' [/builds/worker/checkouts/gecko/toolkit/components/protobuf/src/google/protobuf/descriptor.pb.cc @ 0]]:
00000001`800112f0 488d0d31c79908  lea     rcx,[xul!descriptor_table_google_2fprotobuf_2fdescriptor_2eproto (00000001`889ada28)]
00000001`800112f7 e91450ffff      jmp     xul!google::protobuf::`anonymous namespace'::AddDescriptors (00000001`80006310)

Based on the code above, these five functions pointers are the initialization code for:

• descriptor_table_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto;
• descriptor_table_opentelemetry_2fproto_2fcommon_2fv1_2fcommon_2eproto;
• descriptor_table_opentelemetry_2fproto_2fresource_2fv1_2fresource_2eproto;
• descriptor_table_opentelemetry_2fproto_2ftrace_2fv1_2ftrace_2eproto;
• descriptor_table_google_2fprotobuf_2fdescriptor_2eproto.

The first four were added this month by bug 1930954, while the last one has been present since 2022. Marking bug 1930954 as a regressor accordingly.

These initializers are run through the constructor of static AddDescriptorsRunner objects, with code that looks like this:

// Force running AddDescriptors() at dynamic initialization time.
PROTOBUF_ATTRIBUTE_INIT_PRIORITY2 static ::_pbi::AddDescriptorsRunner dynamic_init_dummy_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto(&descriptor_table_opentelemetry_2fproto_2fcollector_2ftrace_2fv1_2ftrace_5fservice_2eproto);

Comment 2

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1930954

Comment 3

[Michael van Straten [:michael]]

It looks like the EncodedDescriptorDatabase object has not been initialized when Add is called.

Comment 4

[Michael van Straten [:michael]]

A quick fix would be to switch the proto files to use the LITE_RUNTIME everyone else is using. This does not use the Descriptors at all.

Comment 5

[Yannis Juglaret [:yannis]]

(In reply to Michael van Straten [:michael] from comment #3)

It looks like the EncodedDescriptorDatabase object has not been initialized when Add is called.

I agree, the actual crash site seems to be caused by google::protobuf::GeneratedDatabase() returning a null pointer. In theory this isn't possible, and looking at the code paths I'm fairly confident that the only explanation is the known issue described in bug 1816848 comment 21, i.e. xul.dll getting corrupted during update in a way that breaks static local initialization. We can see there that the volume for mozilla::LinkedListElement<T>::setPreviousUnsafe is lower in 144 beta than in 143 beta, so maybe the current signature has taken precedence for this known issue.

Comment 6

[Gabriele Svelto [:gsvelto]]

Note that in all these crashes the xul.dll entry in the module list doesn't have a valid signature. That points towards the library being corrupted.

Comment 7

[Yannis Juglaret [:yannis]]

Yes, and we can also see the absence of ASLR in the load address for xul.dll like in bug 1816848. All seems to point to a new signature for this known issue, marking as a duplicate accordingly. I don't think you need to fix anything on your side, :michael. Thanks for the quick reaction though!

Comment 8

[Michael van Straten [:michael]]

Thank you for looking into it, :yannis. I might switch to LITE_RUNTIME anyway since it removes some static ctors.

Comment 9

[Yannis Juglaret [:yannis]]

FWIW, I was indeed able to reproduce this crash by downloading Firefox Beta and manually corrupting xul.dll such that anything past the first 155MB is a zero byte (which is a similar corruption to the one explained in bug 1816848 comment 21 ; here 119MB didn't work because the corruption was detected at startup, so I looked where .tls and .reloc are located in xul.dll and chose 155MB instead). This confirms that marking as a duplicate was appropriate.