Skip to content

Commit 47fc936

Browse files
xanlpzwebkit-commit-queue
authored andcommitted
[JSC] Improve offlineasm debug annotations for Linux/ELF
https://bugs.webkit.org/show_bug.cgi?id=232303 Patch by Xan López <[email protected]> on 2021-10-26 Reviewed by Mark Lam. This patch does two things: Add the .size and .type directives to every llint "function" (global, llint opcode, 'glue'). This allows a debugger to tell you in what logical function you are inside the giant chunk of code that is the llint interpreter. So instead of something like this: (gdb) x/5i $pc => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>: b.n 0xf5f8af6c <wasmLLIntPCRangeStart+3868> 0xf5f8af62 <wasmLLIntPCRangeStart+3858>: ldr r2, [r7, #8] 0xf5f8af64 <wasmLLIntPCRangeStart+3860>: ldr r2, [r2, #28] 0xf5f8af66 <wasmLLIntPCRangeStart+3862>: subs r0, #16 0xf5f8af68 <wasmLLIntPCRangeStart+3864>: ldr.w r0, [r2, r0, lsl #3] you get something like this: (gdb) x/5i $pc => 0xf5f8c770 <wasm_f32_add+12>: bge.n 0xf5f8c77c <wasm_f32_add+24> 0xf5f8c772 <wasm_f32_add+14>: add.w r6, r7, r9, lsl #3 0xf5f8c776 <wasm_f32_add+18>: vldr d0, [r6] 0xf5f8c77a <wasm_f32_add+22>: b.n 0xf5f8c78c <wasm_f32_add+40> 0xf5f8c77c <wasm_f32_add+24>: ldr r2, [r7, #8] The other change adds a local symbol (in addition to an internal label) to all the "glue" labels. That allows wasm opcodes to be seen by the debugger (and the user to break on them), among other things. * CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux. * llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels. * offlineasm/asm.rb: emit the .size and .type directives for every llint "function" on ELF systems. Canonical link: https://commits.webkit.org/243545@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284868 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 52f40eb commit 47fc936

File tree

4 files changed

+61
-2
lines changed

4 files changed

+61
-2
lines changed

Source/JavaScriptCore/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ else ()
441441
set(LLIntOutput LLIntAssembly.h)
442442
endif ()
443443

444+
if (CMAKE_SYSTEM_NAME MATCHES "Linux")
445+
set(OFFLINE_ASM_ARGS --binary-format=ELF)
446+
endif ()
447+
444448
add_custom_command(
445449
OUTPUT ${JavaScriptCore_DERIVED_SOURCES_DIR}/${LLIntOutput}
446450
MAIN_DEPENDENCY ${JAVASCRIPTCORE_DIR}/offlineasm/asm.rb

Source/JavaScriptCore/ChangeLog

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,43 @@
1+
2021-10-26 Xan López <[email protected]>
2+
3+
[JSC] Improve offlineasm debug annotations for Linux/ELF
4+
https://bugs.webkit.org/show_bug.cgi?id=232303
5+
6+
Reviewed by Mark Lam.
7+
8+
This patch does two things:
9+
10+
Add the .size and .type directives to every llint "function"
11+
(global, llint opcode, 'glue'). This allows a debugger to tell you
12+
in what logical function you are inside the giant chunk of code
13+
that is the llint interpreter. So instead of something like this:
14+
15+
(gdb) x/5i $pc
16+
=> 0xf5f8af60 <wasmLLIntPCRangeStart+3856>: b.n 0xf5f8af6c <wasmLLIntPCRangeStart+3868>
17+
0xf5f8af62 <wasmLLIntPCRangeStart+3858>: ldr r2, [r7, #8]
18+
0xf5f8af64 <wasmLLIntPCRangeStart+3860>: ldr r2, [r2, #28]
19+
0xf5f8af66 <wasmLLIntPCRangeStart+3862>: subs r0, #16
20+
0xf5f8af68 <wasmLLIntPCRangeStart+3864>: ldr.w r0, [r2, r0, lsl #3]
21+
22+
you get something like this:
23+
24+
(gdb) x/5i $pc
25+
=> 0xf5f8c770 <wasm_f32_add+12>: bge.n 0xf5f8c77c <wasm_f32_add+24>
26+
0xf5f8c772 <wasm_f32_add+14>: add.w r6, r7, r9, lsl #3
27+
0xf5f8c776 <wasm_f32_add+18>: vldr d0, [r6]
28+
0xf5f8c77a <wasm_f32_add+22>: b.n 0xf5f8c78c <wasm_f32_add+40>
29+
0xf5f8c77c <wasm_f32_add+24>: ldr r2, [r7, #8]
30+
31+
The other change adds a local symbol (in addition to an internal
32+
label) to all the "glue" labels. That allows wasm opcodes to be
33+
seen by the debugger (and the user to break on them), among other
34+
things.
35+
36+
* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
37+
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
38+
* offlineasm/asm.rb: emit the .size and .type directives for every
39+
llint "function" on ELF systems.
40+
141
2021-10-25 Yusuke Suzuki <[email protected]>
242

343
[JSC] Fix stale assertion in InternalFunctionAllocationProfile after r284757

Source/JavaScriptCore/llint/LowLevelInterpreter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,9 @@ JSValue CLoop::execute(OpcodeID entryOpcodeID, void* executableAddress, VM* vm,
490490
OFFLINE_ASM_OPCODE_DEBUG_LABEL(llint_##__opcode) \
491491
OFFLINE_ASM_LOCAL_LABEL(llint_##__opcode)
492492

493-
#define OFFLINE_ASM_GLUE_LABEL(__opcode) OFFLINE_ASM_LOCAL_LABEL(__opcode)
493+
#define OFFLINE_ASM_GLUE_LABEL(__opcode) \
494+
OFFLINE_ASM_OPCODE_DEBUG_LABEL(__opcode) \
495+
OFFLINE_ASM_LOCAL_LABEL(__opcode)
494496

495497
#if CPU(ARM_THUMB2)
496498
#define OFFLINE_ASM_GLOBAL_LABEL(label) \

Source/JavaScriptCore/offlineasm/asm.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,12 @@ def putsLabel(labelName, isGlobal)
264264
@outp.puts(formatDump(" _#{labelName}:", lastComment))
265265
end
266266
end
267+
if $emitELFDebugDirectives
268+
deferNextLabelAction {
269+
putStr(" \".size #{labelName} , . - #{labelName} \\n\"")
270+
putStr(" \".type #{labelName} , function \\n\"")
271+
}
272+
end
267273
@newlineSpacerState = :none # After a global label, we can use another spacer.
268274
end
269275

@@ -341,14 +347,17 @@ def debugAnnotation(text)
341347

342348
$options = {}
343349
OptionParser.new do |opts|
344-
opts.banner = "Usage: asm.rb asmFile offsetsFile outputFileName [--assembler=<ASM>] [--webkit-additions-path=<path>]"
350+
opts.banner = "Usage: asm.rb asmFile offsetsFile outputFileName [--assembler=<ASM>] [--webkit-additions-path=<path>] [--binary-format=<format>]"
345351
# This option is currently only used to specify the masm assembler
346352
opts.on("--assembler=[ASM]", "Specify an assembler to use.") do |assembler|
347353
$options[:assembler] = assembler
348354
end
349355
opts.on("--webkit-additions-path=PATH", "WebKitAdditions path.") do |path|
350356
$options[:webkit_additions_path] = path
351357
end
358+
opts.on("--binary-format=FORMAT", "Specify the binary format used by the target system.") do |format|
359+
$options[:binary_format] = format
360+
end
352361
end.parse!
353362

354363
begin
@@ -366,6 +375,9 @@ def debugAnnotation(text)
366375
$emitWinAsm = isMSVC ? outputFlnm.index(".asm") != nil : false
367376
$commentPrefix = $emitWinAsm ? ";" : "//"
368377

378+
# We want this in all ELF systems we support, except for C_LOOP (we'll disable it later on if we are building cloop)
379+
$emitELFDebugDirectives = $options.has_key?(:binary_format) && $options[:binary_format] == "ELF"
380+
369381
inputHash =
370382
$commentPrefix + " offlineasm input hash: " + parseHash(asmFile, $options) +
371383
" " + Digest::SHA1.hexdigest(configurationList.map{|v| (v[0] + [v[1]]).join(' ')}.join(' ')) +
@@ -412,6 +424,7 @@ def debugAnnotation(text)
412424
if backend == "C_LOOP" || backend == "C_LOOP_WIN"
413425
$enableDebugAnnotations = false
414426
$preferredCommentStartColumn = 60
427+
$emitELFDebugDirectives = false
415428
end
416429

417430
lowLevelAST = lowLevelAST.demacroify({})

0 commit comments

Comments
 (0)