Bug 106590 - Wrong line numbers expanded while compiling shaders
Summary: Wrong line numbers expanded while compiling shaders
Status: NEW
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: 17.1
Hardware: ARM Linux (All)
: medium normal
Assignee: mesa-dev
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-21 06:05 UTC by Zhaowei Yuan
Modified: 2018-08-21 05:45 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
patch that fix GLSL compiling issue (9.14 KB, application/mbox)
2018-05-21 06:05 UTC, Zhaowei Yuan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zhaowei Yuan 2018-05-21 06:05:31 UTC
Created attachment 139657 [details]
patch that fix GLSL compiling issue

Following shader code is got from a CTS test case which is intend to check if the shader compiler can expand macro with right line number:
1         #version 300 es
2         precision mediump float;
3         #define BBB      __LINE__, /*
4            */ __LINE__
5         #define AAA(a,b) BBB, a, b
6
7         void main()
8         {
9            out0 = vec4(AAA(__LINE__,
10                  __LINE__
11                  ));
12         }

It expects to make out0 filled with (11, 11, 9, 10) values, But mesa gives (3,4, 9, 10).

There are 2 problems:
1. Macro BBB should be expanded as the line number exactly where it is referenced, not declared
2. Macro BBB have a character '\n' within "/**/" which should be ignored and treat 2 "__LINE__" as the in the same line

I fixed this problem with the attached patch, please review, thanks
Comment 1 Timothy Arceri 2018-05-21 06:15:46 UTC
Thanks for the patch! However you should sign up to the mesa mailing list [1] and send it there for review.

[1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev

See also: https://www.mesa3d.org/submittingpatches.html
Comment 2 Zhaowei Yuan 2018-06-04 07:52:00 UTC
(In reply to Timothy Arceri from comment #1)
> Thanks for the patch! However you should sign up to the mesa mailing list
> [1] and send it there for review.
> 
> [1] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> See also: https://www.mesa3d.org/submittingpatches.html

I have posted a patch here: https://patchwork.freedesktop.org/patch/225818/
but it hasn't been reviewed by anybody yet
Comment 3 Zhaowei Yuan 2018-07-23 09:07:44 UTC
I posted patch v2 to cc more maintainer:
https://patchwork.freedesktop.org/patch/240324/
Comment 4 Zhaowei Yuan 2018-08-17 02:29:49 UTC
A new patch to fix this issue:
https://patchwork.freedesktop.org/patch/245027/
Comment 5 Mark Janes 2018-08-19 21:32:30 UTC
This seems like a duplicate of 

 https://bugs.freedesktop.org/show_bug.cgi?id=106807

My faint recollection is that the specification of the __LINE__ macro is inconsistent.  If, as Ken says, this has been removed from the specification, why are we looking at patches to change the implementation?
Comment 6 Kenneth Graunke 2018-08-20 11:22:33 UTC
(In reply to Mark Janes from comment #5)
> This seems like a duplicate of 
> 
>  https://bugs.freedesktop.org/show_bug.cgi?id=106807
> 
> My faint recollection is that the specification of the __LINE__ macro is
> inconsistent.  If, as Ken says, this has been removed from the
> specification, why are we looking at patches to change the implementation?

No, __LINE__ is perfectly well defined and not going away.  #line directives to alter the current line using expressions is going away.  I'm pretty sure Zhaowei Yuan's patch fixes a real bug we've had for a long time.
Comment 7 Zhaowei Yuan 2018-08-21 05:45:32 UTC
(In reply to Kenneth Graunke from comment #6)
> (In reply to Mark Janes from comment #5)
> > This seems like a duplicate of 
> > 
> >  https://bugs.freedesktop.org/show_bug.cgi?id=106807
> > 
> > My faint recollection is that the specification of the __LINE__ macro is
> > inconsistent.  If, as Ken says, this has been removed from the
> > specification, why are we looking at patches to change the implementation?
> 
> No, __LINE__ is perfectly well defined and not going away.  #line directives
> to alter the current line using expressions is going away.  I'm pretty sure
> Zhaowei Yuan's patch fixes a real bug we've had for a long time.

Thanks. However, I'm not sure my patch is the best solution, hope someone can review it and give me some suggestion


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.