Bug 183072 - Clang reports undefined reference errors of poison variables while building LLIntOffsetsExtractor with -gsplit-dwarf
Summary: Clang reports undefined reference errors of poison variables while building L...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on:
Blocks: 145121
  Show dependency treegraph
 
Reported: 2018-02-22 20:17 PST by Fujii Hironori
Modified: 2018-02-25 18:41 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.36 KB, patch)
2018-02-22 22:48 PST, Fujii Hironori
mark.lam: review-
Hironori.Fujii: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2018-02-22 20:17:11 PST
CC=clang-5.0 CXX=clang++-5.0 ./Tools/Scripts/build-webkit --gtk --debug

> [1/2400] Linking CXX executable bin/LLIntOffsetsExtractor
> FAILED: bin/LLIntOffsetsExtractor 
> : && /usr/bin/clang++-5.0  -fdiagnostics-color=always -fcolor-diagnostics -Wno-parentheses-equality -Wno-noexcept-type -Qunused-arguments -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wextra -Wall  -fno-strict-aliasing -fno-exceptions -std=c++14 -fno-rtti -gsplit-dwarf -g  -L/home/fujii/work/webkit/ga/WebKitBuild/DependenciesGTK/Root/lib -fuse-ld=gold -Wl,--disable-new-dtags -Wl,--gdb-index   -rdynamic Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o  -o bin/LLIntOffsetsExtractor  lib/libWTFGTK.a lib/libbmalloc.a -ldl -licudata -licui18n -licuuc -lpthread -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lz && :
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x8): error: undefined reference to 'g_NativeCodePoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x10): error: undefined reference to 'g_JITCodePoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x18): error: undefined reference to 'g_UnlinkedSourceCodePoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x20): error: undefined reference to 'g_JSFunctionPoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x28): error: undefined reference to 'g_GlobalDataPoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x30): error: undefined reference to 'g_JSGlobalObjectPoison'
> Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o(.debug_addr+0x38): error: undefined reference to 'g_CodeBlockPoison'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
Comment 1 Fujii Hironori 2018-02-22 22:32:14 PST
I created a reduced LLIntOffsetsExtractor.cpp:
https://gist.github.com/fujii/9940945fc788aba2577e41eb06ec7271

This is caused by -gsplit-dwarf.

> fujii@ubuntu $ clang++-5.0 -std=c++14 -gsplit-dwarf  LLIntOffsetsExtractor.cpp
> /tmp/LLIntOffsetsExtractor-15eec5.o:(.debug_addr+0x0): undefined reference to `g_JITCodePoison'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> fujii@ubuntu $ clang++-5.0 -std=c++14  LLIntOffsetsExtractor.cpp
> fujii@ubuntu $

This is a document about .debug_addr.
https://gcc.gnu.org/wiki/DebugFission
Comment 2 Fujii Hironori 2018-02-22 22:36:30 PST
GTK port release builds has no problem because it doesn't use -gsplit-dwarf.

> [1/1] : && /usr/bin/clang++-5.0  -fdiagnostics-color=always -fcolor-diagnostics -Wno-parentheses-equality -Wno-noexcept-type -Qunused-arguments -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -Wextra -Wall  -fno-strict-aliasing -fno-exceptions -std=c++14 -fno-rtti -O3 -DNDEBUG  -L/home/fujii/work/webkit/ga/WebKitBuild/DependenciesGTK/Root/lib -fuse-ld=gold -Wl,--disable-new-dtags   -rdynamic Source/JavaScriptCore/CMakeFiles/LLIntOffsetsExtractor.dir/llint/LLIntOffsetsExtractor.cpp.o  -o bin/LLIntOffsetsExtractor  lib/libWTFGTK.a lib/libbmalloc.a -ldl -licudata -licui18n -licuuc -lpthread -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lz && :
Comment 3 Fujii Hironori 2018-02-22 22:38:47 PST
GCC doesn't seem to have this issue.

$ g++-7 -std=c++14 -gsplit-dwarf  LLIntOffsetsExtractor.cpp
Comment 4 Fujii Hironori 2018-02-22 22:48:58 PST
Created attachment 334505 [details]
Patch
Comment 5 Build Bot 2018-02-22 22:51:48 PST
Attachment 334505 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:68:  POISON_KEY_NAME is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Mark Lam 2018-02-22 23:08:26 PST
Comment on attachment 334505 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334505&action=review

> Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:69
> +#define DEFINE_POISON(poisonID) \
> +    uintptr_t POISON_KEY_NAME(poisonID);
> +FOR_EACH_JSC_POISON(DEFINE_POISON)

Why is this necessary at all?  We don't ever use these as offsets.  So, I find it suspicious that we need them here.

Secondly, the LLInt asm code only refers to them as global addresses that should come from the JavaScriptCore library.  Fabricating a second set of such variables here in LLIntOffsetsExtractor will only at best stop the compiler from complaining, but any addresses or offsets generated from these will have nothing to do with the versions in the JavaScriptCore library.  As such, you'll definitely be getting wrong values.  Hence, I'm not sure that this is not the right solution.  r- for now.

Can you explain what -gsplit-dwarf is and what does it have to do with triggering this issue?