Skip to content

Commit c0838a8

Browse files
aoikonomopouloswebkit-commit-queue
aoikonomopoulos
authored and
webkit-commit-queue
committed
Enable offlineasm debug annotations for GCC
https://bugs.webkit.org/show_bug.cgi?id=207119 Patch by Angelos Oikonomopoulos <angelos@igalia.com> on 2020-04-02 Reviewed by Darin Adler. This simply reuses the existing code that generates debug annotations, adding two workarounds for limitations in GCC and GDB. First, the .file directives that offlineasm inserts in inline asm use file slots that conflict with those in the compilation unit that includes LLIntAssembly.h (specifically, LowLevelInterpreter.cpp). Clang's built-in assembler will transparently fix that for us, but for GCC we need to post-process the generated assembler. Unfortunately, cmake doesn't allow us to introduce a compiler wrapper for a single source file, so we need to create a separate target for it. This wrapping only happens when building with GCC and the user has explicitly requested debug information, either by selecting a Debug/RelWithDebInfo build or setting GCC_OFFLINEASM_SOURCE_MAP. Second, GDB will only look at the line table for a compilation unit if it can first resolve the address to one of the known symbols in the file. Introduce marker symbols to work around this bug. * CMakeLists.txt: Canonical link: https://commits.webkit.org/222820@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@259390 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent f88a218 commit c0838a8

9 files changed

Lines changed: 691 additions & 6 deletions

File tree

‎Source/JavaScriptCore/CMakeLists.txt‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ add_custom_command(
323323
OUTPUT ${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput}
324324
MAIN_DEPENDENCY ${JAVASCRIPTCORE_DIR}/offlineasm/asm.rb
325325
DEPENDS LLIntOffsetsExtractor ${LLINT_ASM} ${OFFLINE_ASM} ${JavaScriptCore_DERIVED_SOURCES_DIR}/InitBytecodes.asm ${JavaScriptCore_DERIVED_SOURCES_DIR}/InitWasm.asm
326-
COMMAND ${RUBY_EXECUTABLE} ${JAVASCRIPTCORE_DIR}/offlineasm/asm.rb -I${JavaScriptCore_DERIVED_SOURCES_DIR}/ ${JAVASCRIPTCORE_DIR}/llint/LowLevelInterpreter.asm $<TARGET_FILE:LLIntOffsetsExtractor> ${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput} ${OFFLINE_ASM_ARGS}
326+
COMMAND ${CMAKE_COMMAND} -E env CMAKE_CXX_COMPILER_ID=${CMAKE_CXX_COMPILER_ID} GCC_OFFLINEASM_SOURCE_MAP=${GCC_OFFLINEASM_SOURCE_MAP} ${RUBY_EXECUTABLE} ${JAVASCRIPTCORE_DIR}/offlineasm/asm.rb -I${JavaScriptCore_DERIVED_SOURCES_DIR}/ ${JAVASCRIPTCORE_DIR}/llint/LowLevelInterpreter.asm $<TARGET_FILE:LLIntOffsetsExtractor> ${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput} ${OFFLINE_ASM_ARGS}
327327
COMMAND ${CMAKE_COMMAND} -E touch_nocreate ${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput}
328328
WORKING_DIRECTORY ${JavaScriptCore_DERIVED_SOURCES_DIR}
329329
VERBATIM)
@@ -348,10 +348,17 @@ if (MSVC AND NOT ENABLE_C_LOOP)
348348
COMMAND ${MASM_EXECUTABLE} ${LLINT_MASM_FLAGS} ${JavaScriptCore_DERIVED_SOURCES_DIR}/LowLevelInterpreterWin.obj ${JavaScriptCore_DERIVED_SOURCES_DIR}/LowLevelInterpreterWin.asm
349349
VERBATIM)
350350
list(APPEND JavaScriptCore_SOURCES ${JavaScriptCore_DERIVED_SOURCES_DIR}/LowLevelInterpreterWin.obj)
351+
add_library(LowLevelInterpreterLib OBJECT llint/LowLevelInterpreter.cpp)
351352
else ()
352-
list(APPEND JavaScriptCore_HEADERS
353-
${JavaScriptCore_DERIVED_SOURCES_DIR}/LLIntAssembly.h
354-
)
353+
# As there's poor toolchain support for using `.file` directives in
354+
# inline asm (i.e. there's no way to avoid clashes with the `.file`
355+
# directives generated by the C code in the compilation unit), we
356+
# introduce a postprocessing pass for the asm that gets assembled into
357+
# an object file. We only need to do this for LowLevelInterpreter.cpp
358+
# and cmake doesn't allow us to introduce a compiler wrapper for a
359+
# single source file, so we need to create a separate target for it.
360+
add_library(LowLevelInterpreterLib OBJECT llint/LowLevelInterpreter.cpp
361+
${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput})
355362
endif ()
356363

357364
# WebAssembly generator
@@ -1362,6 +1369,29 @@ target_include_directories(LLIntOffsetsExtractor SYSTEM PRIVATE ${JavaScriptCore
13621369

13631370
add_subdirectory(shell)
13641371

1372+
target_include_directories(LowLevelInterpreterLib
1373+
PRIVATE "$<TARGET_PROPERTY:JavaScriptCore,INCLUDE_DIRECTORIES>")
1374+
1375+
if (CMAKE_COMPILER_IS_GNUCXX AND GCC_OFFLINEASM_SOURCE_MAP)
1376+
message(STATUS "Enabling asm postprocessing")
1377+
1378+
set(LowLevelInterpreter_LAUNCHER "${JavaScriptCore_SCRIPTS_SOURCES_DIR}/postprocess-asm")
1379+
get_target_property(PROP_RULE_LAUNCH_COMPILE LowLevelInterpreterLib RULE_LAUNCH_COMPILE)
1380+
if (PROP_RULE_LAUNCH_COMPILE)
1381+
set(LowLevelInterpreter_LAUNCHER "${LowLevelInterpreter_LAUNCHER} ${PROP_RULE_LAUNCH_COMPILE}")
1382+
endif ()
1383+
set_property(TARGET LowLevelInterpreterLib
1384+
PROPERTY RULE_LAUNCH_COMPILE "${LowLevelInterpreter_LAUNCHER}")
1385+
1386+
# Pass in the filename as a magic preprocessor directive, so that
1387+
# the wrapper can accurately identify the source file.
1388+
set_source_files_properties("llint/LowLevelInterpreter.cpp"
1389+
PROPERTIES
1390+
COMPILE_DEFINITIONS "POSTPROCESS_ASM=llint/LowLevelInterpreter.cpp")
1391+
endif ()
1392+
1393+
list(APPEND JavaScriptCore_LIBRARIES LowLevelInterpreterLib)
1394+
13651395
WEBKIT_COMPUTE_SOURCES(JavaScriptCore)
13661396
WEBKIT_WRAP_SOURCELIST(${JavaScriptCore_SOURCES})
13671397
WEBKIT_FRAMEWORK(JavaScriptCore)

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
2020-04-02 Angelos Oikonomopoulos <angelos@igalia.com>
2+
3+
Enable offlineasm debug annotations for GCC
4+
https://bugs.webkit.org/show_bug.cgi?id=207119
5+
6+
Reviewed by Darin Adler.
7+
8+
This simply reuses the existing code that generates debug
9+
annotations, adding two workarounds for limitations in GCC and
10+
GDB.
11+
12+
First, the .file directives that offlineasm inserts in inline asm
13+
use file slots that conflict with those in the compilation unit
14+
that includes LLIntAssembly.h (specifically,
15+
LowLevelInterpreter.cpp). Clang's built-in assembler will
16+
transparently fix that for us, but for GCC we need to
17+
post-process the generated assembler.
18+
19+
Unfortunately, cmake doesn't allow us to introduce a compiler wrapper for a
20+
single source file, so we need to create a separate target for it. This
21+
wrapping only happens when building with GCC and the user has explicitly
22+
requested debug information, either by selecting a Debug/RelWithDebInfo build
23+
or setting GCC_OFFLINEASM_SOURCE_MAP.
24+
25+
Second, GDB will only look at the line table for a compilation unit if
26+
it can first resolve the address to one of the known symbols in the
27+
file. Introduce marker symbols to work around this bug.
28+
29+
* CMakeLists.txt:
30+
131
2020-04-01 Ross Kirsling <ross.kirsling@sony.com>
232

333
Intl.NumberFormat.prototype.format must preserve sign of -0

‎Source/JavaScriptCore/Scripts/postprocess-asm‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
#!/usr/bin/env ruby
2+
3+
# Copyright (C) 2020 Igalia S. L.
4+
#
5+
# Redistribution and use in source and binary forms, with or without
6+
# modification, are permitted provided that the following conditions
7+
# are met:
8+
# 1. Redistributions of source code must retain the above copyright
9+
# notice, this list of conditions and the following disclaimer.
10+
# 2. Redistributions in binary form must reproduce the above copyright
11+
# notice, this list of conditions and the following disclaimer in the
12+
# documentation and/or other materials provided with the distribution.
13+
#
14+
# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
15+
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
16+
# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
17+
# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
18+
# BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
19+
# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
20+
# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
21+
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
22+
# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
23+
# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
24+
# THE POSSIBILITY OF SUCH DAMAGE.
25+
26+
# Wrapper for a .cpp -> .o compilation command. It
27+
# 1. converts the command to generate a `.s' file
28+
# 2. runs the ASM postprocessor on it to generate the final `.s` file
29+
# 3. assembles the `.s` file to a `.o` file
30+
31+
$asm_suffix_pre = ".pre.s"
32+
$asm_suffix = ".s"
33+
$postprocessor = "#{File.dirname($0)}/resolve-asm-file-conflicts.rb"
34+
35+
36+
$intermediate_paths = []
37+
38+
# We need to work with indices a lot and unfortunately 'getoptlong' in
39+
# the standard library doesn't expose optind, so we're going with
40+
# array searches for simplicity.
41+
def index_nofail(ary, f, errmsg)
42+
idx = ary.index { |el|
43+
f.call(el)
44+
}
45+
if idx.nil?
46+
$stderr.puts(errmsg)
47+
exit(3)
48+
end
49+
idx
50+
end
51+
52+
# Find and return the source file for this compilation command,
53+
# removing it from the args. Note that the path (A) as it appears here
54+
# (coming from a cmake rule) is likely to be different to the
55+
# anonymous argument (B) to the compilation command. However, A will
56+
# be a suffix of B.
57+
#
58+
# Exit with an error if the argument is not there. This has already
59+
# been checked by `cxx-wrapper`, otherwise we wouldn't be running.
60+
def extract_input!(args)
61+
prefix = '-DPOSTPROCESS_ASM='
62+
63+
idx = index_nofail(args, Proc.new { |arg|
64+
arg.start_with?(prefix)
65+
}, "No `-DPOSTPROCESS_ASM` argument`")
66+
67+
path = args[idx][prefix.size..-1]
68+
if path.size == 0
69+
$stderr.puts("Empty path in -DPOSTPROCESS_ASM=")
70+
exit(3)
71+
end
72+
# We only need this to be defined for the preprocessor (not any
73+
# wrapper) from now on.
74+
args[idx] = "-DPOSTPROCESS_ASM"
75+
return path
76+
end
77+
78+
# Get the index of the first argument ending in this suffix.
79+
#
80+
# Exit with an error if the argument isn't there. We're only ever
81+
# called with arguments we know are being passed in by the build
82+
# system.
83+
def get_arg_idx_suffix(args, wanted)
84+
index_nofail(args, Proc.new { |arg|
85+
arg.end_with?(wanted)
86+
}, "No argument ends with #{wanted}")
87+
end
88+
89+
# Get index of a given argument. Die if it's not there.
90+
def get_arg_idx(args, wanted)
91+
index_nofail(args, Proc.new { |arg|
92+
arg == wanted
93+
}, "No `#{wanted}` argument")
94+
end
95+
96+
# Get the index of `-o` and verify that an argument follows.
97+
# Both are guaranteed to exist (from our build system).
98+
def get_o_idx(args)
99+
i = get_arg_idx(args, '-o')
100+
if (i + 1) >= args.size
101+
$stderr.puts("No argument to `-o`")
102+
exit(3)
103+
end
104+
i
105+
end
106+
107+
# Run command and die if it fails, propagating the exit code.
108+
def run_cmd(cmd)
109+
pid = Process.spawn(*cmd)
110+
Process.waitpid(pid)
111+
ret = $?
112+
if not ret.success?
113+
$stderr.puts("Error running cmd: #{ret}")
114+
exit(ret.exitstatus)
115+
end
116+
end
117+
118+
# Convert
119+
# cxx -o blah.o -c blah.cpp
120+
# to
121+
# cxx -o blah.s -S blah.cpp
122+
def build_cxx_cmd(args)
123+
c_idx = get_arg_idx(args, '-c')
124+
o_idx = get_o_idx(args)
125+
126+
cxx_args = args.clone
127+
128+
cxx_args[c_idx] = '-S'
129+
o_path = cxx_args[o_idx + 1]
130+
cxx_args[o_idx + 1] = o_path.sub(/[.]o$/, $asm_suffix)
131+
$intermediate_paths << cxx_args[o_idx + 1]
132+
if cxx_args[o_idx + 1] == o_path
133+
$stderr.puts("Output file name not an object file: `#{o_path}`")
134+
exit(3)
135+
end
136+
cxx_args
137+
end
138+
139+
# Do
140+
# mv blah.S blah.pre.S
141+
# The reason we do a rename instead of directly generating the .pre.s
142+
# file when compiling is so that the corresponding .dwo file will have
143+
# the correct name embedded.
144+
def rename_s_file(args)
145+
o_path = args[get_o_idx(args) + 1]
146+
File.rename(o_path.sub(/[.]o$/, $asm_suffix),
147+
o_path.sub(/[.]o$/, $asm_suffix_pre))
148+
end
149+
150+
# Build
151+
# postprocessor blah.pre.S blah.S
152+
def build_postprocessor_cmd(args)
153+
o_path = args[get_o_idx(args) + 1]
154+
155+
pp_args = [
156+
$postprocessor,
157+
o_path.sub(/[.]o$/, $asm_suffix_pre), # input
158+
o_path.sub(/[.]o$/, $asm_suffix) # output
159+
]
160+
$intermediate_paths << pp_args[-2]
161+
$intermediate_paths << pp_args[-1]
162+
pp_args
163+
end
164+
165+
# Build
166+
# cxx -o blah.o -c blah.S
167+
def build_as_cmd(args, i_path)
168+
i_idx = get_arg_idx_suffix(args, i_path)
169+
o_path = args[get_o_idx(args) + 1]
170+
171+
as_args = args.clone
172+
i_path = as_args[i_idx]
173+
as_args[i_idx] = o_path.sub(/[.]o$/, $asm_suffix)
174+
as_args
175+
end
176+
177+
args = ARGV.to_a
178+
i_path = extract_input!(args)
179+
180+
begin
181+
run_cmd(build_cxx_cmd(args))
182+
rename_s_file(args)
183+
run_cmd(build_postprocessor_cmd(args))
184+
run_cmd(build_as_cmd(args, i_path))
185+
ensure
186+
$intermediate_paths.each { |p|
187+
if File.exist?(p)
188+
File.delete(p)
189+
end
190+
}
191+
end
192+
193+
exit(0)

0 commit comments

Comments
 (0)