Change the definition of shemu_printf so that unused parameters are removed from the compiled binary

[junkzours]

With the current implementation of shemu_printf, when BDDISASM_NO_FORMAT is defined, the parameters of shemu_printf are present in the output binary even though they are not used. This is due to the fact that shemu_printf is defined as a function.

For example, strings like " Loop BREAK from RIP 0x%016llx, TARGET 0x%016llx, ITER %llu\n" can be found in the binary.
I am working on an embedded device where I do not have a lot of memory and it does not make sense in this context to embed these strings if they are not used.

This PR introduces a new declaration of shemu_printf based on a macro that enables the compilers to remove these strings from the output binary and reduce its size.

[ianichitei]

Unfortunately UNREFERENCED_PARAMETER can't be applied to __VA_ARGS__, but how about something like this?

static inline void shemu_printf_helper(SHEMU_CONTEXT *c, ...) { (void)c; }
#define shemu_printf(c, f, ...) do { (void)f; shemu_printf_helper(c, __VA_ARGS__); } while (0)

Alternatively we could disable the unused variable warnings for BDDISASM_NO_FORMAT builds.

[vlutas]

You could just rename shemu_printf to shemu_printf_internal, and do:

#ifndef BDDISASM_NO_FORMAT
#define shemu_printf    shemu_printf_internal
#else
#define shemu_printf
#endif

This does not lead to any warnings, and the formatting strings will no longer be a part of the binary.

[junkzours]

Both approaches look good to me, as long as it does remove the strings.
The PR I proposed compiles fine on MSVC, I don't have any problem with UNREFERENCED_PARAMETER applied to VA_ARGS.
On MSVC the approach mentionned by vlutas did generate errors due to unused parameters.
Feel free to fix it in the way that you prefer. I think all approaches are more or less the same.

Btw thank you for your fast replies!

[vlutas]

On MSVC the approach mentionned by vlutas did generate errors due to unused parameters.

Where, exactly? There shouldn't be any warnings/errors, if implemented correctly (I tested it locally, and there are none).

[ianichitei]

You could just rename shemu_printf to shemu_printf_internal, and do:

#ifndef BDDISASM_NO_FORMAT
#define shemu_printf    shemu_printf_internal
#else
#define shemu_printf
#endif

This does not lead to any warnings, and the formatting strings will no longer be a part of the binary.

Triggers multiple warnings with clang: https://godbolt.org/z/6WPGsvTWK

[junkzours]

Where, exactly? There shouldn't be any warnings/errors, if implemented correctly (I tested it locally, and there are none).

Ok my bad, I checked again and there are no warnings/errors with your approach. I probably did not implemented it exactly like that when I tested previously.
I would say this is my prefered solution as it is super simple.

[Edit] Just saw the comment from ianichitei. Checking the approach suggested by ianichitei.

[vlutas]

Triggers multiple warnings with clang: https://godbolt.org/z/6WPGsvTWK

True, but we can accept -Wno-unused-value as a solution in this case.

[junkzours]

static inline void shemu_printf_helper(SHEMU_CONTEXT *c, ...) { (void)c; }
#define shemu_printf(c, f, ...) do { (void)f; shemu_printf_helper(c, __VA_ARGS__); } while (0)

This does not work either because the formatstring is missing in the invocation of shemu_printf_helper and leads to types mismatch when interpreting VA_ARGS:
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(711,9): error C2220: the following warning is treated as an error
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(711,9): warning C4047: 'function': 'char *' differs in levels of indirection from 'ND_UINT64'
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(711,9): warning C4024: 'shemu_internal_printf': different types for formal and actual parameter 2
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(732,9): warning C4047: 'function': 'char *' differs in levels of indirection from 'ND_UINT64'
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(732,9): warning C4024: 'shemu_internal_printf': different types for formal and actual parameter 2
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(1274,17): warning C4047: 'function': 'char *' differs in levels of indirection from 'ND_UINT32'
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(1274,17): warning C4024: 'shemu_internal_printf': different types for formal and actual parameter 2
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(1297,17): warning C4047: 'function': 'char *' differs in levels of indirection from 'int'
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(1297,17): warning C4024: 'shemu_internal_printf': different types for formal and actual parameter 2
2>C:\git\bddisasm\bdshemu\bdshemu_x86.c(1302,17): warning C4047: 'function': 'char *' differs in levels of indirection from 'int'
Etc.

[ianichitei]

Triggers multiple warnings with clang: https://godbolt.org/z/6WPGsvTWK

True, but we can accept -Wno-unused-value as a solution in this case.

Yes, we can solve this from our CMake build script. Speaking of which, the way we always enable all warnings isn't friendly to people who consume bddisasm as a subproject anyway.

A solution that will work regardless of compiler and compiler flags:

#define shemu_printf(c, f, ...) do { (void)f; shemu_printf_internal(c, NULL, __VA_ARGS__); } while (0)

With shemu_printf_internal implemented exactly as shemu_printf is implemented now for BDDISASM_NO_FORMAT.

[vlutas]

Yes, we can solve this from our CMake build script. Speaking of which, the way we always enable all warnings isn't friendly to people who consume bddisasm as a subproject anyway.

A solution that will work regardless of compiler and compiler flags:

#define shemu_printf(c, f, ...) do { (void)f; shemu_printf_internal(c, NULL, __VA_ARGS__); } while (0)

With shemu_printf_internal implemented exactly as shemu_printf is implemented now for BDDISASM_NO_FORMAT.

This works as well.

[junkzours]

I fixed the code on my branch according to your last comment, and pushed.

[ianichitei]

This part needs to stay the same, only the name must be changed to shemu_internal_printf.

[junkzours]

Done

[vlutas]

There is no NULL defined in bddisasm/bdshemu. There is, instead, ND_NULL - which you should use instead.

[junkzours]

Fixed

[ianichitei]

All checks passed, the required one that's stuck can be ignored (not a real check, config issue).