-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[lldb][Wasm] Handle imports when parsing Wasm name sections #170960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
LLDB can use the wasm name section to populate its symbol table and get names for functions. However the index space used in the name section is the "function index space" which includes imported as well as locally defined functions.
|
@llvm/pr-subscribers-lldb Author: Derek Schuff (dschuff) ChangesLLDB can use the wasm name section to populate its symbol table and get names for functions. However the index space used in the name section is the "function index space" which includes imported as well as locally defined functions. Full diff: https://github.com/llvm/llvm-project/pull/170960.diff 4 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
index 0bedb5e753b77..1982239577b6f 100644
--- a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
+++ b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
@@ -307,6 +307,43 @@ struct WasmFunction {
uint32_t size = 0;
};
+static llvm::Expected<uint32_t>
+ParseImports(DataExtractor &import_data) {
+ // Currently this function just returns the number of imported functions.
+ // If we want to do anything with global names in the future, we'll also
+ // need to know those.
+ llvm::DataExtractor data = import_data.GetAsLLVM();
+ llvm::DataExtractor::Cursor c(0);
+
+ llvm::Expected<uint32_t> count = GetULEB32(data, c);
+ if (!count)
+ return count.takeError();
+
+ uint32_t function_imports = 0;
+ for (uint32_t i = 0; c && i < *count; ++i) {
+ // We don't need module and field names, so we can just get them as raw
+ // strings and discard.
+ if (!GetWasmString(data, c))
+ return llvm::createStringError("failed to parse module name");
+ if (!GetWasmString(data, c))
+ return llvm::createStringError("failed to parse field name");
+
+ uint8_t kind = data.getU8(c);
+ if (kind == llvm::wasm::WASM_EXTERNAL_FUNCTION) {
+ function_imports++;
+ }
+
+ // For function imports, this is a type index. For others it's different.
+ // We don't need it, just need to parse it to advance the cursor.
+ data.getULEB128(c);
+ }
+
+ if (!c)
+ return c.takeError();
+
+ return function_imports;
+}
+
static llvm::Expected<std::vector<WasmFunction>>
ParseFunctions(DataExtractor &data) {
lldb::offset_t offset = 0;
@@ -410,7 +447,7 @@ static llvm::Expected<std::vector<WasmSegment>> ParseData(DataExtractor &data) {
static llvm::Expected<std::vector<Symbol>>
ParseNames(SectionSP code_section_sp, DataExtractor &name_data,
const std::vector<WasmFunction> &functions,
- std::vector<WasmSegment> &segments) {
+ std::vector<WasmSegment> &segments, uint32_t num_imported_functions) {
llvm::DataExtractor data = name_data.GetAsLLVM();
llvm::DataExtractor::Cursor c(0);
@@ -434,15 +471,31 @@ ParseNames(SectionSP code_section_sp, DataExtractor &name_data,
llvm::Expected<std::string> name = GetWasmString(data, c);
if (!name)
return name.takeError();
- if (*idx >= functions.size())
+ if (*idx >= num_imported_functions + functions.size())
continue;
- symbols.emplace_back(
- symbols.size(), *name, lldb::eSymbolTypeCode,
- /*external=*/false, /*is_debug=*/false, /*is_trampoline=*/false,
- /*is_artificial=*/false, code_section_sp,
- functions[i].section_offset, functions[i].size,
- /*size_is_valid=*/true, /*contains_linker_annotations=*/false,
- /*flags=*/0);
+
+ if (*idx < num_imported_functions) {
+ symbols.emplace_back(
+ symbols.size(), *name, lldb::eSymbolTypeCode,
+ /*external=*/true, /*is_debug=*/false,
+ /*is_trampoline=*/false,
+ /*is_artificial=*/false,
+ /*section_sp=*/lldb::SectionSP(),
+ /*value=*/0, /*size=*/0,
+ /*size_is_valid=*/false,
+ /*contains_linker_annotations=*/false,
+ /*flags=*/0);
+ } else {
+ const WasmFunction &func = functions[*idx - num_imported_functions];
+ symbols.emplace_back(
+ symbols.size(), *name, lldb::eSymbolTypeCode,
+ /*external=*/false, /*is_debug=*/false,
+ /*is_trampoline=*/false, /*is_artificial=*/false,
+ code_section_sp, func.section_offset, func.size,
+ /*size_is_valid=*/true,
+ /*contains_linker_annotations=*/false,
+ /*flags=*/0);
+ }
}
} break;
case llvm::wasm::WASM_NAMES_DATA_SEGMENT: {
@@ -590,6 +643,20 @@ void ObjectFileWasm::CreateSections(SectionList &unified_section_list) {
}
}
+ // Parse the import section. The number of functions is needed because the
+ // function index space used in the name section includes imports.
+ if (std::optional<section_info> info =
+ GetSectionInfo(llvm::wasm::WASM_SEC_IMPORT)) {
+ DataExtractor import_data = ReadImageData(info->offset, info->size);
+ llvm::Expected<uint32_t> num_imports = ParseImports(import_data);
+ if (!num_imports) {
+ LLDB_LOG_ERROR(log, num_imports.takeError(),
+ "Failed to parse Wasm import section: {0}");
+ } else {
+ m_num_imported_functions = *num_imports;
+ }
+ }
+
// Parse the data section.
std::optional<section_info> data_info =
GetSectionInfo(llvm::wasm::WASM_SEC_DATA);
@@ -609,7 +676,7 @@ void ObjectFileWasm::CreateSections(SectionList &unified_section_list) {
DataExtractor names_data = ReadImageData(info->offset, info->size);
llvm::Expected<std::vector<Symbol>> symbols = ParseNames(
m_sections_up->FindSectionByType(lldb::eSectionTypeCode, false),
- names_data, functions, segments);
+ names_data, functions, segments, m_num_imported_functions);
if (!symbols) {
LLDB_LOG_ERROR(log, symbols.takeError(),
"Failed to parse Wasm names: {0}");
diff --git a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
index 86ecbf26803cf..17fe23c1131d2 100644
--- a/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
+++ b/lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h
@@ -146,6 +146,7 @@ class ObjectFileWasm : public ObjectFile {
/// \}
std::vector<section_info> m_sect_infos;
+ uint32_t m_num_imported_functions = 0;
std::vector<Symbol> m_symbols;
ArchSpec m_arch;
UUID m_uuid;
diff --git a/lldb/test/Shell/Symtab/Inputs/simple.wasm.yaml b/lldb/test/Shell/Symtab/Inputs/simple.wasm.yaml
index 088d6163d6b0b..8f9742c4edff8 100644
--- a/lldb/test/Shell/Symtab/Inputs/simple.wasm.yaml
+++ b/lldb/test/Shell/Symtab/Inputs/simple.wasm.yaml
@@ -10,6 +10,7 @@
# int j = 2;
# return add(i, j);
# }
+# Additional imports have been manually added and indexes adjusted after compiling
--- !WASM
FileHeader:
Version: 0x1
@@ -29,6 +30,21 @@ Sections:
ParamTypes: []
ReturnTypes:
- I32
+ - Type: IMPORT
+ Imports:
+ - Module: env
+ Field: foo
+ Kind: FUNCTION
+ SigIndex: 0
+ - Module: env
+ Field: another_import
+ Kind: FUNCTION
+ SigIndex: 0
+ - Module: env
+ Field: bar
+ Kind: GLOBAL
+ GlobalType: I32
+ GlobalMutable: true
- Type: FUNCTION
FunctionTypes: [ 0, 1, 2, 1 ]
- Type: TABLE
@@ -44,73 +60,73 @@ Sections:
- Minimum: 0x2
- Type: GLOBAL
Globals:
- - Index: 0
+ - Index: 1
Type: I32
Mutable: true
InitExpr:
Opcode: I32_CONST
Value: 66576
- - Index: 1
+ - Index: 2
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1036
- - Index: 2
+ - Index: 3
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1024
- - Index: 3
+ - Index: 4
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1040
- - Index: 4
+ - Index: 5
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1040
- - Index: 5
+ - Index: 6
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 66576
- - Index: 6
+ - Index: 7
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1024
- - Index: 7
+ - Index: 8
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 66576
- - Index: 8
+ - Index: 9
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 131072
- - Index: 9
+ - Index: 10
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 0
- - Index: 10
+ - Index: 11
Type: I32
Mutable: false
InitExpr:
Opcode: I32_CONST
Value: 1
- - Index: 11
+ - Index: 12
Type: I32
Mutable: false
InitExpr:
@@ -123,16 +139,16 @@ Sections:
Index: 0
- Name: __wasm_call_ctors
Kind: FUNCTION
- Index: 0
+ Index: 2
- Name: add
Kind: FUNCTION
- Index: 1
+ Index: 3
- Name: __original_main
Kind: FUNCTION
- Index: 2
+ Index: 4
- Name: main
Kind: FUNCTION
- Index: 3
+ Index: 5
- Name: str
Kind: GLOBAL
Index: 1
@@ -174,20 +190,20 @@ Sections:
Index: 11
- Type: CODE
Functions:
- - Index: 0
+ - Index: 2
Locals: []
Body: 0B
- - Index: 1
+ - Index: 3
Locals:
- Type: I32
Count: 1
Body: 23808080800041106B21022002200036020C20022001360208200228020C20022802086A0F0B
- - Index: 2
+ - Index: 4
Locals:
- Type: I32
Count: 2
Body: 23808080800041106B210020002480808080002000410036020C2000410136020820004102360204200028020820002802041081808080002101200041106A24808080800020010F0B
- - Index: 3
+ - Index: 5
Locals: []
Body: 1082808080000F0B
- Type: DATA
@@ -208,15 +224,19 @@ Sections:
Name: name
FunctionNames:
- Index: 0
- Name: __wasm_call_ctors
+ Name: foo
- Index: 1
- Name: add
+ Name: another_import
- Index: 2
- Name: __original_main
+ Name: __wasm_call_ctors
- Index: 3
+ Name: add
+ - Index: 4
+ Name: __original_main
+ - Index: 5
Name: main
GlobalNames:
- - Index: 0
+ - Index: 1
Name: __stack_pointer
DataSegmentNames:
- Index: 0
diff --git a/lldb/test/Shell/Symtab/symtab-wasm.test b/lldb/test/Shell/Symtab/symtab-wasm.test
index 524691b897322..e8628dbd04fb9 100644
--- a/lldb/test/Shell/Symtab/symtab-wasm.test
+++ b/lldb/test/Shell/Symtab/symtab-wasm.test
@@ -1,16 +1,18 @@
# RUN: yaml2obj %S/Inputs/simple.wasm.yaml -o %t.wasm
# RUN: %lldb %t.wasm -o 'image dump symtab' | FileCheck %s --check-prefix SYMTAB
+SYMTAB: Code 0x0000000000000000 0x0000000000000000 0x00000000 foo
+SYMTAB: Code 0x0000000000000000 0x0000000000000000 0x00000000 another_import
SYMTAB: Code 0x0000000000000002 0x0000000000000002 0x00000000 __wasm_call_ctors
SYMTAB: Code 0x0000000000000005 0x0000000000000029 0x00000000 add
SYMTAB: Code 0x000000000000002f 0x000000000000004c 0x00000000 __original_main
SYMTAB: Code 0x000000000000007c 0x0000000000000009 0x00000000 main
# RUN: %lldb %t.wasm -o 'image dump sections' | FileCheck %s --check-prefix SECTIONS
-SECTIONS: 0x0000000000000001 code [0x0000000000000000-0x0000000000000085) --- 0x000001a1 0x00000085 0x00000000 symtab-wasm.test.tmp.wasm.code
-SECTIONS: 0x0000000000000040 wasm-name --- 0x00000251 0x00000059 0x00000000 symtab-wasm.test.tmp.wasm.name
-SECTIONS: 0x0000000000000100 data [0x0000000000000400-0x0000000000000409) --- 0x00000233 0x00000009 0x00000000 symtab-wasm.test.tmp.wasm..rodata
-SECTIONS: 0x0000000000000200 data [0x000000000000040c-0x0000000000000410) --- 0x00000242 0x00000004 0x00000000 symtab-wasm.test.tmp.wasm..data
+SECTIONS: 0x0000000000000001 code [0x0000000000000000-0x0000000000000085) --- 0x000001d2 0x00000085 0x00000000 symtab-wasm.test.tmp.wasm.code
+SECTIONS: 0x0000000000000040 wasm-name --- 0x00000282 0x0000006e 0x00000000 symtab-wasm.test.tmp.wasm.name
+SECTIONS: 0x0000000000000100 data [0x0000000000000400-0x0000000000000409) --- 0x00000264 0x00000009 0x00000000 symtab-wasm.test.tmp.wasm..rodata
+SECTIONS: 0x0000000000000200 data [0x000000000000040c-0x0000000000000410) --- 0x00000273 0x00000004 0x00000000 symtab-wasm.test.tmp.wasm..data
# RUN: %lldb %t.wasm -o 'x/s 0x0000000000000400' | FileCheck %s --check-prefix STR
STR: "data str"
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I'm definitely not an expert in LLDB, so I don't know if this is the exact way to solve this issue, but the behavior should now be correct in the presence of import sections. |
LLDB can use the wasm name section to populate its symbol table and get names for functions. However the index space used in the name section is the "function index space" which includes imported as well as locally defined functions.