Closed Bug 1363116 Opened 7 years ago Closed 7 years ago

-Werror=ignored-attributes: ignoring attributes applied to '__va_list_tag' after definition

Tracking

()

Status:

RESOLVED FIXED

People

(Reporter: tromey, Assigned: glandium)

References

Details

Attachments

(2 files)

Bug 1363116 - Do not fail the build on -Wattributes or -Wignored-attributes until it is actually fixed 7 years ago Sylvestre Ledru [:Sylvestre] 59 bytes, text/x-review-board-request	glandium : review-	Details
Bug 1363116 - Remove the report{ExtraWarning,StrictMode}ErrorNumberVA wrappers in EitherParser. 7 years ago Mike Hommey [:glandium] 59 bytes, text/x-review-board-request	arai : review+	Details

Tom Tromey :tromey

Reporter

Description

•

7 years ago

I'm building from today's autoland on x86-64 Fedora 25, using the Fedora 25
system gcc:

pokyo. gcc --version
gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1)


The build fails for me with:

/bin/g++ -std=gnu++11 -o Unified_cpp_js_src34.o -c  -I/home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/dist/system_wrappers -include /home/tromey/firefox-git/gecko/config/gcc_hidden.h -DDEBUG=1 -DENABLE_BINARYDATA -DENABLE_SIMD -DJS_CACHEIR_SPEW -DENABLE_SHARED_ARRAY_BUFFER -DEXPORT_JS_API -DJS_HAS_CTYPES '-DDLL_PREFIX="lib"' '-DDLL_SUFFIX=".so"' -DFFI_BUILDING -DMOZ_HAS_MOZGLUE -I/home/tromey/firefox-git/gecko/js/src -I/home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/js/src -I/home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/js/src/ctypes/libffi/include -I/home/tromey/firefox-git/gecko/js/src/ctypes/libffi/src/x86 -I/home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/dist/include  -I/home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/dist/include/nspr        -fPIC  -DMOZILLA_CLIENT -include /home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/js/src/js-confdefs.h -MD -MP -MF .deps/Unified_cpp_js_src34.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe  -g -freorder-blocks -O3 -fno-omit-frame-pointer  -Werror -Wno-shadow -Werror=format  /home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/js/src/Unified_cpp_js_src34.cpp
In file included from /home/tromey/firefox-git/gecko/js/src/frontend/BytecodeEmitter.h:20:0,
                 from /home/tromey/firefox-git/gecko/js/src/frontend/BytecodeCompiler.cpp:16,
                 from /home/tromey/firefox-git/gecko/obj-x86_64-pc-linux-gnu/js/src/Unified_cpp_js_src3.cpp:29:
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h: In instantiation of ‘bool js::frontend::EitherParser<ParseHandler>::reportExtraWarningErrorNumberVA(Args&& ...) [with Args = {std::nullptr_t, unsigned int&, unsigned int&, __va_list_tag (&)[1]}; ParseHandler = js::frontend::FullParseHandler]’:
/home/tromey/firefox-git/gecko/js/src/frontend/BytecodeEmitter.cpp:3661:95:   required from here
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h:253:13: error: ignoring attributes applied to ‘__va_list_tag’ after definition [-Werror=attributes]
             matcher { mozilla::Forward<Args>(args)... };
             ^~~~~~~
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h:253:13: error: ignoring attributes on template argument ‘mozilla::detail::DecaySelector<__va_list_tag [1], true, false>::Type {aka __va_list_tag*}’ [-Werror=ignored-attributes]
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h: In instantiation of ‘bool js::frontend::EitherParser<ParseHandler>::reportStrictModeErrorNumberVA(Args&& ...) [with Args = {std::nullptr_t, unsigned int&, bool, unsigned int&, __va_list_tag (&)[1]}; ParseHandler = js::frontend::FullParseHandler]’:
/home/tromey/firefox-git/gecko/js/src/frontend/BytecodeEmitter.cpp:3675:73:   required from here
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h:281:13: error: ignoring attributes applied to ‘__va_list_tag’ after definition [-Werror=attributes]
             matcher { mozilla::Forward<Args>(args)... };
             ^~~~~~~
/home/tromey/firefox-git/gecko/js/src/frontend/EitherParser.h:281:13: error: ignoring attributes on template argument ‘mozilla::detail::DecaySelector<__va_list_tag [1], true, false>::Type {aka __va_list_tag*}’ [-Werror=ignored-attributes]

Sean Stangl [:sstangl]

Comment 1

•

7 years ago

I can't reproduce locally with same Fedora and GCC version.

Tom Tromey :tromey

Reporter

Comment 2

•

7 years ago

I did a clobber and I still see it.
My current config is:

mk_add_options MOZ_MAKE_FLAGS="-j4"
ac_add_options --enable-dmd
ac_add_options --enable-jitspew
ac_add_options --enable-debug
ac_add_options --enable-optimize
ac_add_options --enable-warnings-as-errors
ac_add_options --disable-crashreporter
mk_add_options AUTOCLOBBER=1
ac_add_options --enable-debug-js-modules
ac_add_options --enable-js-shell

Tom Tromey :tromey

Reporter

Comment 3

•

7 years ago

I'm using cinnabar, the latest commit I have is:

commit c8cb689c1b4acc5a1856c766f34130b01c68ca77
Author: Shane Caraveo <scaraveo@mozilla.com>
Date:   Fri May 5 15:54:19 2017 -0700

    Bug 1360052 handle cancel during http-on-examine-response, r=mayhemer
    
    MozReview-Commit-ID: 607FLbUWjxT

Sean Stangl [:sstangl]

Comment 4

•

7 years ago

Confirmed that it reproduces with --enable-warnings-as-errors. The cause is Bug 1351107. Unfortunately, Waldo just left on an extended vacation.

For the moment, you could work around it by removing that.

Blocks: 1351107

Sylvestre Ledru [:Sylvestre]

Updated

•

7 years ago

Summary: build failure with fedora 25 system gcc → -Werror=ignored-attributes: ignoring attributes applied to '__va_list_tag' after definition

Sylvestre Ledru [:Sylvestre]

Updated

•

7 years ago

Keywords: leave-open

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/165024/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/165024/

Mike Hommey [:glandium]

Assignee

Comment 7

•

7 years ago

mozreview-review

Comment on attachment 8893959 [details]
Bug 1363116 - Do not fail the build on -Wattributes or -Wignored-attributes until it is actually fixed

https://reviewboard.mozilla.org/r/165024/#review172134

The use of va_list in the code that triggers the warning smells very much non-portability. I wouldn't be surprised if it actually actively breaks on non-x86 non-arm. I'd rather not ignore this warning "until it is actually fixed".

Attachment #8893959 - Flags: review?(mh+mozilla) → review-

Sylvestre Ledru [:Sylvestre]

Comment 8

•

7 years ago

Makes sense!
Do you have a clue on how to fix that?

Mike Hommey [:glandium]

Assignee

Comment 9

•

7 years ago

Looking deeper at the code, it might actually be fine, but it's such a maze of templates that it's hard to tell for sure.

I have no idea why this code needs to be entangled so much... plus, according to dxr:
- there are no callers for BytecodeEmitter::reportStrictModeError
- only two for BytecodeEmitter::reportExtraWarning
- the only uses of those two templates EitherParser::reportStrictModeErrorNumberVA and EitherParser::reportExtraWarningErrorNumberVA are the two BytecodeEmitter method mentioned above, that are not templated.

So it would seem the "simple" fix would be to get rid of the templating?

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Blocks: gcc6

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Assignee: nobody → mh+mozilla

Comment hidden (mozreview-request)

Copy construction for va_list is implementation dependent, and most
likely doesn't actually work properly on some platforms. While the
wrapping in EitherParser might currently work on tier-1 platforms,
it also triggers a warning on newer GCC versions that could very well
indicate that they might not work entirely properly. Mixing va_list,
copy construction and templates variadics surely sounds like something
very fragile.

While I understand that the pending changes from bug 1351107 might be
relying on the current state of the code, I'd rather that bug be
reworked so as to please newer versions of GCC than having to figure out
what parts of that bug are still relevant after bug 1351105 and blocking
bug 1389435 further.

Review commit: https://reviewboard.mozilla.org/r/168982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168982/

Tooru Fujisawa [:arai]

Comment 11

•

7 years ago

mozreview-review

Comment on attachment 8897696 [details]
Bug 1363116 - Remove the report{ExtraWarning,StrictMode}ErrorNumberVA wrappers in EitherParser.

https://reviewboard.mozilla.org/r/168982/#review174330

given that bug 1351107 is not active now, this change would be fine.
please add some comment to the BytecodeEmitter methods that it's temporary fix, with bug number (bug 1351107),
there we could look for a workaround (like virtual or something, as discussed in IRC).

Attachment #8897696 - Flags: review?(arai.unmht) → review+

Tooru Fujisawa [:arai]

Comment 12

•

7 years ago

forgot to mention that, I think the proper fix for the template+va_list+copy things should be considered based on the remaining patches for bug 1351107, that's not published yet,
so the fix here should be minimal.

Comment hidden (mozreview-request)

Comment on attachment 8897696 [details]
Bug 1363116 - Remove the report{ExtraWarning,StrictMode}ErrorNumberVA wrappers in EitherParser.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168982/diff/1-2/

Pulsebot

Comment 14

•

7 years ago

Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/1be0e5978942
Remove the report{ExtraWarning,StrictMode}ErrorNumberVA wrappers in EitherParser. r=arai

Carsten Book [:Tomcat]

Comment 15

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/1be0e5978942

Mike Hommey [:glandium]

Assignee

Updated

•

7 years ago

Status: NEW → RESOLVED

Closed: 7 years ago

Keywords: leave-open

Resolution: --- → FIXED

You need to log in before you can comment on or make changes to this bug.