[lld] [llvm] [WebAssembly][Object] Support more elem segment flags (PR #123427)
Derek Schuff via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 17 16:17:57 PST 2025
https://github.com/dschuff updated https://github.com/llvm/llvm-project/pull/123427
>From e0368613aae8bfaa4f59981ee7b4a362d7f8c790 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Wed, 15 Jan 2025 15:18:52 -0800
Subject: [PATCH 1/5] Support more elem segment flags
---
llvm/include/llvm/BinaryFormat/Wasm.h | 8 +++++
llvm/lib/Object/WasmObjectFile.cpp | 43 ++++++++++++++++++---------
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 759e4321250912..66b9cb1cdc8359 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -415,6 +415,14 @@ struct WasmDataSegment {
uint32_t Comdat; // from the "comdat info" section
};
+// 3 different element segment modes are encodable. This class is currently
+// only used during decoding (see WasmElemSegment below).
+enum class ElemSegmentMode {
+ Active,
+ Passive,
+ Declarative
+};
+
// Represents a Wasm element segment, with some limitations compared the spec:
// 1) Does not model passive or declarative segments (Segment will end up with
// an Offset field of i32.const 0)
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 2c9b878a4cde93..41499c98ccdc5f 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1440,15 +1440,22 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
Info.Flags = 0;
switch (Ex.Kind) {
case wasm::WASM_EXTERNAL_FUNCTION: {
- if (!isDefinedFunctionIndex(Ex.Index))
+ if (!isValidFunctionIndex(Ex.Index))
return make_error<GenericBinaryError>("invalid function export",
object_error::parse_failed);
- getDefinedFunction(Ex.Index).ExportName = Ex.Name;
Info.Kind = wasm::WASM_SYMBOL_TYPE_FUNCTION;
Info.ElementIndex = Ex.Index;
- unsigned FuncIndex = Info.ElementIndex - NumImportedFunctions;
- wasm::WasmFunction &Function = Functions[FuncIndex];
- Signature = &Signatures[Function.SigIndex];
+ if (isDefinedFunctionIndex(Ex.Index)) {
+ getDefinedFunction(Ex.Index).ExportName = Ex.Name;
+ unsigned FuncIndex = Info.ElementIndex - NumImportedFunctions;
+ wasm::WasmFunction &Function = Functions[FuncIndex];
+ Signature = &Signatures[Function.SigIndex];
+ break;
+ } else {
+ // Function is imported. LLVM object files don't use this pattern and
+ // we still treat this as an undefined symbol, but we want to parse
+ // it.
+ }
break;
}
case wasm::WASM_EXTERNAL_GLOBAL: {
@@ -1645,17 +1652,24 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
return make_error<GenericBinaryError>(
"Unsupported flags for element segment", object_error::parse_failed);
- bool IsPassive = (Segment.Flags & wasm::WASM_ELEM_SEGMENT_IS_PASSIVE) != 0;
- bool IsDeclarative =
- IsPassive && (Segment.Flags & wasm::WASM_ELEM_SEGMENT_IS_DECLARATIVE);
+ wasm::ElemSegmentMode Mode;
+ if ((Segment.Flags & wasm::WASM_ELEM_SEGMENT_IS_PASSIVE) == 0) {
+ Mode = wasm::ElemSegmentMode::Active;
+ } else if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_IS_DECLARATIVE) {
+ Mode = wasm::ElemSegmentMode::Declarative;
+ } else {
+ Mode = wasm::ElemSegmentMode::Passive;
+ }
bool HasTableNumber =
- !IsPassive &&
+ Mode == wasm::ElemSegmentMode::Active &&
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER);
- bool HasInitExprs =
- (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
bool HasElemKind =
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) &&
- !HasInitExprs;
+ ~(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
+ bool HasElemType = (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS) &&
+ ~(Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND);
+ bool HasInitExprs =
+ (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
if (HasTableNumber)
Segment.TableNumber = readVaruint32(Ctx);
@@ -1666,7 +1680,7 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
return make_error<GenericBinaryError>("invalid TableNumber",
object_error::parse_failed);
- if (IsPassive || IsDeclarative) {
+ if (Mode != wasm::ElemSegmentMode::Active) {
Segment.Offset.Extended = false;
Segment.Offset.Inst.Opcode = wasm::WASM_OPCODE_I32_CONST;
Segment.Offset.Inst.Value.Int32 = 0;
@@ -1692,10 +1706,11 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
object_error::parse_failed);
Segment.ElemKind = wasm::ValType::FUNCREF;
}
- } else if (HasInitExprs) {
+ } else if (HasElemType) {
auto ElemType = parseValType(Ctx, readVaruint32(Ctx));
Segment.ElemKind = ElemType;
} else {
+ // Here the type is nullable (ref null func) but we don't distinguish.
Segment.ElemKind = wasm::ValType::FUNCREF;
}
>From 3930ba4c8971cf0b15a13bf94bcb8bd5415d5e58 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Fri, 17 Jan 2025 15:58:25 -0800
Subject: [PATCH 2/5] rename for clarity
---
lld/wasm/SyntheticSections.cpp | 2 +-
llvm/include/llvm/BinaryFormat/Wasm.h | 2 +-
llvm/lib/MC/WasmObjectWriter.cpp | 2 +-
llvm/lib/Object/WasmObjectFile.cpp | 8 ++++----
llvm/lib/ObjectYAML/WasmEmitter.cpp | 2 +-
llvm/lib/ObjectYAML/WasmYAML.cpp | 2 +-
llvm/test/Object/Inputs/WASM/multi-table.wasm | Bin 185 -> 190 bytes
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index 715fba1ee6da54..7fb44b9f0c0091 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -594,7 +594,7 @@ void ElemSection::writeBody() {
}
writeInitExpr(os, initExpr);
- if (flags & WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) {
+ if (flags & WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) {
// We only write active function table initializers, for which the elem kind
// is specified to be written as 0x00 and interpreted to mean "funcref".
const uint8_t elemKind = 0;
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 66b9cb1cdc8359..19097b458fb93b 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -170,7 +170,7 @@ enum : unsigned {
WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER = 0x02, // if passive == 0
WASM_ELEM_SEGMENT_HAS_INIT_EXPRS = 0x04,
};
-const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
+const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC = 0x3;
// Feature policy prefixes used in the custom "target_features" section
enum : uint8_t {
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 29a8c53d350a40..8ddbe929e68b94 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1019,7 +1019,7 @@ void WasmObjectWriter::writeElemSection(
encodeSLEB128(InitialTableOffset, W->OS);
W->OS << char(wasm::WASM_OPCODE_END);
- if (Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) {
+ if (Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) {
// We only write active function table initializers, for which the elem kind
// is specified to be written as 0x00 and interpreted to mean "funcref".
const uint8_t ElemKind = 0;
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 41499c98ccdc5f..7951bd0a606c76 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1664,10 +1664,10 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
Mode == wasm::ElemSegmentMode::Active &&
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER);
bool HasElemKind =
- (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) &&
- ~(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
- bool HasElemType = (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS) &&
- ~(Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND);
+ (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) &&
+ !(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
+ bool HasElemType = (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) &&
+ (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
bool HasInitExprs =
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
diff --git a/llvm/lib/ObjectYAML/WasmEmitter.cpp b/llvm/lib/ObjectYAML/WasmEmitter.cpp
index 817d364694b43a..bd016764f58626 100644
--- a/llvm/lib/ObjectYAML/WasmEmitter.cpp
+++ b/llvm/lib/ObjectYAML/WasmEmitter.cpp
@@ -497,7 +497,7 @@ void WasmWriter::writeSectionContent(raw_ostream &OS,
writeInitExpr(OS, Segment.Offset);
- if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) {
+ if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) {
// We only support active function table initializers, for which the elem
// kind is specified to be written as 0x00 and interpreted to mean
// "funcref".
diff --git a/llvm/lib/ObjectYAML/WasmYAML.cpp b/llvm/lib/ObjectYAML/WasmYAML.cpp
index 0636e19e05353d..6af66ba62be188 100644
--- a/llvm/lib/ObjectYAML/WasmYAML.cpp
+++ b/llvm/lib/ObjectYAML/WasmYAML.cpp
@@ -381,7 +381,7 @@ void MappingTraits<WasmYAML::ElemSegment>::mapping(
Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER)
IO.mapOptional("TableNumber", Segment.TableNumber);
if (!IO.outputting() ||
- Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND)
+ Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC)
IO.mapOptional("ElemKind", Segment.ElemKind);
// TODO: Omit "offset" for passive segments? It's neither meaningful nor
// encoded.
diff --git a/llvm/test/Object/Inputs/WASM/multi-table.wasm b/llvm/test/Object/Inputs/WASM/multi-table.wasm
index 47f5d8311cb74f76485577df85578b62f896361d..81e52a2d3e28658cfdb2a488bcd1bce7fea16575 100644
GIT binary patch
delta 35
ncmdnVxQ}sy6H^xVM3)pX7Dond1}=VPR)$MVQryf8j9?4^javnp
delta 30
icmdnTxRY^$6H_YJM3)pnE`DZKhD%IR+{_G&U<?3pss%p)
>From 256d901ebac58e07c8858aa52f52b4694f99f1e0 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Fri, 17 Jan 2025 16:01:26 -0800
Subject: [PATCH 3/5] clean up
---
llvm/lib/Object/WasmObjectFile.cpp | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 7951bd0a606c76..871f3cf8d50d38 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1450,12 +1450,10 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
unsigned FuncIndex = Info.ElementIndex - NumImportedFunctions;
wasm::WasmFunction &Function = Functions[FuncIndex];
Signature = &Signatures[Function.SigIndex];
- break;
- } else {
- // Function is imported. LLVM object files don't use this pattern and
- // we still treat this as an undefined symbol, but we want to parse
- // it.
}
+ // Else the function is imported. LLVM object files don't use this
+ // pattern and we still treat this as an undefined symbol, but we want to
+ // parse it without crashing.
break;
}
case wasm::WASM_EXTERNAL_GLOBAL: {
>From a6f60fc120e856d805c5f99278c46cca81950a37 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Fri, 17 Jan 2025 16:08:00 -0800
Subject: [PATCH 4/5] remove bad comment
---
llvm/lib/Object/WasmObjectFile.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 871f3cf8d50d38..beed5774813484 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1708,7 +1708,6 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
auto ElemType = parseValType(Ctx, readVaruint32(Ctx));
Segment.ElemKind = ElemType;
} else {
- // Here the type is nullable (ref null func) but we don't distinguish.
Segment.ElemKind = wasm::ValType::FUNCREF;
}
>From 28b5205a066fe58b1cf9ae09976b83a954e1315a Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Fri, 17 Jan 2025 16:17:33 -0800
Subject: [PATCH 5/5] clang-format
---
llvm/include/llvm/BinaryFormat/Wasm.h | 6 +-----
llvm/lib/Object/WasmObjectFile.cpp | 5 +++--
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 19097b458fb93b..ede2d692a59491 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -417,11 +417,7 @@ struct WasmDataSegment {
// 3 different element segment modes are encodable. This class is currently
// only used during decoding (see WasmElemSegment below).
-enum class ElemSegmentMode {
- Active,
- Passive,
- Declarative
-};
+enum class ElemSegmentMode { Active, Passive, Declarative };
// Represents a Wasm element segment, with some limitations compared the spec:
// 1) Does not model passive or declarative segments (Segment will end up with
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index beed5774813484..0f6fd5612f9d82 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1664,8 +1664,9 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
bool HasElemKind =
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) &&
!(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
- bool HasElemType = (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) &&
- (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
+ bool HasElemType =
+ (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_DESC) &&
+ (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
bool HasInitExprs =
(Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS);
More information about the llvm-commits
mailing list