[Mesa-dev] [PATCH 03/39] llvmpipe: Fix overflow for 32 bits available memory computation

[Emil Velikov]

On 15 May 2016 at 11:45, Axel Davy <axel.davy at ens.fr> wrote:
> And cap to 2 GB on 32 bits.
>
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94561
>
Nitpick: please use "Bugzilla: htt.."

> Signed-off-by: Axel Davy <axel.davy at ens.fr>
> ---
>  src/gallium/auxiliary/os/os_misc.c       | 2 +-
>  src/gallium/drivers/llvmpipe/lp_screen.c | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/os/os_misc.c b/src/gallium/auxiliary/os/os_misc.c
> index d6b83e9..cf2ef95 100644
> --- a/src/gallium/auxiliary/os/os_misc.c
> +++ b/src/gallium/auxiliary/os/os_misc.c
> @@ -117,7 +117,7 @@ os_get_total_physical_memory(uint64_t *size)
>     const long phys_pages = sysconf(_SC_PHYS_PAGES);
>     const long page_size = sysconf(_SC_PAGE_SIZE);
>
> -   *size = phys_pages * page_size;
> +   *size = (int64_t)phys_pages * (int64_t)page_size;
>     return (phys_pages > 0 && page_size > 0);
Can you please split this out roughly like the following

if (phys_pages =< 0 || page_size =< 0)
  return false;

*size = (unt64_t)phys_pages * (unt64_t)page_size;
return true;

I might have butchered it while copying from i915/i965.

>  #elif defined(PIPE_OS_APPLE) || defined(PIPE_OS_BSD)
>     size_t len = sizeof(*size);
> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c b/src/gallium/drivers/llvmpipe/lp_screen.c
> index 4f61de8..2f84b75 100644
> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
> @@ -279,6 +279,11 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
>        if (!os_get_total_physical_memory(&system_memory))
>           return 0;
>
> +#ifdef PIPE_ARCH_X86
> +      /* cap to 2 GB on 32 bits system */
> +      system_memory = MIN2(system_memory, 2048);
> +#endif
> +
The 2GiB feels somewhat arbitrary, can we add a brief comment about it
? Then there's the question if other drivers want to apply, and thus
move, it elsewhere.

-Emil