[Mesa-dev] [PATCH] gallium/util: Fix debug_printf under Haiku

Mon Jul 18 18:10:30 UTC 2016

On 18 July 2016 at 16:28, Alexander von Gluck IV <kallisti5 at unixzen.com> wrote:
> July 18 2016 9:20 AM, "Emil Velikov" <emil.l.velikov at gmail.com> wrote:
>> On 18 July 2016 at 14:39, Alexander von Gluck IV <kallisti5 at unixzen.com> wrote:
>>
>>> July 18 2016 3:29 AM, "Nicolai Hähnle" <nhaehnle at gmail.com> wrote:
>>>> A comment further up in the same file says
>>>>
>>>> /* Haiku provides debug_printf in libroot with OS.h */
>>>>
>>>> Is that no longer true?
>>>>
>>>> Nicolai
>>>>
>>>> On 16.07.2016 16:27, Alexander von Gluck IV wrote:
>>>>
>>>>> ---
>>>>> src/gallium/auxiliary/util/u_debug.h | 5 ++++-
>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
>>>>> index 7da7f53..7dc4ce8 100644
>>>>> --- a/src/gallium/auxiliary/util/u_debug.h
>>>>> +++ b/src/gallium/auxiliary/util/u_debug.h
>>>>> @@ -83,7 +83,10 @@ _debug_printf(const char *format, ...)
>>>>> * - avoid outputing large strings (512 bytes is the current maximum length
>>>>> * that is guaranteed to be printed in all platforms)
>>>>> */
>>>>> -#if !defined(PIPE_OS_HAIKU)
>>>>> +#if defined(PIPE_OS_HAIKU)
>>>>> +void
>>>>> +debug_printf(const char *format, ...) _util_printf_format(1,2);
>>>>> +#else
>>>>> static inline void
>>>>> debug_printf(const char *format, ...) _util_printf_format(1,2);
>>
>> Hmm I moved the include further up with commit
>> 373f118c6c750d717fd0727fc3fc191828714c6f although that should not have
>> made any difference, barring fragile include file order. Can you check
>> if reverting the u_debug.h gets you up and running ? If so can you
>> please:
>> - Please add the stable tag Cc: <mesa-stable at lists.freedesktop.org>
>> - Attempt to straighten the includes (it might be mesa, llvm and/or
>> Haiku that is getting confused)
>>
>>> It's still true, however without the _util_printf_format I get odd llvm
>>> symbol errors.
>>
>> I would suspect that the above is in play, but without details
>> (build/error log) little to no one will be able to tell you if this is
>> the correct fix, I'm afraid.
>
> gcc 5.4.0 / llvm 3.8.0
> Sorry, I wasn't near the machine, here is the error without any changes:
>
> src/gallium/auxiliary/gallivm/lp_bld_assert.c: In function 'lp_assert':
> src/gallium/auxiliary/gallivm/lp_bld_assert.c:43:7: warning: implicit declaration of function 'debug_printf' [-Wimplicit-function-declaration]
>        debug_printf("LLVM assertion '%s' failed!\n", msg);
>        ^
Ok, this happens as PIPE_OS_HAIKU isn't defined that early in
u_debug.h, thus the header is not included....

>   Compiling src/gallium/auxiliary/gallivm/lp_bld_const.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_conv.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_debug.cpp ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_flow.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_aos_array.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_aos.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_cached.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_float.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_soa.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_srgb.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_format_yuv.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_gather.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_init.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_intr.c ...
> src/gallium/auxiliary/gallivm/lp_bld_intr.c: In function 'lp_build_intrinsic_binary_anylength':
> src/gallium/auxiliary/gallivm/lp_bld_intr.c:252:10: warning: implicit declaration of function 'debug_printf' [-Wimplicit-function-declaration]
>           debug_printf("%s: should handle arbitrary vector size\n",
>           ^
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_logic.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_misc.cpp ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_pack.c ...
>   Compiling src/gallium/auxiliary/gallivm/lp_bld_printf.c ...
> src/gallium/auxiliary/gallivm/lp_bld_printf.c: In function 'lp_build_print_args':
> src/gallium/auxiliary/gallivm/lp_bld_printf.c:68:84: error: 'debug_printf' undeclared (first use in this function)
>     func_printf = lp_build_const_int_pointer(gallivm, func_to_pointer((func_pointer)debug_printf));
>                                                                                     ^
> src/gallium/auxiliary/gallivm/lp_bld_printf.c:68:84: note: each undeclared identifier is reported only once for each function it appears in
> scons: *** [build/haiku-x86_64-debug/gallium/auxiliary/gallivm/lp_bld_printf.os] Error 1
>
> debug_printf is definitely declared however (and it should be all c code, no C++ thus no mangling)
>
>>> The linux code just below defines debug_printf twice as well:
>>>
>>> https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/auxiliary/util/u_debug.h#n87
>>
>> AFAICT that is due to the _util_printf_format attribute, which
>> older/some compilers were not have been happy with [being alongside
>> the function definition].
>>
>>> I'm honestly not 100% sure what's going on here as the arguments are the same,
>>> but defining twice on Haiku (once in OS.h, and again here) seems to resolve the issue.
>>>
>>> I have to define it differently under Haiku (and can't use the same one for all platforms)
>>> because Haiku's debug_printf is not static inline.
>>
>> Would replacing the Haiku one with our static inline work ? Would be
>> nice if things don't diverge (too much) across platforms.
>
> I'm all for replacing the OS's debug_printf, but that symbol is in the libroot which must be
> linked into every binary... so I'm a bit worried about overriding it causing issues.
> This seems to compile and might be a better solution?
>
The runtime should pick the local symbol afaict. In a similar way that
one can have local function called "dlsym" on their linux/posix setup.

> diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
> index 7da7f53..a3f046b 100644
> --- a/src/gallium/auxiliary/util/u_debug.h
> +++ b/src/gallium/auxiliary/util/u_debug.h
> @@ -39,11 +39,6 @@
>  #define U_DEBUG_H_
>
>
> -#if defined(PIPE_OS_HAIKU)
> -/* Haiku provides debug_printf in libroot with OS.h */
> -#include <OS.h>
> -#endif
> -
>  #include "os/os_misc.h"
>
>  #include "pipe/p_format.h"
> @@ -54,6 +49,10 @@
>  extern "C" {
>  #endif
>
> +// Some OS's like Haiku define debug_printf
> +#ifdef debug_printf
> +#undef debug_printf
> +#endif
>
Is debug_printf a macro, can we check for__HAIKU__ || PIPE_OS_HAIKU
(the latter is optional imho) ?

>  #if defined(__GNUC__)
>  #define _util_printf_format(fmt, list) __attribute__ ((format (printf, fmt, list)))
> @@ -83,7 +82,6 @@ _debug_printf(const char *format, ...)
>   * - avoid outputing large strings (512 bytes is the current maximum length
>   * that is guaranteed to be printed in all platforms)
>   */
> -#if !defined(PIPE_OS_HAIKU)
>  static inline void
>  debug_printf(const char *format, ...) _util_printf_format(1,2);
>
> @@ -99,7 +97,6 @@ debug_printf(const char *format, ...)
>     (void) format; /* silence warning */
>  #endif
>  }
> -#endif
>
>
>  /*
>
>
>
> I haven't been able to test it though due to this strange issue that popped up:
>
>   Compiling src/mesa/main/scissor.c ...
> In file included from src/mesa/main/remap.c:46:0:
> src/mesa/main/remap_helper.h:7675:39: error: array type has incomplete element type 'struct gl_function_remap'
>  static const struct gl_function_remap MESA_alt_functions[] = {
>                                        ^
Interesting, is this a tarball or a specific git branch ? Can you
double-check if the tarball directory/git tree is clean.

First thing that comes to mind is old generated file getting picked
up, while working with master branch (that one has the struct nuked by
yours truly ;-)

-Emil