New issue
Advanced search Search tips

Issue 901776 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 7
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 894363



Sign in to add a comment

windows component builds failing on clang tot bots

Project Member Reported by h...@chromium.org, Nov 5

Issue description

Example builds:
https://ci.chromium.org/buildbot/chromium.clang/ToTWin%28dll%29/1842
https://ci.chromium.org/buildbot/chromium.clang/ToTWin64%28dll%29/1746
https://ci.chromium.org/buildbot/chromium.clang/CrWinAsan%28dll%29/2152

From the first one:

FAILED: win_clang_x64/protobuf_lite.dll win_clang_x64/protobuf_lite.dll.lib win_clang_x64/protobuf_lite.dll.pdb 
ninja -t msvc -e environment.x64 -- ../../third_party/llvm-build/Release+Asserts/bin/lld-link.exe /nologo /IMPLIB:win_clang_x64/protobuf_lite.dll.lib /DLL /OUT:win_clang_x64/protobuf_lite.dll /PDB:win_clang_x64/protobuf_lite.dll.pdb @win_clang_x64/protobuf_lite.dll.rsp
lld-link: error: undefined symbol: "static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteGroupToArray<class google::protobuf::MessageLite>(int, class google::protobuf::MessageLite const &, bool, unsigned char *)" (??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPEAEHAEBVMessageLite@23@_NPEAE@Z)
>>> referenced by win_clang_x64/obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj

lld-link: error: undefined symbol: "static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int, class google::protobuf::MessageLite const &, bool, unsigned char *)" (??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPEAEHAEBVMessageLite@23@_NPEAE@Z)
>>> referenced by win_clang_x64/obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj


I made a dllexport change recently, so it's not unlikely that this is my fault.
 
Status: Started (was: Assigned)
Repros nicly with the Linux -> Win cross compile:

$ cat out/release/args.gn 
llvm_force_head_revision=true clang_use_chrome_plugins=false clang_base_path="/work/llvm.combined/build.release"
target_os = "win"
is_component_build = true
is_debug = false
target_cpu = "x86"

$ ninja -C out/release protobuf_lite.dll
ninja: Entering directory `out/release'
[1/1] LINK(DLL) protobuf_lite.dll protobuf_lite.dll.lib protobuf_lite.dll.pdb
FAILED: protobuf_lite.dll protobuf_lite.dll.lib protobuf_lite.dll.pdb 
../../../../llvm.combined/build.release/bin/lld-link --rsp-quoting=posix /nologo -libpath:../../third_party/depot_tools/win_toolchain/vs_files/3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c/VC/Tools/MSVC/14.14.26428/lib/x86 -libpath:../../third_party/depot_tools/win_toolchain/vs_files/3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c/win_sdk/Lib/10.0.17134.0/um/x86 -libpath:../../third_party/depot_tools/win_toolchain/vs_files/3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c/win_sdk/Lib/10.0.17134.0/ucrt/x86 -libpath:../../third_party/depot_tools/win_toolchain/vs_files/3bc0ec615cf20ee342f3bc29bc991b5ad66d8d2c/VC/Tools/MSVC/14.14.26428/atlmfc/lib/x86 /IMPLIB:./protobuf_lite.dll.lib /DLL /OUT:./protobuf_lite.dll /PDB:./protobuf_lite.dll.pdb @./protobuf_lite.dll.rsp
lld-link: error: undefined symbol: "static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteGroupToArray<class google::protobuf::MessageLite>(int, class google::protobuf::MessageLite const &, bool, unsigned char *)" (??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z)
>>> referenced by obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj

lld-link: error: undefined symbol: "static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int, class google::protobuf::MessageLite const &, bool, unsigned char *)" (??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z)
>>> referenced by obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj
Bisection points to r344987:

commit 4f2789cf629116978b53d5a27a3d487aa5241656
Author: Richard Trieu <rtrieu@google.com>
Date:   Tue Oct 23 01:26:28 2018 +0000

    [CodeGen] Attach InlineHint to more functions
    
    For instantiated functions, search the template pattern to see if it marked
    inline to determine if InlineHint attribute should be added to the function.
> lld-link: error: undefined symbol: "static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteGroupToArray<class google::protobuf::MessageLite>(int, class google::protobuf::MessageLite const &, bool, unsigned char *)" (? $InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z)

It's defined here:
https://cs.chromium.org/chromium/src/third_party/protobuf/src/google/protobuf/wire_format_lite_inl.h?rcl=6d6f299693e29b659899f6190492bdf21f2e047f&l=887

WireFormatLite is marked dllexport here:
https://cs.chromium.org/chromium/src/third_party/protobuf/src/google/protobuf/wire_format_lite.h?rcl=6d6f299693e29b659899f6190492bdf21f2e047f&l=86


>>> referenced by obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj

That one includes wire_format_lite.h, but presumably not wire_format_lite_inl.h...


Interestingly, implicit_weak_message.obj doesn't seem to change at all between the two compiler versions, but maybe some other file does...
Before the Clang change:

$ find . -name "*.obj" -exec sha1sum {} \;
5f0d710231c16261fcfa07cd4949c68dc91d658e  ./protobuf_lite/coded_stream.obj
07cd2be0efe775070cb965f5a8e0f63a5ca6b1fa  ./protobuf_lite/int128.obj
ce725f9e22e2bde2317a04d666da68bc0e226e35  ./protobuf_lite/zero_copy_stream.obj
e2b47e2183281a22d7a5626d3905a4ab5ced56fb  ./protobuf_lite/stringpiece.obj
589a16f5bf3bc031b3ba45e2fe6f1db81080b359  ./protobuf_lite/strutil.obj
8f84b6a16c013d84a6b5c5bb38ee40e93bc95c7a  ./protobuf_lite/message_lite.obj
c2ff3bf32f4d8612bcbaf727e24f48a9a62deebd  ./protobuf_lite/wire_format_lite.obj
d040fe93ef32b21d58fad53a6bb40ed08d227d55  ./protobuf_lite/extension_set.obj
808750ba657255c818763833522d75087a4f1bdf  ./protobuf_lite/status.obj
dec8106b991911be024d3d2f41f56202674d08f1  ./protobuf_lite/generated_message_util.obj
11c3271dbe3dfb0db3cd24b76ab8b391a14ae787  ./protobuf_lite/zero_copy_stream_impl_lite.obj
a8655c4ca020f997fa437e1120be184c47dac012  ./protobuf_lite/arena.obj
6ab7b09e6aaf21332ba17a4258078fc570f3d4f6  ./protobuf_lite/stringprintf.obj
1cc59005ee396fb67e509d2dcd7e27d2f79f0e56  ./protobuf_lite/implicit_weak_message.obj
a826a22943e624426819e7ccd2e053e6912d968c  ./protobuf_lite/once.obj
547c256c6949e133a2d970236e6568d8d6b02bfa  ./protobuf_lite/repeated_field.obj
201391a0185ff95c047d7735268a7b4d27ca3d91  ./protobuf_lite/generated_message_table_driven_lite.obj
ed7623a7202f5ac46fda4f9b1a5b4762d6ab7806  ./protobuf_lite/io_win32.obj
454c6bb29c0ad4346d28244cd89f8aae9b2ff7ff  ./protobuf_lite/arenastring.obj
d02e1864c8244efebe85b301df836a8d9469ac29  ./protobuf_lite/statusor.obj
86811f3f1820e24acd7e5d698411547b70ecd672  ./protobuf_lite/time.obj
563046b8aeff0abceec977ece6e50f83a81199b1  ./protobuf_lite/structurally_valid.obj
4fa60748edb65c47d9ffd4f15f0ce3be914b934a  ./protobuf_lite/bytestream.obj
d0ea05199746bd1d96d8f50a86d33b43f22eebfb  ./protobuf_lite/common.obj

After:

$ find . -name "*.obj" -exec sha1sum {} \;
5f0d710231c16261fcfa07cd4949c68dc91d658e  ./protobuf_lite/coded_stream.obj
07cd2be0efe775070cb965f5a8e0f63a5ca6b1fa  ./protobuf_lite/int128.obj
ce725f9e22e2bde2317a04d666da68bc0e226e35  ./protobuf_lite/zero_copy_stream.obj
e2b47e2183281a22d7a5626d3905a4ab5ced56fb  ./protobuf_lite/stringpiece.obj
589a16f5bf3bc031b3ba45e2fe6f1db81080b359  ./protobuf_lite/strutil.obj
af1ea8bd7816759197831b3f002e5bff542dea92  ./protobuf_lite/message_lite.obj       <--- DIFF
0b358ee9da78d41fa0545c48394e46630de78931  ./protobuf_lite/wire_format_lite.obj   <--- DIFF
cb78ea78ee70bf86abaa185c4cd320e126ceda9c  ./protobuf_lite/extension_set.obj      <--- DIFF
808750ba657255c818763833522d75087a4f1bdf  ./protobuf_lite/status.obj
b74a3f986d0acc47bd8b311e6c183c857489a62c  ./protobuf_lite/generated_message_util.obj  <--- DIFF
11c3271dbe3dfb0db3cd24b76ab8b391a14ae787  ./protobuf_lite/zero_copy_stream_impl_lite.obj
a8655c4ca020f997fa437e1120be184c47dac012  ./protobuf_lite/arena.obj
6ab7b09e6aaf21332ba17a4258078fc570f3d4f6  ./protobuf_lite/stringprintf.obj
1cc59005ee396fb67e509d2dcd7e27d2f79f0e56  ./protobuf_lite/implicit_weak_message.obj
a826a22943e624426819e7ccd2e053e6912d968c  ./protobuf_lite/once.obj
547c256c6949e133a2d970236e6568d8d6b02bfa  ./protobuf_lite/repeated_field.obj
360e71ca5b4a50b3132ee56a3747c5e11d641718  ./protobuf_lite/generated_message_table_driven_lite.obj  <--- DIFF
ed7623a7202f5ac46fda4f9b1a5b4762d6ab7806  ./protobuf_lite/io_win32.obj
454c6bb29c0ad4346d28244cd89f8aae9b2ff7ff  ./protobuf_lite/arenastring.obj
d02e1864c8244efebe85b301df836a8d9469ac29  ./protobuf_lite/statusor.obj
86811f3f1820e24acd7e5d698411547b70ecd672  ./protobuf_lite/time.obj
563046b8aeff0abceec977ece6e50f83a81199b1  ./protobuf_lite/structurally_valid.obj
4fa60748edb65c47d9ffd4f15f0ce3be914b934a  ./protobuf_lite/bytestream.obj
d0ea05199746bd1d96d8f50a86d33b43f22eebfb  ./protobuf_lite/common.obj
Which of these provided the definition of InternalWriteGroupToArray before?

$ for i in `ls *.obj` ; do ( objdump -t $i | grep '?$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z' ) && echo $i ; done
[571](sec 191)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
extension_set.obj
[422](sec 142)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
generated_message_table_driven_lite.obj
[527](sec 177)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
generated_message_util.obj
[1244](sec  0)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
implicit_weak_message.obj
[527](sec 177)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
message_lite.obj
[422](sec 142)(fl 0x00)(ty  20)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
wire_format_lite.obj



And after?

$ for i in `ls *.obj` ; do ( objdump -t $i | grep '?$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z' ) && echo $i ; done
[1244](sec  0)(fl 0x00)(ty   0)(scl   2) (nx 0) 0x00000000 ??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z
implicit_weak_message.obj

(sec 0) means that's not a definition, it's an undefined reference.
Okay, what about wire_format_lite.obj, why isn't the definition emitted there anymore?


Before the compiler change:

define weak_odr dso_local dllexport i8* @"?WriteGroupToArray@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@34@PAE@Z"(i32 %field_number, %"class.google::protobuf::MessageLite"* dereferenceable(4) %value, i8* %target) local_unnamed_addr #3 comdat align 2 !dbg !8537 {
entry:
  call void @llvm.dbg.value(metadata i8* %target, metadata !8539, metadata !DIExpression()), !dbg !8542
  call void @llvm.dbg.value(metadata %"class.google::protobuf::MessageLite"* %value, metadata !8540, metadata !DIExpression()), !dbg !8542
  call void @llvm.dbg.value(metadata i32 %field_number, metadata !8541, metadata !DIExpression()), !dbg !8543
  %call = tail call i8* @"??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z"(i32 %field_number, %"class.google::protobuf::MessageLite"* nonnull dereferenceable(4) %value, i1 zeroext false, i8* %target) #16, !dbg !8544
  ret i8* %call, !dbg !8544
}

define linkonce_odr dso_local i8* @"??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z"(i32 %field_number, %"class.google::protobuf::MessageLite"* dereferenceable(4) %value, i1 zeroext %deterministic, i8* %target) local_unnamed_addr #3 comdat align 2 !dbg !8545 {
entry:
...
}


After the change, the call from WriteGroupToArray was inlined, and the definition of InternalWriteGroupToArray no longer emitted.

I guess we were just getting lucky before that it wasn't inlined.


The bug is that InternalWriteGroupToArray is marked inline, but its definition is not available when referenced in implicit_weak_message.obj. (The reference might just be through pulling in the definition of a dllexport function.)

What does that reference look in implicit_weak_message.obj look like?

; Function Attrs: nounwind optsize sspstrong
define weak_odr dso_local dllexport i8* @"?WriteGroupToArray@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@34@PAE@Z"(i32 %field_number, %"class.google::protobuf::MessageLite"* dereferenceable(4) %value, i8* %target) local_unnamed_addr #3 comdat align 2 !dbg !7582 {
entry:
  call void @llvm.dbg.value(metadata i8* %target, metadata !7584, metadata !DIExpression()), !dbg !7587
  call void @llvm.dbg.value(metadata %"class.google::protobuf::MessageLite"* %value, metadata !7585, metadata !DIExpression()), !dbg !7587
  call void @llvm.dbg.value(metadata i32 %field_number, metadata !7586, metadata !DIExpression()), !dbg !7588
  %call = tail call i8* @"??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z"(i32 %field_number, %"class.google::protobuf::MessageLite"* nonnull dereferenceable(4) %value, i1 zeroext false, i8* %target) #14, !dbg !7589
  ret i8* %call, !dbg !7589
}

; Function Attrs: optsize
declare dso_local i8* @"??$InternalWriteGroupToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z"(i32, %"class.google::protobuf::MessageLite"* dereferenceable(4), i1 zeroext, i8*) local_unnamed_addr #4
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4c84c1b7d66c3b305d0362fad2c34cf986e4606

commit a4c84c1b7d66c3b305d0362fad2c34cf986e4606
Author: Hans Wennborg <hans@chromium.org>
Date: Tue Nov 06 10:00:49 2018

Add wire_format_lite_inl.h include to implicit_weak_message.cc

implicit_weak_message.cc is part of protobuf_lite.dll, and it includes
wire_format_lite.h, which includes the dllexport inline function
WireFormatLite::WriteGroupToArray which will therefore be emitted.

WriteGroupToArray in turn calls the inline function
InternalWriteGroupToArray, however that definition is provided in the
_inl file. To make sure the definition is available, the _inl file must
be included.

Before Clang r344987 the build worked anyway (due to luck), because
InternalWriteGroupToArray was emitted into other object files (e.g. in
wire_format_lite.obj). After that Clang revision, those definitions
started getting inlined, and so are longer available and cause a link
failure for the reference from implicit_weak_message.obj.

Bug:  901776 
Change-Id: I60ba265495eba6e868f0799f697c0d172e4c2fb1
Reviewed-on: https://chromium-review.googlesource.com/c/1317910
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605641}
[modify] https://crrev.com/a4c84c1b7d66c3b305d0362fad2c34cf986e4606/third_party/protobuf/README.chromium
[add] https://crrev.com/a4c84c1b7d66c3b305d0362fad2c34cf986e4606/third_party/protobuf/patches/0024-add_wire_format_lite_inl_include.patch
[modify] https://crrev.com/a4c84c1b7d66c3b305d0362fad2c34cf986e4606/third_party/protobuf/src/google/protobuf/implicit_weak_message.cc

Are we sure that clang's behavior before r344987 is consistent with MSVC's?
Status: Fixed (was: Started)
> Are we sure that clang's behavior before r344987 is consistent with MSVC's?

Yes, I believe so.

Building the same way as above, but with is_clang=false, here's the reference in implicit_weak_message.obj:

>dumpbin /symbols obj/third_party/protobuf/protobuf_lite/implicit_weak_message.obj | grep InternalWriteMessageToArray@VMessageLite
4D8 00000000 UNDEF  notype ()    External     | ??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z (public: static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int,class google::protobuf::MessageLite const &,bool,unsigned char *))


It's UNDEF, so this would be a link error unless it finds the symbol elsewhere. As in #5, the symbol can in be found in some of the other object files, e.g. in extension_set.obj:

>dumpbin /symbols obj/third_party/protobuf/protobuf_lite/extension_set.obj | grep InternalWriteMessageToArray@VMessageLite
CB6 00000000 SECT8A notype ()    External     | ??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z (public: static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int,class google::protobuf::MessageLite const &,bool,unsigned char *))

Here's what the section header for that looks for it looks like:

SECTION HEADER #8A
.text$mn name
       0 physical address
       0 virtual address
      39 size of raw data
   38791 file pointer to raw data (00038791 to 000387C9)
   387CA file pointer to relocation table
       0 file pointer to line numbers
       2 number of relocations
       0 number of line numbers
60101020 flags
         Code
         COMDAT; sym= "public: static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int,class google::protobuf::MessageLite const &,bool,unsigned char *)" (??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z)
         1 byte align
         Execute Read

I don't see anything in the file that would prevent MSVC from inlining the call to this function and dropping the definition.



When compiled with Clang before the inline hint change, the header looks the same:

SECTION HEADER #C1
   .text name
       0 physical address
       0 virtual address
      76 size of raw data
    DE6A file pointer to raw data (0000DE6A to 0000DEDF)
       0 file pointer to relocation table
       0 file pointer to line numbers
       0 number of relocations
       0 number of line numbers
60201020 flags
         Code
         COMDAT; sym= "public: static unsigned char * __cdecl google::protobuf::internal::WireFormatLite::InternalWriteMessageToArray<class google::protobuf::MessageLite>(int,class google::protobuf::MessageLite const &,bool,unsigned char *)" (??$InternalWriteMessageToArray@VMessageLite@protobuf@google@@@WireFormatLite@internal@protobuf@google@@SAPAEHABVMessageLite@23@_NPAE@Z)
         2 byte align
         Execute Read
Oh, right, I understand now. The effect of adding inlinehint was to change inliner behavior, which generated the import of the unexported symbol. Makes sense.

Sign in to add a comment