Skip to content

Commit

Permalink
[WGSL] Empty programs should be valid
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259655
rdar://113153864

Reviewed by Dan Glastonbury.

This fixes two bugs related to evaluating an empty program:
1) The wgslc executable uses FileSystem::readEntireFile to read the input file,
   which returns nullopt if the file is empty, which is indistinguishable from
   when it fails to open the file. To fix that we check if the file exists first
   and then treat nullopt as an empty file.
2) The ASTBuilder assumed that we would always have had an allocation when saving
   and restoring its state. This caused a crash with empty programs, since no nodes
   are allocated. The fix is simply to check if we actually have an arena before
   trying to read it.

* Source/WebGPU/WGSL/AST/ASTBuilder.cpp:
(WGSL::AST::Builder::saveCurrentState):
(WGSL::AST::Builder::restore):
* Source/WebGPU/WGSL/tests/valid/empty.wgsl: Added.
* Source/WebGPU/WGSL/wgslc.cpp:
(runWGSL):

Canonical link: https://commits.webkit.org/266469@main
  • Loading branch information
tadeuzagallo committed Aug 1, 2023
1 parent 95275b6 commit 119aaa4
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
15 changes: 11 additions & 4 deletions Source/WebGPU/WGSL/AST/ASTBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ auto Builder::saveCurrentState() -> State
State state;
state.m_arena = m_arena;
#if ASSERT_ENABLED
state.m_arenaStart = arena();
state.m_arenaEnd = m_arenaEnd;
if (m_arenaEnd)
state.m_arenaStart = arena();
else
state.m_arenaStart = nullptr;
#endif
state.m_numberOfArenas = m_arenas.size();
state.m_numberOfNodes = m_nodes.size();
Expand All @@ -82,11 +85,15 @@ void Builder::restore(State&& state)
m_nodes.shrink(state.m_numberOfNodes);
m_arena = state.m_arena;
m_arenas.shrink(state.m_numberOfArenas);
m_arenaEnd = m_arenas.last().get() + arenaSize;
if (m_arenas.isEmpty())
m_arenaEnd = nullptr;
else {
m_arenaEnd = m_arenas.last().get() + arenaSize;
#if ASSERT_ENABLED
ASSERT(state.m_arenaStart == m_arenas.last().get());
ASSERT(state.m_arenaEnd == m_arenaEnd);
ASSERT(state.m_arenaStart == m_arenas.last().get());
ASSERT(state.m_arenaEnd == m_arenaEnd);
#endif
}
}

} // namespace WGSL::AST
1 change: 1 addition & 0 deletions Source/WebGPU/WGSL/tests/valid/empty.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// RUN: %wgslc
11 changes: 8 additions & 3 deletions Source/WebGPU/WGSL/wgslc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,18 @@ static int runWGSL(const CommandLine& options)


String fileName = String::fromLatin1(options.file());
auto readResult = FileSystem::readEntireFile(fileName);
if (!readResult.has_value()) {
auto handle = FileSystem::openFile(fileName, FileSystem::FileOpenMode::Read);
if (!FileSystem::isHandleValid(handle)) {
FileSystem::closeFile(handle);
dataLogLn("Failed to open ", fileName);
return EXIT_FAILURE;
}

auto source = String::fromUTF8WithLatin1Fallback(readResult->data(), readResult->size());
auto readResult = FileSystem::readEntireFile(handle);
FileSystem::closeFile(handle);
auto source = emptyString();
if (readResult.has_value())
source = String::fromUTF8WithLatin1Fallback(readResult->data(), readResult->size());
auto checkResult = WGSL::staticCheck(source, std::nullopt, configuration);
if (auto* failedCheck = std::get_if<WGSL::FailedCheck>(&checkResult)) {
for (const auto& error : failedCheck->errors)
Expand Down

0 comments on commit 119aaa4

Please sign in to comment.