Bug 239339 - -Wdangling-pointer warning when building ANGLE with GCC 12
Summary: -Wdangling-pointer warning when building ANGLE with GCC 12
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-14 08:57 PDT by Michael Catanzaro
Modified: 2022-05-16 15:36 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2022-04-14 08:57:42 PDT
This is probably a false-positive. I'm a little tempted to build ANGLE with -w since it is third-party code and doesn't make a ton of sense for us to be responsible for their build warnings, but for now we just have a steadily-increasing list of warnings to suppress when building ANGLE.

[1645/6690] Building CXX object Source/ThirdParty/ANGLE/CM....dir/src/compiler/translator/tree_util/IntermRebuild.cpp.o
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp: In member function ‘sh::PostResult sh::TIntermRebuild::traversePost(sh::NodeType, const sh::TIntermNode&, sh::TIntermNode&, VisitBits)’:
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:482:16: warning: storing the address of local variable ‘guard’ in ‘*this.sh::TIntermRebuild::mNodeStack.sh::TIntermRebuild::ConsList<sh::TIntermNode*>::tail’ [-Wdangling-pointer=]
  482 |     mNodeStack = {&currNode, &guard.oldNodeStack};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:481:20: note: ‘guard’ declared here
  481 |     NodeStackGuard guard(mNodeStack);
      |                    ^~~~~
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:481:20: note: ‘<unknown>’ declared here
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp: In function ‘sh::TIntermNode* sh::TIntermRebuild::traverseChildren(sh::NodeType, const sh::TIntermNode&, sh::TIntermNode&, VisitBits)’:
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:411:16: warning: storing the address of local variable ‘guard’ in ‘*this.sh::TIntermRebuild::mNodeStack.sh::TIntermRebuild::ConsList<sh::TIntermNode*>::tail’ [-Wdangling-pointer=]
  411 |     mNodeStack = {&currNode, &guard.oldNodeStack};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:410:20: note: ‘guard’ declared here
  410 |     NodeStackGuard guard(mNodeStack);
      |                    ^~~~~
/home/mcatanzaro/Projects/WebKit/Source/ThirdParty/ANGLE/src/compiler/translator/tree_util/IntermRebuild.cpp:410:20: note: ‘<unknown>’ declared here
Comment 1 Kenneth Russell 2022-04-18 16:11:01 PDT
Please consider filing this bug on http://anglebug.com/ as well. These warnings would be best fixed upstream and from there rolled into WebKit.

If you can provide a comprehensive list of the warnings from your build (or, even better, a patch addressing them - please see https://chromium.googlesource.com/angle/angle/+/refs/heads/main/doc/ContributingCode.md ) then we can help get them fixed.

Thanks.
Comment 2 Michael Catanzaro 2022-04-18 17:28:02 PDT
(In reply to Kenneth Russell from comment #1)
> Please consider filing this bug on http://anglebug.com/ as well. These
> warnings would be best fixed upstream and from there rolled into WebKit.

I agree that would be ideal, but I'd rather not get involved with this upstream.

> If you can provide a comprehensive list of the warnings from your build (or,
> even better, a patch addressing them - please see
> https://chromium.googlesource.com/angle/angle/+/refs/heads/main/doc/
> ContributingCode.md ) then we can help get them fixed.
> 
> Thanks.

These are the only non-suppressed warnings currently.

We have a list of warnings to suppress at the bottom of Source/ThirdParty/ANGLE/CMakeLists.txt:

-Wno-cast-align
-Wno-extra
-Wno-suggest-attribute=format
-Wno-undef
-Wno-unused-parameter
-Wno-return-type
-Wno-comment

I'll either add -Wdangling-pointer there, or maybe just replace the list with -w (warnings are useful for upstream, not for downstream).

If you want, I can disable that and record all the warnings that I see. I would expect it to be quite a lot.
Comment 3 Kenneth Russell 2022-04-18 18:18:30 PDT
If it's not feasible for you to file a bug or propose a fix upstream in ANGLE then I suggest you add -Wdangling-pointer to Source/ThirdParty/ANGLE/CMakeLists.txt rather than -w so that at least new warnings are looked at and their severity can be evaluated in WebKit.

I would ask though that you at least copy/paste the warnings you've found into a new bug on anglebug.com.
Comment 4 Michael Catanzaro 2022-04-19 11:25:19 PDT
OK, will do...
Comment 5 Michael Catanzaro 2022-05-09 07:35:05 PDT
(In reply to Kenneth Russell from comment #3)
> If it's not feasible for you to file a bug or propose a fix upstream in
> ANGLE then I suggest you add -Wdangling-pointer to
> Source/ThirdParty/ANGLE/CMakeLists.txt rather than -w so that at least new
> warnings are looked at and their severity can be evaluated in WebKit.

OK, I will use -Wdangling-pointer.

> I would ask though that you at least copy/paste the warnings you've found
> into a new bug on anglebug.com.

Before I got around to this, bug #240041 happened. I can work around it easily enough by using my development build, but am sufficiently irritated that I'm disinclined to do so. (That said, thanks for helping with it.)
Comment 6 Radar WebKit Bug Importer 2022-05-09 07:35:15 PDT
<rdar://problem/92959790>
Comment 7 Michael Catanzaro 2022-05-09 07:40:49 PDT
Pull request: https://github.com/WebKit/WebKit/pull/566
Comment 8 Michael Catanzaro 2022-05-09 11:45:36 PDT
(In reply to Michael Catanzaro from comment #5)
> Before I got around to this, bug #240041 happened.

It has mysteriously fixed itself, at least for now. I reported:

https://bugs.chromium.org/p/angleproject/issues/detail?id=7288
Comment 9 Michael Catanzaro 2022-05-13 10:06:02 PDT
The ANGLE developers think this might be a real bug, not a false-positive. Hi Dean, it looks like you are familiar with this code. Can you check it please?
Comment 10 Michael Catanzaro 2022-05-13 13:39:35 PDT
Um, an ANGLE developer landed a change hoping to fix this, but it didn't work.

I provided an updated copy of the new warning in the upstream ticket. I was also going to offer to test any updated patch before it lands, to avoid landing another commit that doesn't work, but now I'm blocked from commenting again due to bug #240041. If anybody wants to forward that offer along on the issue tracker, feel free. Seems absurd that I cannot log in even though I could 10 minutes ago, but oh well.
Comment 11 Michael Catanzaro 2022-05-13 13:47:03 PDT
Eh nevermind, I finally realized it's actually just discrimination, not an outright block. I was able to comment after all. See that bug #240041 for the full exciting story....
Comment 12 EWS 2022-05-16 15:36:17 PDT
Committed r294271 (250617@main): <https://commits.webkit.org/250617@main>

Reviewed commits have been landed. Closing PR #566 and removing active labels.