Bug 66346 - shader_query.cpp:49: error: invalid conversion from 'void*' to 'GLuint'
Summary: shader_query.cpp:49: error: invalid conversion from 'void*' to 'GLuint'
Status: REOPENED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Mac OS X (All)
: medium blocker
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks:
 
Reported: 2013-06-29 00:40 UTC by Vinson Lee
Modified: 2016-07-09 00:10 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
use a function to convert from GLhandleARB to GLuint (2.15 KB, patch)
2013-07-01 14:56 UTC, Brian Paul
Details | Splinter Review
use a function to convert from GLhandleARB to GLuint (2.37 KB, patch)
2015-04-22 22:25 UTC, Julien Isorce
Details | Splinter Review
shader_query: convert GLhandleARB to uintptr_t before troncating to GLuint (1.71 KB, patch)
2015-04-22 22:27 UTC, Julien Isorce
Details | Splinter Review
darwin: Suppress type conversion warnings for GLhandleARB (3.42 KB, patch)
2015-05-05 21:52 UTC, Julien Isorce
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vinson Lee 2013-06-29 00:40:42 UTC
mesa: 5a925cc5504575c22dbb7d29842d7fc5babcb5c7 (master)

$ make
[...]
  CXX      shader_query.lo
../../src/mesa/main/shader_query.cpp: In function 'void _mesa_BindAttribLocation(void*, GLuint, const GLcharARB*)':
../../src/mesa/main/shader_query.cpp:49: error: invalid conversion from 'void*' to 'GLuint'
../../src/mesa/main/shader_query.cpp:49: error:   initializing argument 2 of 'gl_shader_program* _mesa_lookup_shader_program_err$../../src/mesa/main/shader_query.cpp: In function 'void _mesa_GetActiveAttrib(void*, GLuint, GLsizei, GLsizei*, GLint*, GLenum*,$../../src/mesa/main/shader_query.cpp:87: error: invalid conversion from 'void*' to 'GLuint'
../../src/mesa/main/shader_query.cpp:87: error:   initializing argument 2 of 'gl_shader_program* _mesa_lookup_shader_program_err$../../src/mesa/main/shader_query.cpp: In function 'GLint _mesa_GetAttribLocation(void*, const GLcharARB*)':
../../src/mesa/main/shader_query.cpp:139: error: invalid conversion from 'void*' to 'GLuint'
../../src/mesa/main/shader_query.cpp:139: error:   initializing argument 2 of 'gl_shader_program* _mesa_lookup_shader_program_er$make[4]: *** [shader_query.lo] Error 1

$ gcc --version
i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

include/GL/glext.h
  3468  #ifndef GL_ARB_shader_objects
  3469  #define GL_ARB_shader_objects 1
  3470  #ifdef __APPLE__
  3471  typedef void *GLhandleARB;
  3472  #else
  3473  typedef unsigned int GLhandleARB;
  3474  #endif
Comment 1 Brian Paul 2013-07-01 14:56:49 UTC
Created attachment 81814 [details] [review]
use a function to convert from GLhandleARB to GLuint

Vinson, can you try this patch?  There may be other files to fix similarly...
Comment 2 Vinson Lee 2013-07-02 05:15:05 UTC
(In reply to comment #1)
> Created attachment 81814 [details] [review] [review]
> use a function to convert from GLhandleARB to GLuint
> 
> Vinson, can you try this patch?  There may be other files to fix similarly...

  Compiling src/mesa/main/uniform_query.cpp ...
src/mesa/main/uniform_query.cpp: In function ‘void _mesa_GetActiveUniform(void*, GLuint, GLsizei, GLsizei*, GLint*, GLenum*, GLcharARB*)’:
src/mesa/main/uniform_query.cpp:49: error: invalid conversion from ‘void*’ to ‘GLuint’
src/mesa/main/uniform_query.cpp:49: error:   initializing argument 2 of ‘gl_shader_program* _mesa_lookup_shader_program_err(gl_context*, GLuint, const char*)’
Comment 3 Vinson Lee 2014-03-07 06:43:56 UTC
mesa: 378c6f2246d66254ce0f88cac6daf25b1c012a41 (master 10.2.0-devel)

Mac OS X build is still broken.
Comment 4 Ian Romanick 2014-03-07 11:26:55 UTC
This is in glext.h:

#ifdef __APPLE__
typedef void *GLhandleARB;
#else
typedef unsigned int GLhandleARB;
#endif

Thanks, Apple!

I think Mesa is just broken on Apple, probably forever, because of this.  We assume *everywhere* that GLhandleARB == GLuint... to the point that glGetActiveAttrib and glGetActiveAttribARB (and many, many others) are the same function.

This occurs in shader_query.cpp, uniforms.c, uniform_query.cpp, shaderapi.c, and dlist.c.

I'm honestly inclined to close this as WONTFIX...
Comment 5 Brian Paul 2014-03-07 15:20:37 UTC
Looks like there's about 20 functions which would have be split into ARB (GLhandleARB) and non-ARB (GLuint) flavors.

I might take a crack at it someday...

I'd like to see Mesa compile on MacOS even if it's not a common thing.
Comment 6 Eric Anholt 2014-03-07 18:48:37 UTC
I ran into this with epoxy, and in the process of trying to come up with a fix noted that all the GLhandleARBs are in the first 6 args of the functions, so they'll be stored in registers in amd64's calling convention.  That let me actually still treat them as aliases.

Not sure if that will be useful to solving mesa problems, but I thought it was an interesting note.
Comment 7 Jose Fonseca 2014-03-07 20:38:16 UTC
(In reply to comment #4)
> This is in glext.h:
> 
> #ifdef __APPLE__
> typedef void *GLhandleARB;
> #else
> typedef unsigned int GLhandleARB;
> #endif
> 
> Thanks, Apple!
> 
> I think Mesa is just broken on Apple, probably forever, because of this.  We
> assume *everywhere* that GLhandleARB == GLuint... to the point that
> glGetActiveAttrib and glGetActiveAttribARB (and many, many others) are the
> same function.

Although Apple's GLhandleARB are void *, I recall reading that only 32bits are used. (Unfortunately Apple can't go back in time and replace GLhandleARB with uints, as that would break binary compatibility.)

Furthermore what Apple does is irrelevant, as Mesa is free to do what it wants (ie, it's free to assume using GLhandleARB are always 32bits).

In short, all we have to do we fix the compiler errors, either:

1) typedefing GLhandleARB as uintptr_t internally when

  #ifdef __APPLE__
  #  ifdef BUILDING_MESA
  typedef uintptr_t GLhandleARB;
  #  else
  typedef void *GLhandleARB;
  #  endif
  #else
  typedef unsigned int GLhandleARB;
  #endif

2) add intermediate casts to (uintptr_t) between uint<->GLhandleARB conversions.

> This occurs in shader_query.cpp, uniforms.c, uniform_query.cpp, shaderapi.c,
> and dlist.c.
> 
> I'm honestly inclined to close this as WONTFIX...

Jeez, it seems nobody has patient for non-Linux platform these days.  Yes, cross-platform its often a pain, but not really the end of the world.


(In reply to comment #5)
> Looks like there's about 20 functions which would have be split into ARB
> (GLhandleARB) and non-ARB (GLuint) flavors.
> 
> I might take a crack at it someday...
> 
> I'd like to see Mesa compile on MacOS even if it's not a common thing.

Mesa and MacOS in general no, but compatibility with Gallium and MacOS is important for VMware down the road.
Comment 8 Jose Fonseca 2014-03-07 20:52:51 UTC
(In reply to comment #7)
> Furthermore what Apple does is irrelevant, as Mesa is free to do what it
> wants (ie, it's free to assume using GLhandleARB are always 32bits).

To be perfectly clear, Mesa is free to assume GLhandleARB are always 32bits integer *values*.  But sizeof(GLhandleARB) must match sizeof(void *) to avoid breaking binary compatibility of 64 bits MacOSX apps.
Comment 9 Jose Fonseca 2014-03-07 22:45:00 UTC
(In reply to comment #4)
> We
> assume *everywhere* that GLhandleARB == GLuint... to the point that
> glGetActiveAttrib and glGetActiveAttribARB (and many, many others) are the
> same function.

(In reply to comment #5)
> Looks like there's about 20 functions which would have be split into ARB
> (GLhandleARB) and non-ARB (GLuint) flavors.

Er.. Yes, I glossed over the aliasing issue.  So a proper fix is not as simple as I painted earlier.

It would however fix the build -- and leave it no less no more broken than before.
Comment 10 Brian Paul 2014-03-07 22:46:26 UTC
(In reply to comment #9)
> (In reply to comment #4)
> > We
> > assume *everywhere* that GLhandleARB == GLuint... to the point that
> > glGetActiveAttrib and glGetActiveAttribARB (and many, many others) are the
> > same function.
> 
> (In reply to comment #5)
> > Looks like there's about 20 functions which would have be split into ARB
> > (GLhandleARB) and non-ARB (GLuint) flavors.
> 
> Er.. Yes, I glossed over the aliasing issue.  So a proper fix is not as
> simple as I painted earlier.
> 
> It would however fix the build -- and leave it no less no more broken than
> before.

I've started a patch series to handle GLuint vs. GLhandleARB properly. I'll try to wrap it up tomorrow.
Comment 11 Jeremy Huddleston Sequoia 2014-05-25 09:30:59 UTC
(In reply to comment #7)
> > Thanks, Apple!

You should thank the lack of standards regarding what GLhandleARB should've been defined as back in 2005.  It's no more Apple's fault than it is Mesa's.

Furthermore, we had been dealing with this in src/glx/apple without major issues through 8.0.5 (which was the last version before src/glx/apple stopped building in mesa due to the transition to autotools-only builds).  GLhandleARB was always 'void *' in mesa on OS X.

> > I think Mesa is just broken on Apple, probably forever, because of this.  We
> > assume *everywhere* that GLhandleARB == GLuint... 

Well, that's certainly incorrect and worthy of being fixed.  You can't make that assumption and should fix any code which does.

> > to the point that
> > glGetActiveAttrib and glGetActiveAttribARB (and many, many others) are the
> > same function.

An obvious bug that should be straight forward to address.
 
> Although Apple's GLhandleARB are void *, I recall reading that only 32bits
> are used.

I don't think you should not assume anything about the implementation.

> (Unfortunately Apple can't go back in time and replace GLhandleARB
> with uints, as that would break binary compatibility.)

Yes, but fixing this doesn't really have any binary compatibility impact in mesa since GLhandleARB has always been void * on darwin anyways.  Most of this is about fixing build failures and fixing correctness for other architectures to make mesa more robust in genera.

> Furthermore what Apple does is irrelevant, as Mesa is free to do what it
> wants (ie, it's free to assume using GLhandleARB are always 32bits).

Except for the fact that all the OpenGL functions in OpenGL.framework need to match the signature of the corresponding functions in mesa, since they're passed straight through.  That's why GLhandleARB has always been void * in mesa on darwin.

> In short, all we have to do we fix the compiler errors, either:
> 
> 1) typedefing GLhandleARB as uintptr_t internally when
> 
>   #ifdef __APPLE__
>   #  ifdef BUILDING_MESA
>   typedef uintptr_t GLhandleARB;
>   #  else
>   typedef void *GLhandleARB;
>   #  endif
>   #else
>   typedef unsigned int GLhandleARB;
>   #endif

Ugly, and this just continues to mask underlying bugs.

> 2) add intermediate casts to (uintptr_t) between uint<->GLhandleARB
> conversions.

Ugly and also just masks underlying bugs =/

That being said, #2 is possibly the safest option because we are at least forced to look at every problem point in the API.
Comment 12 Vinson Lee 2014-09-13 07:53:34 UTC
Regression introduced with commit 9a14e412d6de93349a490a9c4534b52c3b524ee9 during mesa 9.1.

commit 9a14e412d6de93349a490a9c4534b52c3b524ee9
Author: Brian Paul <brianp@vmware.com>
Date:   Tue Jun 25 10:35:37 2013 -0600

    mesa: update glext.h to version 20130624
    
    In glapi_priv.h we always need the typedef for the GLclampx type
    since GL_OES_fixed_point is now defined in glext.h but the
    GLclampx type is not.  GLclampx is not used by anything in glext.h
    but we need it for GL ES dispatch.
    
    This is a huge patch because the structure of the file has been
    changed.
    
    The following extensions are new, however:
    
    GL_AMD_interleaved_elements
    GL_AMD_shader_trinary_minmax
    GL_IBM_static_data
    GL_INTEL_map_texture
    GL_NV_compute_program5
    GL_NV_deep_texture3D
    GL_NV_draw_texture
    GL_NV_shader_atomic_counters
    GL_NV_shader_storage_buffer_object
    GL_NVX_conditional_render
    GL_OES_byte_coordinates
    GL_OES_compressed_paletted_texture
    GL_OES_fixed_point
    GL_OES_query_matrix
    GL_OES_single_precision
    
    And these extensions were removed:
    
    GL_FfdMaskSGIX
    GL_INGR_palette_buffer
    GL_INTEL_texture_scissor
    GL_SGI_depth_pass_instrument
    GL_SGIX_fog_scale
    GL_SGIX_impact_pixel_texture
    GL_SGIX_texture_select
    
    Reviewed-by: José Fonseca <jfonseca@vmware.com>
Comment 13 Julien Isorce 2015-04-22 22:23:34 UTC
Problem still exists:

CXX      main/shader_query.lo
main/shader_query.cpp:71:7: error: no matching function for call to '_mesa_lookup_shader_program_err'
      _mesa_lookup_shader_program_err(ctx, program, "glBindAttribLocation");
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/mesa/main/shaderobj.h:89:1: note: candidate function not viable: cannot convert argument of incomplete type 'GLhandleARB' (aka 'void *') to
      'GLuint' (aka 'unsigned int')
_mesa_lookup_shader_program_err(struct gl_context *ctx, GLuint name,
^
main/shader_query.cpp:139:13: error: no matching function for call to '_mesa_lookup_shader_program_err'
   shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib");
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/mesa/main/shaderobj.h:89:1: note: candidate function not viable: cannot convert argument of incomplete type 'GLhandleARB' (aka 'void *') to
      'GLuint' (aka 'unsigned int')
_mesa_lookup_shader_program_err(struct gl_context *ctx, GLuint name,
^
main/shader_query.cpp:253:7: error: no matching function for call to '_mesa_lookup_shader_program_err'
      _mesa_lookup_shader_program_err(ctx, program, "glGetAttribLocation");
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/mesa/main/shaderobj.h:89:1: note: candidate function not viable: cannot convert argument of incomplete type 'GLhandleARB' (aka 'void *') to
      'GLuint' (aka 'unsigned int')
_mesa_lookup_shader_program_err(struct gl_context *ctx, GLuint name,
^
3 errors generated.

----

gcc --version
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.1.0 (clang-602.0.49) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.3.0
Thread model: posix

---

OSX 10.10.3 / Dev Command Line Tools v6.3
Comment 14 Julien Isorce 2015-04-22 22:25:51 UTC
Created attachment 115279 [details] [review]
use a function to convert from GLhandleARB to GLuint

Brian's patch rebased. Works for me.
Comment 15 Julien Isorce 2015-04-22 22:27:11 UTC
Created attachment 115280 [details] [review]
shader_query: convert GLhandleARB to uintptr_t before troncating to GLuint

Other suggestion.
Comment 16 Julien Isorce 2015-05-05 21:52:26 UTC
Created attachment 115566 [details] [review]
darwin: Suppress type conversion warnings for GLhandleARB

Patch from macports: https://trac.macports.org/browser/trunk/dports/x11/mesa/files/5002-darwin-Suppress-type-conversion-warnings-for-GLhandl.patch
Comment 17 Emil Velikov 2015-07-09 11:14:06 UTC
A slightly different fix has landed as below. Perhaps one day we'll either get this upstreamed with Khronos or have our build apply/revert it on demand.
Not a huge deal either way :-)


 #ifdef __APPLE__
+#ifdef BUILDING_MESA
+/* Avoid uint <-> void* warnings */
+typedef unsigned long GLhandleARB;
+#else
 typedef void *GLhandleARB;
+#endif
Comment 18 Vinson Lee 2015-07-09 21:47:25 UTC
mesa: 0166b4c165271bd7525a91049e58e390cb596c60 (master 10.7.0-devel)

Still see this build error. BUILDING_MESA is only defined for darwin DRI enabled builds.
Comment 19 Jose Fonseca 2015-07-09 22:41:07 UTC
(In reply to Vinson Lee from comment #18)
> mesa: 0166b4c165271bd7525a91049e58e390cb596c60 (master 10.7.0-devel)
> 
> Still see this build error. BUILDING_MESA is only defined for darwin DRI
> enabled builds.

It should be a matter of adding the define on SCons builds too.

But I don't know if there's much point for building Mesa w/ SCons on Mac -- we don't actually build an usable libGL there.
Comment 20 Vinson Lee 2015-07-09 23:13:27 UTC
(In reply to José Fonseca from comment #19)
> (In reply to Vinson Lee from comment #18)
> > mesa: 0166b4c165271bd7525a91049e58e390cb596c60 (master 10.7.0-devel)
> > 
> > Still see this build error. BUILDING_MESA is only defined for darwin DRI
> > enabled builds.
> 
> It should be a matter of adding the define on SCons builds too.
> 
> But I don't know if there's much point for building Mesa w/ SCons on Mac --
> we don't actually build an usable libGL there.

BUILDING_MESA is also not defined on automake dri-disabled build.
Comment 21 Emil Velikov 2015-07-10 17:48:33 UTC
(In reply to Vinson Lee from comment #20)
> (In reply to José Fonseca from comment #19)
> > (In reply to Vinson Lee from comment #18)
> > > mesa: 0166b4c165271bd7525a91049e58e390cb596c60 (master 10.7.0-devel)
> > > 
> > > Still see this build error. BUILDING_MESA is only defined for darwin DRI
> > > enabled builds.
> > 
> > It should be a matter of adding the define on SCons builds too.
> > 
> > But I don't know if there's much point for building Mesa w/ SCons on Mac --
> > we don't actually build an usable libGL there.
> 
> BUILDING_MESA is also not defined on automake dri-disabled build.
It took me a second read to understand what you meant here. Esp. since the code just does not work, with indirect rendering under MacOS/Darwin.

Please post the complete configure line that you're using. Thanks.


bug/show.html.tmpl processed on Feb 19, 2017 at 11:49:06.
(provided by the Example extension).