baldurk / renderdoc

RenderDoc is a stand-alone graphics debugging tool.
https://renderdoc.org
MIT License
8.6k stars 1.3k forks source link

jpeg-compressor: Reorder stdio.h include location #3369

Open kraj opened 2 days ago

kraj commented 2 days ago

Current, location ends up with compile errors with clang and glibc 2.40 fortified headers

| /mnt/b/yoe/master/build/tmp/work/core2-64-yoe-linux/renderdoc/1.33/recipe-sysroot/usr/include/bits/stdio2.h:128:13: error: use of undeclared identifier 'builtin___vfprintf_chk'; did you mean 'builtin_sprintf_chk'? | 128 | int r = builtin___vfprintf_chk (stream, USE_FORTIFY_LEVEL - 1, | | ^ | /mnt/b/yoe/master/build/tmp/work/core2-64-yoe-linux/renderdoc/1.33/recipe-sysroot/usr/include/bits/stdio2.h:128:39: error: cannot initialize a parameter of type 'char ' with an lvalue of type 'FILE const restrict' (aka 'jpge::_IO_FILE *const restrict') | 128 | int r = builtin___vfprintf_chk (stream, __USE_FORTIFY_LEVEL - 1, | | ^~~~

This re-ordering ensures that fortified function prototypes are used correctly.

Description

zatrazz commented 2 days ago

This is a workaround to what seems to be a clang issue. The next glibc 2.40 will provide better fortify wrappers (_FORTIFY_SOURCE) for clang, and for the variadic argument printf family (sprintf, snprintf, fprintf, printf, dprintf, asprintf, __asprintf, obstack_printf) the glibc expands to a inline function like:

__fortify_function_error_function __attribute_overloadable__ __nonnull ((1)) int
fprintf (__fortify_clang_overload_arg (FILE *, __restrict, __stream),
         const char *__restrict __fmt, ...)
{
  __gnuc_va_list __fortify_ap;
  __builtin_va_start (__fortify_ap, __fmt);
  int __r = __builtin___vfprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1,
                                      __fmt, __fortify_ap);
  __builtin_va_end (__fortify_ap);
  return __r;
}

Different than gcc, clang does not provide a va_arg_pack like builtin so we can't reimplement the wrapper by calling fprintf_chk directly (we might implement with __VA_ARGS__, but it would add a macro for reserved name which not ideal).

The issues seems that clang does not handle __builtin___vfprintf_chk inside a namespace for some reason:

$ cat t.cc
#include <cstdarg>

#ifdef NAMESPACE
namespace bar {
#endif

struct FILE;

int foo (FILE *__stream, const char *__fmt, ...)
{
  __gnuc_va_list __fortify_ap;
  __builtin_va_start (__fortify_ap, __fmt);
  int __r = __builtin___vfprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1,
                                      __fmt, __fortify_ap);
  __builtin_va_end (__fortify_ap);
  return __r;
}

#ifdef NAMESPACE
}
#endif
$ clang++ -v 2>&1 | head -1
clang version 18.1.4 (git@github.com:llvm/llvm-project.git b6ebea7972cd05a8e4dcf0d5a54f2c793999995a)
$ clang++ t.cc -c
$ clang++ t.cc -c -DNAMESPACE
t.cc:13:13: error: use of undeclared identifier '__builtin___vfprintf_chk'; did you mean '__builtin___sprintf_chk'?
   13 |   int __r = __builtin___vfprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1,
      |             ^~~~~~~~~~~~~~~~~~~~~~~~
      |             __builtin___sprintf_chk
t.cc:13:13: note: '__builtin___sprintf_chk' declared here
t.cc:13:39: error: cannot initialize a parameter of type 'char *' with an lvalue of type 'FILE *'
   13 |   int __r = __builtin___vfprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1,
      |                                       ^~~~~~~~
2 errors generated.

Different than gcc that does not show this issue. My understanding is compiler builtins should not be suject by namespace rules, and it is seems to be failing only for __builtin___vfprintf_chk.