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

Wed Jul 20 15:03:13 UTC 2016

On 19 July 2016 at 03:12, Alexander von Gluck IV <kallisti5 at unixzen.com> wrote:
> July 18 2016 1:10 PM, "Emil Velikov" <emil.l.velikov at gmail.com> wrote:
>> 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) ?
>
> After a lot of trial and error, I think I found an acceptable solution.
>
> If we drop the static from debug_printf all of the HAIKU checks go away :-)
>
Hmm this sounds strange. Either way - I'll let others weight in on
their preferred solution:
 - keep things as-is and s/PIPE_OS_HAIKU/__HAIKU__/
 - use "#undef debug_printf", considering it works
 - make the function non-static

> master from git+ssh.  I deleted the repo, re-checked out, and re-applied my changes.
> Issue went away. The repo I was using has been around for a while, maybe Scons left something
> behind.
>
git clean -fXd/fxd should be enough. IIRC Scons does not leave (m)any
artefacts in the source tree, but it would have picked the in-tree
sources if they were generated by say - autoconf and/or manually. That
is without the patch I've CC-ed you on ;-)

-Emil