[Mesa-dev] [PATCH] gallium/util: Fix debug_printf under Haiku
Alexander von Gluck IV
kallisti5 at unixzen.com
Tue Jul 19 02:12:49 UTC 2016
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 :-)
diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
index 7da7f53..2170cef 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,7 +49,6 @@
extern "C" {
#endif
-
#if defined(__GNUC__)
#define _util_printf_format(fmt, list) __attribute__ ((format (printf, fmt, list)))
#else
@@ -62,7 +56,7 @@ extern "C" {
#endif
void _debug_vprintf(const char *format, va_list ap);
-
+
static inline void
_debug_printf(const char *format, ...)
@@ -83,11 +77,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)
-static inline void
+inline void
debug_printf(const char *format, ...) _util_printf_format(1,2);
-static inline void
+inline void
debug_printf(const char *format, ...)
{
#ifdef DEBUG
@@ -99,7 +92,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:
>> 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 ;-)
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.
Thanks!
-- Alex
More information about the mesa-dev
mailing list