[BUG] Tools fail to build on i686 and musl systems

[cinerea0]

Describe the bug
The tools found in the "tools" directory in this repository cannot be built on i686 or x86_64-musl systems. On i686 systems the tools fail to build because of type mismatch (My knowledge of C is minimal, but it seems like "ld" should be "zd" in convert.c.). On x86_64-musl systems the tools fail to build because a topology file can't be parsed. The full error logs for these can be found here: void-linux/void-packages#33318.

To Reproduce
Attempt to build the tools on the platforms listed above.

Reproduction Rate
This happens every build attempt.

Expected behavior
I expect the tools to build correctly.

Impact
This prevents Void Linux from distributing the tools since they currently cannot be built from source on all of the expected architectures. Additionally, due to the way the tools used to be distributed, Void cannot provide an update to v1.9 of sof-firmware.

Environment

1. Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
   • Kernel: N/A
   • SOF: fa8575d
2. Name of the topology file
   • Topology: sof-cml-nocodec.tplg
3. Name of the platform(s) on which the bug is observed.
   • Platform: i686 and x86_64-musl

Screenshots or console output

i686 error

[54/388] Building C object logger/CMakeFiles/sof-logger.dir/convert.c.o
FAILED: logger/CMakeFiles/sof-logger.dir/convert.c.o 
/usr/bin/cc  -I/builddir/sof-1.9/tools/../src/include -I/builddir/sof-1.9/tools/../rimage/src/include -I/builddir/sof-1.9/tools/.. -DNDEBUG -fstack-clash-protection -D_FORTIFY_SOURCE=2 -mtune=i686 -O2 -Wall -Werror -MD -MT logger/CMakeFiles/sof-logger.dir/convert.c.o -MF logger/CMakeFiles/sof-logger.dir/convert.c.o.d -o logger/CMakeFiles/sof-logger.dir/convert.c.o -c /builddir/sof-1.9/tools/logger/convert.c
/builddir/sof-1.9/tools/logger/convert.c: In function 'fetch_entry':
/builddir/sof-1.9/tools/logger/convert.c:746:44: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
  746 |     log_err("Partial read of %u bytes of %lu, reading more\n",
      |                                          ~~^
      |                                            |
      |                                            long unsigned int
      |                                          %u
  747 |      ret, size);
      |           ~~~~                              
      |           |
      |           size_t {aka unsigned int}
/builddir/sof-1.9/tools/logger/convert.c: In function 'logger_read':
/builddir/sof-1.9/tools/logger/convert.c:877:63: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'unsigned int' [-Werror=format=]
  877 |      log_err("file '%s' is unaligned with trace entry size (%ld)\n",
      |                                                             ~~^
      |                                                               |
      |                                                               long int
      |                                                             %d
  878 |       global_config->in_file, sizeof(dma_log));
      |                               ~~~~~~~~~~~~~~~                  
      |                               |
      |                               unsigned int
/builddir/sof-1.9/tools/logger/convert.c: In function 'dump_ldc_info':
/builddir/sof-1.9/tools/logger/convert.c:997:55: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'ssize_t' {aka 'int'} [-Werror=format=]
  997 |  fprintf(out_fd, "Components uuid dictionary size:\t%ld bytes\n",
      |                                                     ~~^
      |                                                       |
      |                                                       long int
      |                                                     %d
  998 |   remaining);
      |   ~~~~~~~~~                                            
      |   |
      |   ssize_t {aka int}
/builddir/sof-1.9/tools/logger/convert.c:1010:26: error: format '%lX' expects argument of type 'long unsigned int', but argument 3 has type 'uintptr_t' {aka 'unsigned int'} [-Werror=format=]
 1010 |   fprintf(out_fd, "\t0x%lX  %s\n", uid_addr, name);
      |                        ~~^         ~~~~~~~~
      |                          |         |
      |                          |         uintptr_t {aka unsigned int}
      |                          long unsigned int
      |                        %X
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

x86_64-musl error

[5/388] Generating sof-cml-nocodec.tplg
ALSA lib parser.c:213:(tplg_load_config) failed to parse topology
Unable to load configuration: Invalid argument
FAILED: topology/topology1/sof-cml-nocodec.tplg /builddir/sof-1.9/tools/build/topology/topology1/sof-cml-nocodec.tplg 
cd /builddir/sof-1.9/tools/build/topology/topology1 && alsatplg ${VERBOSE:+-v 1} -c /builddir/sof-1.9/tools/build/topology/topology1/sof-cml-nocodec.conf -o sof-cml-nocodec.tplg
[7/388] Building C object probes/CMakeFiles/sof-probes.dir/probes_main.c.o
ninja: build stopped: subcommand failed.

[lgirdwood]

@cinerea0 what version of GCC are you using here ?

[Johnnynator]

gcc (GCC) 10.2.1 20201203 (technically a git version somewhere between 10.2 and 10.3), but these errors are unrelated to gcc.

The i686 errors are due to the usage of -Wall -Werrror but not having strict enough code, you could e.g. cast size_t to long unsigned int so it always qualifies as the %lu

[Johnnynator]

The musl issue as reported is most likely not fo this repo, but for alsa-utils/alsa-lib (currently looking into it).

But there is another issue not mentioned. The musl libc asprintf and vasprintf clash with the ones from sof, and they have both different signatures. Is there a reason that the apsrintf implementation is called so even if it implements a differen API than usually seen in libc's (glibc, musl, *BSD)?

[lgirdwood]

@Johnnynator thanks for the details, I think you are probably the only person to try and build for 32 bit ? or musl C library. Fwiw I think all the target platforms are 64bit for all SOF vendors. The long type format specifiers may not be required here and %p could be used for the uid ? Are you able to give this a try and submit a fix ?

[Johnnynator]

Openend alsa-project/alsa-lib#187 for part of the musl issue.

Are you able to give this a try and submit a fix ?

Done

Regarding {v}asprintf, what would be considered a acceptable rename?
Sof already uses quite a few nonstandard functions, so forcing musl into a standard compliant mode will break other parts.

[lgirdwood]

Regarding {v}asprintf, what would be considered a acceptable rename?
Sof already uses quite a few nonstandard functions, so forcing musl into a standard compliant mode will break other parts.

Can you point to the places where we break musl ?

[Johnnynator]

Regarding {v}asprintf, what would be considered a acceptable rename?
Sof already uses quite a few nonstandard functions, so forcing musl into a standard compliant mode will break other parts.

Can you point to the places where we break musl ?

You reimplement asprintf and vsprintf here:

sof/tools/logger/misc.c

Lines 15 to 42 in 158089c

	char *vasprintf(const char *format, va_list args)
	{
	va_list args_copy;
	int size;
	char localbuf[1];
	char *result;
	va_copy(args_copy, args);
	size = vsnprintf(localbuf, 1, format, args_copy);
	va_end(args_copy);
	result = malloc(size + 1);
	if (result)
	vsnprintf(result, size + 1, format, args);
	return result;
	}
	char *asprintf(const char *format, ...)
	{
	va_list args;
	char *result;
	va_start(args, format);
	result = vasprintf(format, args);
	va_end(args);
	return result;
	}

The api differs from what is usually expected on most Unix systems, so the compiler will refuse it on musl, you just got lucky not hitting the problem also on glibc.

[lgirdwood]

I wonder why we do this, it could be to support Windows?
@Johnnynator You are right, we should not be defining if already available.
@juimonen is this something you can fix. Thanks !

[lgirdwood]

@juimonen any update, I did see some related fixes ?

[juimonen]

@lgirdwood @Johnnynator #5045, just trivial rename to avoid clash. I think this was originally done this way because of windows... would this fix your issue?

[Johnnynator]

Yeah, this fixes this specific issue.

The only remaining part is the parsing of topology files. For which there is a different tracking issue: #5039

So I suppose this one can be closed for now.