After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723530 - cogl-pipeline: SEGV in _cogl_pipeline_fragend_glsl_end
cogl-pipeline: SEGV in _cogl_pipeline_fragend_glsl_end
Status: RESOLVED FIXED
Product: cogl
Classification: Platform
Component: CoglPipeline
git master
Other Mac OS
: Normal normal
: ---
Assigned To: Cogl maintainer(s)
Cogl maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-02-03 13:28 UTC by cee1
Modified: 2014-02-20 13:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't dereference an unitialised pointer in _cogl_container_of (10.81 KB, patch)
2014-02-03 23:30 UTC, Neil Roberts
accepted-commit_now Details | Review

Description cee1 2014-02-03 13:28:44 UTC
https://git.gnome.org/browse/cogl/tree/cogl/driver/gl/cogl-pipeline-fragend-glsl.c#n1011

Should initialize layer_data to NULL.

It may not cause SEGV depends on compilers, but the compiler does give a warning about "referencing before initializing".
Comment 1 Neil Roberts 2014-02-03 23:30:51 UTC
Created attachment 268022 [details] [review]
Don't dereference an unitialised pointer in _cogl_container_of

The previous implementation was dereferencing the sample pointer in
order to get the offset to subtract from the member pointer. The
resulting value is then only used to get a pointer to the member in
order to calculate the offset so it doesn't actually read from the
memory location and shouldn't cause any problems. However this is
probably technically invalid and could have undefined behaviour. It
looks like clang takes advantage of this undefined behaviour and
doesn't actually offset the pointer. It also generates a warning when
it does this.

This patch splits the _cogl_container_of macro into two
implementations. Previously the macro was always used in the list
iterator macros like this:

SomeType *sample = _cogl_container_of(list_node, sample, link)

Instead of doing that there is now a new macro called
_cogl_list_set_iterator which explicitly assigns to the sample pointer
with an initial value before assigning to it again with the real
offset. This redundant initialisation gets optimised out by compiler.

The second macro is still called _cogl_container_of but instead of
taking a sample pointer it just directly takes the type name. That way
it can use the standard offsetof macro.
Comment 2 Robert Bragg 2014-02-20 03:56:46 UTC
Comment on attachment 268022 [details] [review]
Don't dereference an unitialised pointer in _cogl_container_of

Thanks for fixing this, the patch looks good to land to me. Sorry for the delay in replying.

Reviewed-by: Robert Bragg <robert@linux.intel.com>
Comment 3 Neil Roberts 2014-02-20 13:49:06 UTC
Thanks for the review. I've pushed it to master and the 1.16 and 1.18 branches.