Silence -Wunused-parameter warnings in Unwind-wasm.c

[georgthegreat]

No description provided.

[llvmbot]

@llvm/pr-subscribers-libunwind

Author: Yuriy Chernyshov (georgthegreat)

Changes

---

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

1 Files Affected:

• (modified) libunwind/src/Unwind-wasm.c (+3-3)

diff --git a/libunwind/src/Unwind-wasm.c b/libunwind/src/Unwind-wasm.c
index b18b32c5d17847..fe016b8f5923ad 100644
--- a/libunwind/src/Unwind-wasm.c
+++ b/libunwind/src/Unwind-wasm.c
@@ -102,8 +102,8 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {
 }
 
 /// Not used in Wasm.
-_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context,
-                                     uintptr_t value) {}
+_LIBUNWIND_EXPORT void _Unwind_SetIP([[maybe_unused]] struct _Unwind_Context *context,
+                                     [[maybe_unused]] uintptr_t value) {}
 
 /// Called by personality handler to get LSDA for current frame.
 _LIBUNWIND_EXPORT uintptr_t
@@ -116,7 +116,7 @@ _Unwind_GetLanguageSpecificData(struct _Unwind_Context *context) {
 
 /// Not used in Wasm.
 _LIBUNWIND_EXPORT uintptr_t
-_Unwind_GetRegionStart(struct _Unwind_Context *context) {
+_Unwind_GetRegionStart([[maybe_unused]] struct _Unwind_Context *context) {
   return 0;
 }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[MaskRay]

What's your build option?

llvm-project considers this warning low value and llvm/cmake/modules/HandleLLVMOptions.cmake specifies -Wno-unused-parameter.

[georgthegreat]

@MaskRay, we build libunwind using our own build system with the clang default set of warnings enabled.

There is already a couple of [[maybe_unused]] attributes in this library, so I do not bring anything new:

(dflt) thegeorg@jakku:~/contrib/llvm/libunwind@main$ rg unused]]
src/Unwind-EHABI.cpp
890:[[gnu::unused]] static uint64_t

src/UnwindCursor.hpp
3050:  [[maybe_unused]] const int Result = syscall(

[ldionne]

IMO this is reasonable but it's simpler to drop the parameter names in this case, no need for [[maybe_unused]].

[georgthegreat]

I applied @ldionne suggestions, I am fine with his approach.

[ldionne]

I'm fine with this, but I don't want to override @MaskRay 's (dis)approval since his opinion prevails on libunwind matters. If @MaskRay doesn't raise any concerns within a few days, I think this would be reasonable to merge since it's fairly harmless.

[georgthegreat]

@ldionne, may we proceed with merging this?

[ldionne]

Sure, but I am not certain why you added some newlines though. Can you leave the formatting as it was?

[georgthegreat]

I think this is how my clang-format-16 auto-formatted it.
I have applied the formatting from the check, it is green now.

[aheejin]

With these parameters gone, now we have this warning: (-Wc23-extensions is enabled by default unless you use -std=c23)

.../libunwind/src/Unwind-wasm.c:105:62: error: omitting the parameter name in a function
  definition is a C23 extension [-Werror,-Wc23-extensions]
    105 | _LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *, uintptr_t) {}
        |

[sbc100]

It seems that libunwind builds with C++17:

llvm-project/libunwind/src/CMakeLists.txt

Lines 139 to 145 in 3bf5384

	set_target_properties(unwind_shared_objects
	PROPERTIES
	CXX_EXTENSIONS OFF
	CXX_STANDARD 17
	CXX_STANDARD_REQUIRED ON
	COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
	)

Which means this change actually breaks the build. We were about to bump to C++23 downstream but that seems like the wrong solution if upstream is on C++17: emscripten-core/emscripten#26037 (comment)

If this analysis is correct then I propose this change is reverted, and that the revert is backported to LLVM 21.

[sbc100]

In several other places in libunwind it looks like more basic methods of suppressing this warning are used:

llvm-project/libunwind/src/AddressSpace.hpp

Lines 465 to 466 in 3bf5384

	// Avoid warning about unused variable.
	(void)pinfo_size;

[sbc100]

I confirmed the LLVM builds these files with -Wno-unused-parameter. For example here is the command line to build UnwindLevel1.c on my system:

[16/21] /usr/local/google/home/sbc/dev/wasm/llvm-build/bin/clang --target=x86_64-unknown-linux-gnu -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBUNWIND_LINK_DL_LIB -D_LIBUNWIND_LINK_PTHREAD_LIB -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/usr/local/google/home/sbc/dev/wasm/llvm-project/libunwind/include -fPIC -fno-semantic-interposition -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -Werror=return-type -funwind-tables -nostdinc++ -D_DEBUG -UNDEBUG -D_LIBUNWIND_IS_NATIVE_ONLY -UNDEBUG -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wzero-length-array -Wdeprecated-redundant-constexpr-static-def -Wno-nullability-completeness -Wmissing-prototypes -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Wno-error -pedantic -fno-rtti -std=c99 -fexceptions -MD -MT libunwind/src/CMakeFiles/unwind_static_objects.dir/UnwindLevel1.c.o -MF libunwind/src/CMakeFiles/unwind_static_objects.dir/UnwindLevel1.c.o.d -o libunwind/src/CMakeFiles/unwind_static_objects.dir/UnwindLevel1.c.o -c /usr/local/google/home/sbc/dev/wasm/llvm-project/libunwind/src/UnwindLevel1.c

@georgthegreat I suggest you update your downstream build system to match the llvm flags, otherwise you will likely run into more issues like that.