[Buildroot] [PATCH 1/1] package/mesa3d: fix build on riscv32

[Arnout Vandecappelle]

On 22/08/2021 00:06, Fabrice Fontaine wrote:
> Le sam. 21 août 2021 à 21:08, Thomas Petazzoni
> <thomas.petazzoni at bootlin.com> a écrit :
>>
>> On Sat, 21 Aug 2021 18:05:09 +0200
>> Fabrice Fontaine <fontaine.fabrice at gmail.com> wrote:
>>
>>> Fix the following build failure on riscv32:
>>>
>>> ../src/util/futex.h: In function 'sys_futex':
>>> ../src/util/futex.h:39:19: error: 'SYS_futex' undeclared (first use in this function); did you mean 'sys_futex'?
>>>    39 |    return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);
>>>       |                   ^~~~~~~~~
>>>       |                   sys_futex
>>>
>>> Fixes:
>>>  - http://autobuild.buildroot.org/results/692700a5f967760a0b8cd358b1712f1d5a7b681e
>>>
>>> Signed-off-by: Fabrice Fontaine <fontaine.fabrice at gmail.com>
>>
>>> ++#ifndef SYS_futex
>>> ++#define SYS_futex SYS_futex_time64
>>> ++#endif
>>
>> Are we sure that SYS_futex is exactly equivalent to SYS_futex_time64 ?
>> In another e-mail thread Arnout said they are not exactly equivalent.
> I'll let Arnout answers but the same kind of patch was applied for capnproto:
> https://patchwork.ozlabs.org/project/buildroot/patch/20210527210402.555334-1-fontaine.fabrice@gmail.com

 I can't say I fully understand all of it, but this is my understanding:

 The only difference between SYS_futex and SYS_futex_time64 is that the former
takes a legay struct timespec while the latter takes a 64-bit struct timespec.

 Any system which doesn't have SYS_futex defined (but does have SYS_futex_time64
defined) is necessarily a system that only has 64-bit struct timespec. So unless
the libc is completely broken, it must have a 64-bit struct timespec.

 Therefore, a change like this is OK, as long as no code makes any assumptions
about the layout of the struct timespec that is passed as an argument. That
shouldn't be a problem unless some very weird casting is being done. With time_t
the risk is bigger (casting a pointer-to-long to pointer-to-time_t is pretty
easy to do), but for struct timespec it's not really likely.

 So I'm not particularly opposed to patches like this, as long as it's sent
upstream and upstream doesn't complain too much about it.

 A more official approach could be to rely on __TIMESIZE. If __TIMESIZE is 64
(which it is on riscv32), then struct timespec is 64-bit so we have to use
SYS_futex64 instead of SYS_futex. However, __TIMESIZE comes from glibc, I'm not
sure if uClibc and musl define it. I can't fine a reference to it in their repos.

 Of course, the better solution is to make the whole thing time64 compliant, but
that's generally a bigger change. And not really possible at this point - glibc
documentation itself says that it's not ready yet [1].

 Bottom line: I think this patch is good, as in, it's pretty much the only
feasible approach (other than disabling it for riscv32). Therefore, applied to
master,thanks.

 Regards,
 Arnout

[1]
https://www.gnu.org/software/libc/manual/html_node/64_002dbit-time-symbol-handling.html




> 
> Another option would be to disable mesa3d on riscv32, especially if
> upstream rejects the MR.
>>
>> Thomas
>> --
>> Thomas Petazzoni, co-owner and CEO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
> Best Regards,
> 
> Fabrice
>