Skip to content

[llvm-debuginfo-analyzer] Fix incorrect REQUIRES in WebAssembly test case #145848

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

Conversation

CarlosAlbertoEnciso
Copy link
Member

Fixes an incorrect 'REQUIRES' in test case.
The correct value should be:

REQUIRES: webassembly-registered-target

…141616)

llvm#136772
Incorrect handling of 'tombstone' value for WebAssembly.

llvm-debuginfo-analyzer already uses the tombstone approach
to identify dead code. Currently, the tombstone value is
evaluated as std::numeric_limits<uint64_t>::max(). Which is
wrong as it does not take into account the 'Address Byte Size'
from the Compile Unit header.

Fix incorrect 'REQUIRES'. The correct value should be:

REQUIRES: webassembly-registered-target
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-backend-webassembly

@llvm/pr-subscribers-debuginfo

Author: Carlos Alberto Enciso (CarlosAlbertoEnciso)

Changes

Fixes an incorrect 'REQUIRES' in test case.
The correct value should be:

REQUIRES: webassembly-registered-target


Full diff: https://github.com/llvm/llvm-project/pull/145848.diff

1 Files Affected:

  • (modified) llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/wasm-32bit-tombstone.s (+1-1)
diff --git a/llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/wasm-32bit-tombstone.s b/llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/wasm-32bit-tombstone.s
index 3e1c1016c9aee..3609b3c8ecc3e 100644
--- a/llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/wasm-32bit-tombstone.s
+++ b/llvm/test/tools/llvm-debuginfo-analyzer/WebAssembly/wasm-32bit-tombstone.s
@@ -1,4 +1,4 @@
-# REQUIRES: x86-registered-target
+# REQUIRES: webassembly-registered-target
 
 # Test that DWARF tombstones are correctly detected/respected in wasm
 # 32 bit object files.

Copy link
Collaborator

@dyung dyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@CarlosAlbertoEnciso CarlosAlbertoEnciso merged commit 8b55129 into llvm:main Jun 26, 2025
12 checks passed
@CarlosAlbertoEnciso
Copy link
Member Author

@dyung Thanks for your approval.

@CarlosAlbertoEnciso CarlosAlbertoEnciso deleted the wasm-tombstone-crash-invalid-target branch June 26, 2025 08:56
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
…case (llvm#145848)

Fixes an incorrect 'REQUIRES' in test case.
The correct value should be:

REQUIRES: webassembly-registered-target
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants