Bug 106465 - No test for Image Load/Store on format-incompatible texture buffer
Summary: No test for Image Load/Store on format-incompatible texture buffer
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-10 18:37 UTC by Nanley Chery
Modified: 2018-10-09 21:06 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Nanley Chery 2018-05-10 18:37:02 UTC
From the ARB_shader_image_load_store spec:

   If the individual texel identified for an image load, store, or atomic
   operation doesn't exist, the access is treated as invalid.  Invalid image
   loads will return zero.  Invalid image stores will have no effect.
   Invalid image atomics will not update any texture bound to the image unit
   and will return zero.  An access is considered invalid if:

     [...]

     * the internal format of the texture bound to the image unit is
       incompatible with the specified <format> according to Table X.3;

Mesa does not validate the format of texture buffers to ensure this behavior. This bug is fixed with this patch:
https://patchwork.freedesktop.org/patch/211340/

There currently isn't a test in the CTS or in piglit which catches this error. One should be written to prevent this issue from appearing in the future.
Comment 1 Francisco Jerez 2018-05-10 20:34:01 UTC
It should be trivial to extend the arb_shader_image_load_store-invalid piglit test (which already checks a similar condition for other texture targets) to catch this problem.
Comment 2 Danylo 2018-07-17 15:39:14 UTC
Sent the test catch the problem - https://patchwork.freedesktop.org/patch/239424/
Comment 3 Andrés Gómez García 2018-09-07 17:22:39 UTC
Curro, is this fixed by?

commit 5a6814780322988a7adee525899bca8a83907ab7
Author: Francisco Jerez <currojerez@riseup.net>
Date:   Fri Mar 16 13:43:27 2018 -0700

    Revert "mesa: simplify _mesa_is_image_unit_valid for buffers"
    
    This reverts commit c0ed52f6146c7e24e1275451773bd47c1eda3145.  It was
    preventing the image format validation from being done on buffer
    textures, which is required to ensure that the application doesn't
    attempt to bind a buffer texture with an internal format incompatible
    with the image unit format (e.g. of different texel size), which is
    not allowed by the spec (it's not allowed for *any* texture target,
    whether or not there is spec wording restricting this behavior
    specifically for buffer textures) and will cause the driver to
    calculate texel bounds incorrectly and potentially crash instead of
    the expected behavior.
    
    Cc: mesa-stable@lists.freedesktop.org
    Reviewed-by: Marek Olšák <marek.olsak@amd.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465
    Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Comment 4 Andrés Gómez García 2018-09-07 18:05:42 UTC
(In reply to Andrés Gómez García from comment #3)
> Curro, is this fixed by?

[...]

My bad, I just realized that the report is about a missing piglit test.

In that case, shouldn't we move this report to the "piglit" product?
Comment 5 Nanley Chery 2018-10-09 21:03:15 UTC
(In reply to Andrés Gómez García from comment #4)
> (In reply to Andrés Gómez García from comment #3)
> > Curro, is this fixed by?
> 
> [...]
> 
> My bad, I just realized that the report is about a missing piglit test.
> 
> In that case, shouldn't we move this report to the "piglit" product?

I'm not sure. I could see an argument for either product. 

Sometimes when fixing bugs in Mesa, we'd open reports against Mesa and require a fix and (sometimes) a test to close the issue. In this case the fix was already created when the bug was filed, so I just filed the issue about the missing test.
Comment 6 Nanley Chery 2018-10-09 21:06:29 UTC
This issue has been fixed by the following commit:

commit b7f493e3a60baddfcf8003a326a31f6b8be2d4ca
Author: Danylo Piliaiev <danylo.piliaiev@gmail.com>
Date:   Mon Jul 23 15:13:34 2018 +0300

    arb_shader_image_load_store: Test format incompatible texture buffer
    
    Test for the regression which happened when GL_TEXTURE_BUFFER was
    allowed to have incompatible format.
    
    v2: Removed unnecessary code duplication - use upload_image instead
         of init_level. (Francisco Jerez)
    v3: Removed upload_image call because image is already called
         by init_image. (Francisco Jerez)
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465
    
    Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
    Reviewed-by: Francisco Jerez <currojerez@riseup.net>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.