Hazard (GCC) builds shouldn't build with -Werror
Categories
(Firefox Build System :: Task Configuration, task, P3)
Tracking
(Not tracked)
People
(Reporter: jgilbert, Unassigned)
Details
Attachments
(2 files)
They're our only GCC builds, so this only results in annoying -Werror GCC-specific errors.
Comment 1•6 years ago
|
||
(In reply to Jeff Gilbert [:jgilbert] from comment #0)
They're our only GCC builds, so this only results in annoying -Werror GCC-specific errors.
I guess this is really sfink's call, but if they are reasonable warnings, it seems like we should just fix them? Or if they're not reasonable warnings, we could disable them in warnings.configure.
Comment 2•6 years ago
|
||
I don't really have a sense for how much value vs pain these are providing. I would certainly be in favor of aggressively discarding the ones we don't care about. There are definitely some useful warnings that gcc provides and clang doesn't, though.
Is there a good way to collect a sample of build failures caused by this? Look at all hazard failures, do log parsing, compare the extracted stuff against /-W/ or something?
(In reply to Jeff Gilbert [:jgilbert] from comment #0)
They're our only GCC builds, so this only results in annoying -Werror GCC-specific errors.
They're some of the few GCC builds, but not the only. Code coverage, for example, depends on GCC profiler data. I don't know if they disable -Werror. (It might just be that they're scheduled less frequently, so you'd always see the problem on a hazard build first.)
Updated•3 years ago
|
Updated•3 years ago
|
Comment 3•2 years ago
|
||
Hi, I am compiling M-C, C-C source to tree to produce C-C TB on my local PC using GCC.
Currently I am using GCC-13.
And I have enabled -Werror for compilation.
The reason for enabling -Werror was that, once in the past, treeherder build system enables it for a few subdirectories of C-C TB. Some of my local patches failed due to stricter check at compile time in treeheder job.
Since I do not know how to selectively enable -Werror on/off locally, I simply enabled them.
But -Werror flag became effective also for M-C compilation during C-C TB creation locally.
So I got stuck with the problem here.
So I fixed all the files where GCC warnings were generated.
It is true that GCC produces many subtle warnings and since with -Error, these warnings result in compile-time error,
I had to produce a lots of local patches in M-C tree and C-C tree to compile C-C TB possible.
I have been compiling the M-C/C-C tree in this manner a few years at least now.
There are about half a dozen files or so which seemed to cause GCC warnings which could be either false-positive or outright strange gcc bug(?). So some files have particular warnings supressed by the judicious use of pragmas where
the strange warnings are produced.
But there are some files which are not quite easy to handle since the warnigs are produced in so many places.
For them I have to create a separate script to copmpile them.
It depends on your version of GCC.
About a month or ago, I checked what happens with clang with -Werror.
Surprisingly, there are some warnings which the latest clang also generated and I had to modify the code in a few places in a similar manner.
I have no idea how people compile the M-C/C-C tree locally.
If people are interested I can post them.
As a matter of fact, I had to create a few patches in the last seven days because GCC produced new warnings.
It IS true that gcc warnings caught my attention to very dubious code. So -Werror IS useful if we can build the programs.
Comment 4•2 years ago
|
||
I had forgotten about this.
I have no idea how people compile the M-C/C-C tree locally.
I think everybody uses clang. Except for the few who run the hazard jobs locally, which is usually just me. But for that I think local builds do not use -Werror.
Surprisingly, there are some warnings which the latest clang also generated and I had to modify the code in a few places in a similar manner.
Perhaps you are using a newer clang than the bootstrapped clang?
There are about half a dozen files or so which seemed to cause GCC warnings which could be either false-positive or outright strange gcc bug(?).
Would it work better for you to use check_and_add_warning("-Wno-annoying-check") (replacing annoying-check with the actual warning flag that is causing trouble) in build/moz.configure/warnings.configure? Or do you think these warnings are useful in other files?
But there are some files which are not quite easy to handle since the warnigs are produced in so many places.
For them I have to create a separate script to copmpile them.
It seems like you could use the thing I mentioned above in warnings.configure for these. Or, if these warnings are only causing problems in some files, you can append -Wno-... in specific directories or even for specific files using moz.build.
Though if it is for a specific file, then you could also use a pragma in the file (as you said you are already doing).
As a matter of fact, I had to create a few patches in the last seven days because GCC produced new warnings.
It looks like the hazard analysis is currently using gcc 9.5.0. I often have to fix several things in order to upgrade the gcc version that the hazard analysis is using, so I don't see the warnings until I try to do that. (And it is rare that I do an upgrade.)
If people are interested I can post them.
I would be interested in seeing what kinds of fixes were required. Do you have an easy way to upload? If you are using mercurial, it might be easiest to create a bundle file. I am fine with looking at whatever is best for you.
I had to produce a lots of local patches in M-C tree and C-C tree to compile C-C TB possible.
I am sorry you have to spend time on this, but your work might be useful to everyone else. To make things easier for you, though, I would recommend using one of the above techniques to suppress warnings that take up too much of your time or do not seem to be useful for improving the code. Your time is valuable.
It IS true that gcc warnings caught my attention to very dubious code. So -Werror IS useful if we can build the programs.
glandium, do you have an opinion on whether to continue to force everyone to fix gcc -Werror failures due to the hazard build? I do not have a strong opinion either way. I don't know whether the nuisance or the benefit is larger in practice.
Comment 5•2 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] from comment #4)
Thank you for your comment.
I had forgotten about this.
I have no idea how people compile the M-C/C-C tree locally.
I think everybody uses clang. Except for the few who run the hazard jobs locally, which is usually just me. But for that I think local builds do not use -Werror.
I should have mentioned that I think using a different compiler (GCC in my case) is a good thing to catch very subtle compiler issue. If mozilla code and C-C TB somehow rely on clang's interpretation of C/C++ standard and somehow clang++ missed a subtle issue, and IF the interpretation changes suddenly, we may have a tough time to change the code base. Using a different compiler can help us catch such problems earlier.
That is my own choice. I am used to GCC in embedded systems programming, and so I chose to pursue compiling C-C TB
with GCC, which was not difficult until somehow I needed to
pass stricter compile time checking which came to be used on treeserver.
Due to my clueless handling of the option "-Werror", I basically applied this option to every subtree during my local build, and it was also applied to M-C portion of the tree.
And that started to cause many build issues as you can imagine.
But, basically, I solved all of them by now, except for a handful of files. So with my patch I can compile M-C and C-C tree with -Werror (except for the handful of files) using GCC.
For the handful of files, I pass -Wno-annoying-warning to compile them in a separately prepared script files.
It took a long time to create the patches, but it was simply eradicating warnings so it was not that difficult except for places where I was not quite sure what the warnings meant, and where GCC-12, GCC-13 seemed to be generating false positives.
Surprisingly, there are some warnings which the latest clang also generated and I had to modify the code in a few places in a similar manner.
Perhaps you are using a newer clang than the bootstrapped clang?
Maybe. My clang is 17.0.6. I just checked.
But I should hasten to add that because of "-Werror" I am using, a new warning is treated as a compile-time error and thus I was forced to fix the source files to suppress the warning.
There are about half a dozen files or so which seemed to cause GCC warnings which could be either false-positive or outright strange gcc bug(?).
Would it work better for you to use
check_and_add_warning("-Wno-annoying-check")(replacingannoying-checkwith the actual warning flag that is causing trouble) inbuild/moz.configure/warnings.configure? Or do you think these warnings are useful in other files?
Basically, for several files I created a separate script to add -Wno-annoying-check to the compilation.
Maybe I should do this, modifying warnings.configure.
But I should be careful that I do not blindly disable useful warnings for other files.
I think some builtin checks for string manipulation functions are useful for buffer overun static detection.
And also, there are some other warnings, but I can't recall them. Maybe I need to look into my local patches.
But there are some files which are not quite easy to handle since the warnigs are produced in so many places.
For them I have to create a separate script to copmpile them.It seems like you could use the thing I mentioned above in
warnings.configurefor these. Or, if these warnings are only causing problems in some files, you can append-Wno-...in specific directories or even for specific files usingmoz.build.
Thank you for the tips. I will investigate this. But I have learned to use prgram, and -Wno-annoying-warning is used in some script files to compile source files which seem to have so many warnings or for which GCC seems to produce so many false positive or incomprehensible warnings.
Currently, I have to run |mach build|, and then some files cannot be compiled with default compiler options including -Werror I added.
So I invoke the special script to compile then.
And then I invoke |mach build| again to build C-C TB.
Though if it is for a specific file, then you could also use a pragma in the file (as you said you are already doing).
As a matter of fact, I had to create a few patches in the last seven days because GCC produced new warnings.
It looks like the hazard analysis is currently using gcc 9.5.0. I often have to fix several things in order to upgrade the gcc version that the hazard analysis is using, so I don't see the warnings until I try to do that. (And it is rare that I do an upgrade.)
GCC-10, GCC-11 had issues with format stringt checking, and GCC-12, GCC-13 also have the same issue to some extent.
You will probably know how newer GCC issues superficially cleverer warnings than the previous version.
I had to upgrade to GCC-13 to avoid some false-positive warnings related to false-positives the format specifier length issue in snprintf(), and some checks done against built-in functions such as string manipulation functions.
But with GCC-13, I see more clever warnings and thus was forced to tweak files to avoid warnings.
If people are interested I can post them.
I would be interested in seeing what kinds of fixes were required. Do you have an easy way to upload? If you are using mercurial, it might be easiest to create a bundle file. I am fine with looking at whatever is best for you.
I am, to your possible surprise, still using HG MQ extension locally.
So I can send you the MQ files. They may not be that great in the sense that the order of application has to be strictly followed since the MQ patches were created in a random ad-hoc fashion as the warnings were generated over the years.
Some patches are about the same file.
On the other hand, I can apply all the patches (some are related to increase the timeout values for execution under
valgrind to check for memory issues, etc.), then with that state, I can create a bundle?
I had to produce a lots of local patches in M-C tree and C-C tree to compile C-C TB possible.
I am sorry you have to spend time on this, but your work might be useful to everyone else. To make things easier for you, though, I would recommend using one of the above techniques to suppress warnings that take up too much of your time or do not seem to be useful for improving the code. Your time is valuable.
I will try to follow your suggestion so that the separate scripts I had to create to cope with the latest GCC-13 warnings can be retired.
About half a dozen files needed to be compiled with -No-annoying-warning option.
For one of them, the warning does not make sense, and I even tried to create a GCC bugzilla entry and tried reducing the preprocessed file using cvise to come up with a smaller source that exhibits the issue.
Unfortunately, I think llvm library which is used by cvise has a mysterious bug and I could not succeed in producing such a file. I noticed that cvise crashed during the reduction process. :-(
It IS true that gcc warnings caught my attention to very dubious code. So -Werror IS useful if we can build the programs.
glandium, do you have an opinion on whether to continue to force everyone to fix gcc
-Werrorfailures due to the hazard build? I do not have a strong opinion either way. I don't know whether the nuisance or the benefit is larger in practice.
FYI, here is the list of MQ patches I have locally for M-C tree.
Many dealt with signed vs unsigned comparison issues. They have been the cause of many security issues in software and thus I think it is worthwhile to check it, but getting them right is an art.
unused variable can be caused by macro definitions. Some variables are only used when a certain architecture macro is defined, etc.
I mark the line with some characters at the beginning of a line.
T is for the patch to increase the timeout.
N is not related to warnings, but rather some features I needed to debug issues in C-C TB.
M is to suppress warnings from valgrind. Not necessarily a real memory issue.
The list used to about 60-70 patches long, but with GCC-13 and the latest source trees and checking with clang compilation, the list jumped to 150. (!)
M 0 A ssl-uninitialized.patch: uninitialized ssl3 patch
M 1 A pickle-alignment.patch: patch to investigate strange sendmsg ...
2 A remove-an-assert-from-mochitest-runtest.patch: remove an asse...
N 3 A check-if-owner-X-bit-is-there.patch: check if owner-X-bit is ...
4 A modules-unused-variable.patch: unused variable patch in modul...
5 A nsprpub-sign-compare-rev01.patch: Bug 1307958 - -Werror=sign-...
6 A js-libffi-sign-compare.patch: Bug 1307958 - -Werror=sign-comp...
7 A intl-icu-sign-compare-rev02.patch: Bug 1307958 - -Werror=sign...
8 A nss-security-sign-vs-unsigned-correction.patch: Bug 1307958 -...
N 9 A dump-seek-error-in-buffered-stream.patch: dump stream pointer...
N 10 A editorDom-assert.patch: Trying to nail down The offset is out...
11 A pixman-access-unused-const-var.patch: -Wunused-variable. Comm...
12 A ffvpx-config-unix64-remove-HAVE_VALGRIND_VALGRIND_H.patch: Av...
13 A nss-freebl-mpi-unused-variable.patch: Removing unused variables.
T 14 A add-timeout-1200-runner.patch: Add longer timeout to runner.py
N 15 A dump-mutator-failure-path.patch: dump aSpec(i.e., path) for S...
N 16 A netwerk-NS_NewBufferedOutputStream-debug-trace.patch: Tracing...
17 A m-c-snprintf-gcc-security-ssl-and-other.patch: Avoid the bug ...
18 A gfx-ots-signed-vs-unsigned.patch: gfx/ots signed vs unsigned ...
N 19 A test-strict-javascript-issues.patch: remove an assert for moc...
T 20 A xpcshell-longer-timeout.patch: longer timeout for xpcshell te...
21 A third-party-glsl.patch: bug 1633092, Strange bug - third part...
T 22 A make-valgrind-work-for-TB-mochitest.patch: make valgrind work...
N 23 A add-dump-BrowserElementParent.patch: add dump to BrowserEleme...
T 24 A runner-define-timeout.patch: define timeout patch
N 25 A 1700420-nsSafeFileOutputStream-Finish.patch: 1700420 Finish()...
26 A media-ffvpx-null-passed-to-string-format-specifier.patch: gcc...
T 27 A valgrind-mach-and-cpp-timeout.patch: mach timeout patch
N/T 28 A valgrind-support-dont-probe-graphics-locally-support.patch: c...
29 A check-return-value-uxproces.patch: check return value of write()
30 A media-sign-compare-rev03.patch: Bug 1307958 - -Werror=sign-co...
31 A unused-vars.patch: Unused variables, etc.
32 A libwebrtc-unused-and-sign-vs-unsigned.patch: temporary fix fo...
N 33 A strange-configure-status-missing-error-invetigation.patch: Wi...
34 A rust.mk-repalce-frozen-with-offline.patch: replace "-frozen" ...
35 A sslsock-unsigned-vs-sign-comparision.patch: type cast unsigne...
36 A checksum-fix.patch: fix checksum
37 A fgx-sign-vs-unsign.patch: gfx sign vs unsign comparison
38 A gettid-linux.patch: gettid undeclared? under linux, it is not...
N 39 A dump-original-unesapce.patch: assert check
N 40 A print-message-when-preferences-shutdown-is-called.patch: prin...
N 41 A add-test-for-low-level-IO.patch: add dump messages for low-le...
N 42 A timestamp-now-has-leading-0-without-space.patch: Log timestam...
N 43 A investigate-mOffset-mismatch.patch: investigate mOffset mismatch
N 44 A string-enumerator-dump.patch: dump key values when a strange ...
45 A Same-EAGAIN-EWOULDBLOCK.patch: EAGAIN and EWOULDBLOCK have th...
46 A plvrsion-version-string.patch: Need to keep rcsid, sccsid som...
47 A UNISTR_FROM_STRING_EXPLICIT-avoid-redefinition.patch: avoid r...
48 A avoid-multiline-comment-warning.patch: avoid multi-line comme...
49 A gfx-pixman-sign-unsigned-comparison.patch: avoid comparing un...
50 A gfx-avoid-fallthrough.patch: avoid fallthrough of switch case...
51 A make-default-cant-happen-Types.h.patch: gcc warning - make de...
T 52 A make-timeout-longer-by-3.patch: Make timeout longer by 3
53 A mark-non-reacheable-generated_message_tctable_impl.patch: mar...
54 A NS_SUCCEEDED.patch: incorrect use of NS_SUCCEEDED()
55 A nss-signed-vs-unsigned.patch: nss signed vs unsigned patch
56 A strange-NS_SUCCEEDED.patch: strange use of NS_SUCCEEDED()
57 A use-after-free.patch: use after free (one is OK usage)
58 A ipc-chromium-libevent.patch: storing the address of a local v...
59 A rtc_no_sanitize.patch: no sanitize patch
N 60 A xpcshell-exclude-due-to-buggy-libsqlite3.patch: Exclude some ...
61 A handle-pragram-to-ignore-Wuse-after-free-for-older-gcc.patch: ..
T 62 A runtests.py-patch-to-extend-timeout-rev02.patch: extend timeo...
63 A gcc-warning-shutup.patch: gcc warning suppression
64 A gcc++-warn-nonnull.patch: shut up gcc-11 warning
65 A glsl-macro-and-hash-fix.patch: Conditionalize MI_PI definitions
66 A strange-json-vs-text-error.patch: To debug json vs text retur...
67 A gcc-12-G_CONST_RETURN.patch: gcc-12 G_CONST_RETURN -> const p...
68 A gcc-12-dangling-pointer.patch: GCC-12 warned dangling pointer...
69 A gcc12-array-dangling-pointer.patch: GCC-12 warns dangaling po...
70 A gcc12-narrowing.patch: gcc-12 warns narrowing
71 A c12-stringop-overflow-TestAnnexB.patch: gcc12 warning stringo...
72 A gcc12-stringop-overflow-nsTArray.patch: gcc-12 warning
73 A use-after-free-nsISupportsImpl.patch: gcc12 warns use-after-free
74 A use-after-free-ProxyAutoConfigChild.patch: gcc12 use-after-fr...
75 A memory-allocation-SPSCQueue.patch: strange malloc range warni...
76 A gcc12-dangling-pointer-IntermRebuild.patch: gcc 12 warns dang...
77 A gcc12-unused-result-evutil_rand.patch: gcc 12 warns unused re...
78 A gcc12-use-after-free-ReadableByteStreamController.patch: gcc ...
79 A gcc12-stringop-overflow-SkPathRef.patch: gcc 12 warns stringo...
80 A gcc13-use-after-free-nsHTTPCompressConv.patch: gcc13 use-afte...
81 A gcc13-dangling-pointer-RootingAPI.patch: shut up gcc13 dangli...
82 A gcc13-self-move.patch: gcc-13 warns self-move
83 A gcc13-alloc-size-larger-than.patch: gcc-13 alloc-size-larger-...
84 A gcc13-mismarched-new-delete.patch: gcc-13 warns mismatched-ne...
85 A gcc13-dangling-pionter-Marking.patch: gcc-13 dangling pointer...
86 A gcc13-dangling-pointer-generic.patch: shut up gcc13 dangling ...
87 A cairo-remove-static.patch: remove ignored static attribute
88 A avoid-macro-re-defintion-cairo-xlib-xrender-private.h: avoid ...
89 A avoid-macro-re-definition-cairo-output-stream.c: avoid macro ...
90 A avoid-macro-re-definition-cairo-path-stroke-polygon.c: avoid ...
91 A avoid-macro-re-definition-cairo-xlib-surface.patch: avoid mac...
92 A avoid-macro-re-definition-cairo-path-stroke-boxes.patch: avoi...
93 A arg-type-fix-util-internal.patch: make arg type compatible
94 A avoid-macro-redefintion-InterpolateShannon.patch: avoid macro...
95 A remove-unused-function-EbmlBufferWriter.patch: remove unused ...
96 A proper-if-guard-vorbis_block.patch: proper if guard
97 A proper-size-to-memset-row_any.patch: Pass proper size to memset
98 A strange-assigment-convert.patch: fix strange assignment (typo)
99 A pass-strict-matching-type-decode.patch: pass strictly same type
100 A meddia-pass-strictly-same-type-state.patch: pass strictly sam...
101 A handle-same-EAGAIN-EWOULDBLOCK-socket.patch: Handle the warni...
102 A remove-unused-function-rtc_event_field_encoding_parser.patch: ..
103 A parenthesis-matched_filter.patch: gcc warning - insert a pair...
104 A gcc-warning-default-case-cant-happen-transient_suppressor_impl.patch:
105 A not-reached-rnn_fc.patch: gcc warning - not reached
106 A return-at-the-end-of-non-void-function-bitrate_prober.patch: ...
107 A always-false-condition-rtp_video_layers_allocation_extension.patch:
108 A wip-suppress-assertion-spew.patch: suppress assertion spew
109 A gcc-avoid-stringop-overflow-h264_profile_level_id-v2.patch: temp
110 A gcc-avoid-stringop-overflow-intl-icu-source-i18n-decNumber.patch:
111 A media-ffvpx-libacvcodec-dct-type-mismatch.patch: int16_t * is...
112 A media-ffvpx-libavutil-hwcontext_vaapi.patch: Unused variables...
113 A remove-std-move-widget-gtk-nsClipboard.patch: Gcc warns std:;...
N 114 A change-runtests.patch: dump some values from test run scripts
N 115 A temp-runtests.patch: change timeout in runtests.py
116 A sslsecur-signed-vs-unsigned.patch: gcc signed vs unsigned war...
117 A gcc-function-signature.patch: gcc compilation issue
118 A always-false-softfloat.patch: always false expression is comm...
119 A sqlite3-not-reached-return.patch: return not reached
120 A gnuc_wrapper_for_gcc-diagnostic.patch: conditionalize based o...
121 A spellcheck-clang.patch: remove unused var
122 A clang-remove-redundant-parenthesis.patch: remove extra duplic...
123 A clang-unused-variable.patch: remove unreached statement
124 A clang-unreached-protobuf.patch: remove unused statement
125 A clang-gcc-clang-conditionalize-pragma.patch: Conditionalize p...
126 A clang-float-conversion.patch: unexact implicit float conversion
127 A clang-float-conversion-libavutil-eval.patch: unexact float co...
128 A clang-webrtc-warning.patch: remove warning from clang so that...
129 A clang-wearnings-intl-icu.patch: suppress warning for strict -...
130 A clang-warning-removal-media-ffvpx.patch: suppuress warnings f...
131 A clang-liborbis-dubious-code.patch: Unsued variables, but is t...
132 A clang-class-varargs.patch: suppress warning for -Werror clang...
133 A clang-warning-aom.patch: suppress warning for clang -Werror c...
134 A clang-non-executed-stmt-warning-sqlite3.patch: suppress warni...
135 A clang-dubious-macro-generation-arning.patch: suppress dubious...
136 A clang-warning-ipc-chromium-libevent.patch: suppress warning f...
137 A clang-sign-compare-pipeware.patch: suppress sign-compare warn...
138 A clang-jxl.patch: suppress warning for clang -Werror compilation
139 A clang-skia-warning.patch: suppress warning for clang -Werror ...
140 A clang-warning-davId-cpu.patch: suppress warning for clang -We...
141 A clang-warning-libopus-unreached.patch: suppress warning for c...
142 A clang-macro-redefinition.patch: suppress warning for clang -W...
143 A clang-unused-variable-libtheora.patch: suppress warning for c...
144 A clang-float-conversion-libvpx.patch: supprress warning for cl...
145 A remove-continued-comment.patch: remove continued comment whic...
146 A unreachable-congestion-control.patch: add __builtin_unreacha...
147 A ffvpx-unused-variable-due-to-macro-definition.patch: static c...
148 A ir_validate.patch: non-executed return after abort
149 A checksum-json.patch: fix checksum
150 A irregexp-warning.patch: Remove some warnings
MQ patch 127 shows that a compile time constant usage (float -> integer) does not end up in a predefined integer value, which the float number ought to match. It is a surprise, and clang warned this.
GCC did not.
I would try to accommodate whatever method might preferable for you to obtain the locally used patches, but
I am not sure how to create hg bundle for you to pick up my patches.
Thank you again for your comment.
Comment 6•2 years ago
|
||
Wow, that is a big stack of patches!
I am fine with either a tarball of your MQ patches directory (since you gave me the order in the previous comment), or a bundle file (you are correct, if you have all of the patches applied then the bundle file should just work).
It looks like many patches are for 3rd party code, and I probably won't bother doing anything with those. We should probably suppress those with moz.build unless they're something really bad. It looks like there is a lot of value in the remaining patches, though.
Comment 7•2 years ago
|
||
If you're getting build failures from warnings in third party code, it means you manually added -Werror to your build flags rather than use ac_add_options --enable-warnings-as-errors. You should use the latter.
As for GCC, we do have a GCC 11 build that does have warnings as errors.
Comment 8•2 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #7)
If you're getting build failures from warnings in third party code, it means you manually added -Werror to your build flags rather than use
ac_add_options --enable-warnings-as-errors. You should use the latter.
I see. That is what I meant I was clueless.
OTOH, because of the handling of warnings as errors, I needed to check if third party upstreams have later revisions, and sure enough some of the upstream source repositories have already fixed the issues I have noticed.
Great learning experience about how people get along with newer and better compiler warnings.
The thing is I already produced the patches before I learn this --enable-warnings-as-errors. Ouch.
As for GCC, we do have a GCC 11 build that does have warnings as errors.
GCC 10 and 11 had a screwed up handling of length specifier in sprintf and friends.
There are legitimate code in C-C TB that produced bogus (well, it seemed bogus to me) warnings treated as errors (-Werro),
but it WAS useful in that I noticed a few problems in my patch where I did not reserve large enough char array due to simple miscalculation.
Because of this screwed up handling of length by sprintf and friends, I upgraded to GCC-13 (GCC-14 introduced more warnings, and I had to stick to GCC-13 for now).
(In reply to Steve Fink [:sfink] [:s:] from comment #6)
Wow, that is a big stack of patches!
It used to be about 50-60 patches long. But my upgrade to GCC-13
and the tree changes (and then later checking if my patches make sense to clang with -Werror) forced me to create more patches and now it is 150 patches long (!)
I am fine with either a tarball of your MQ patches directory (since you gave me the order in the previous comment), or a bundle file (you are correct, if you have all of the patches applied then the bundle file should just work).
It looks like many patches are for 3rd party code, and I probably won't bother doing anything with those. We should probably suppress those with
moz.buildunless they're something really bad. It looks like there is a lot of value in the remaining patches, though.
I am uploading the tarball of my HG MQ patches for your review.
Like I said, I did not want to bother M-C developers initially since it was me who added -Werror to all of M-C subdirectories by my unfamiliarity of how to tweak these options. However, I have begun noticing that some of my patches became no longer necessary BECAUSE the upstreams have fixed the same issues in their code.
They obviously notice the same warnings from newer compiler versions. However, to my dismay, not all problematic parts have been fixed yet.
Anyway, in my next post, I will upload the tar file of my patch directory.
Comment 9•2 years ago
|
||
Like I said, I realize some later patches fixed the spelling mistakes, etc. of earlier patches and
so not all of them are very large.
The most difficult one to handle was nss-security-sign-vs-unsigned-correction.patch.
You will notice that some unsigned int field was filled with (-1). Remember it is unsigned field.
The intent was to store the largest integer in the unsigned scalar.
It became quite interesting when that scalar is compared to a signed value.
GCC warns us about signed vs unsigned comparison.
Problem was paramount on 32-bit build if I recall correctly.
0xFFFFFFF (32bit value) becomes 0x00000000FFFFFFF if simply extended as 64-bit unsigned value.
But if 0xFFFFFFFF (32bit value) is signed extended then it becomes 64-bit 0xFFFFFFFFFFFFFFFF.
Until I realized this subtle issue, I have to look at a few of my type casts to see why they cause the tests to fail if I cast RHS of comparison and not fail if I cast LHS of comparison, etc.
(The literal "-1" is handled as the natural integer scalar in 32-bit build, so it becomes 0xFFFFFFFF.)
Anyway, thank you for your interest in this issue.
If you apply all the relevant patches, I am sure you can upgrade your GCC to a newer version except for the few files that require the explict passing of -Wno-annoying-warnings parameter.
The files are explained in my next post.
Comment 10•2 years ago
|
||
The files that are handled by passing -Wno-annoying-warning option.
Note, all my local tests are full DEBUG build.
An optimization build may not encounter the warnings that my patches and the scripts to handle
the following files try to resolve.
1. mozilla/mfbt/tests/TestSPSCQueue.cpp
I could not grok the warning. I think there is something wrong with
gcc's warning and thus I di not want to bother changing it.
But I am afraid there may be an uninitialized variable issue somewhere.
# inlined from ‘void TestRingMultiThread(int) [with T = float]’ at /NEW-SSD/NREF-COMM-CENTRAL/mozilla/mfbt/tests/TestSPSCQueue.cpp:121:25:
# /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ specified bound between 18446744065119617024 and 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
# 29 | return __builtin___memcpy_chk (__dest, __src, __len,
# | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
# 30 | __glibc_objsize0 (__dest));
# | ~~~~~~~~~~~~~~~~~~~~~~~~~~
2. Unified_cpp_js_src_irregexp0.cpp
This error puzzled me no end.
It seems to complain about the extraction of value of a pointer to a local variable to a global variable or something.
But that action seems to be done to prevent the optimizer to optimize it away or something.
But GCC-13 complains that the value of the pointer to a local auto variable is kept
in a variable with much longer life time. Makes sense, but the warning is not called for here.
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/js/src/irregexp/RegExpAPI.cpp:461:12: error: storing the address of local variable ‘padding’ in ‘*this.js::irregexp::RegExpDepthCheck::dummy_’ [-Werror=dangling-pointer=]
# 461 | dummy_ = padding; /* Prevent padding from being optimized away.*/
# | ~~~~~~~^~~~~~~~~
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/js/src/irregexp/RegExpAPI.cpp:460:13: note: ‘padding’ declared here
# 460 | uint8_t padding[FRAME_PADDING];
# | ^~~~~~~
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/js/src/irregexp/RegExpAPI.cpp:459:33: note: ‘this’ declared here
# 459 | void*) override {
# | ^~~~~~~~
#
3.Unified_cpp_desktop_capture_gn1.cpp
I could not figure out a good code example to shut up this warning and thus I added
-Wno-return-type
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/modules/desktop_capture/linux/wayland/screencast_portal.cc: In static member function ‘static webrtc::ScreenCastPortal::CaptureSourceType webrtc::ScreenCastPortal::ToCaptureSourceType(webrtc::CaptureType)’:
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/modules/desktop_capture/linux/wayland/screencast_portal.cc:47:1: error: control reaches end of non-void function [-Werror=return-type]
# 47 | }
4. Unified_cpp_8_temporal_layers_gn0.cpp
ditto.
# In file included from Unified_cpp_8_temporal_layers_gn0.cpp:2:
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc: In function ‘size_t webrtc::{anonymous}::BufferToIndex(Vp8BufferReference)’:
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/modules/video_coding/codecs/vp8/default_temporal_layers.cc:113:1: error: control reaches end of non-void function [-Werror=return-type]
# 113 | }
5. Unified_cpp_media_codec_gn0.cpp
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/media/base/codec.cc: In lambda function:
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/libwebrtc/media/base/codec.cc:209:3: error: control reaches end of non-void function [-Werror=return-type]
# 209 | };
# | ^
This one has also a warning due to this.
It was not entirely clear how to change the source code to avoid the warning.
So I added -Wno-vla-parameter
# [-Werror=vla-parameter]
6. Unified_cpp_bytestreams0.cpp
I forgot to record why this was handled separatey by a script.
you will see why when you compile this with GCC-13
7. Unified_cpp_js_src24.cpp
Similar to the issue in the file 1.
I think there is something wrong with the static constant value propagation
in GCC's analysis for size bound issue for builtin string manipulation functions.
#
# In function ‘void* memcpy(void*, const void*, size_t)’,
# inlined from ‘void mozilla::PodCopy(T*, const T*, size_t) [with T = unsigned char]’ at /NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/mozilla/PodOperations.h:109:11,
# inlined from ‘static size_t js::TypedArrayObject::objectMoved(JSObject*, JSObject*)’ at /NEW-SSD/NREF-COMM-CENTRAL/mozilla/js/src/vm/TypedArrayObject.cpp:237:21:
#/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ specified bound between 18446744073709551560 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
# 29 | return __builtin___memcpy_chk (__dest, __src, __len,
# | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
# 30 | __glibc_objsize0 (__dest));
# | ~~~~~~~~~~~~~~~~~~~~~~~~~~
#cc1plus: all warnings being treated as errors
8. Unified_cpp_dom_canvas4.cpp
This warning may be false positive or an outright error by GCC, or I misunderstood and
this could be a serious warning.
I tried to understand what GCC was trying to tell me and used cvise to
create a short example of source file that produced the same warning.
I failed. I think cvise and llvm have a bug that cannot handle this right.
#
# /NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/mozilla/dom/WebGLTypes.h: In member function ‘mozilla::webgl::Padded<std::__cxx11::basic_string<char>, 32>& mozilla::webgl::Padded<std::__cxx11::basic_string<char>, 32>::operator=(mozilla::webgl::Padded<std::__cxx11::basic_string<char>, 32>&&)’:
# /NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/mozilla/dom/WebGLTypes.h:663:8: error: statement has no effect [-Werror=unused-value]
# 663 | struct Padded {
# | ^~~~~~
#
9. Unified_c_modules_brotli0.c
so many vla-parameter warnings.
So I gave up modifying the source code.
And it was not quite clear how to rewrite the code to begin with.
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/modules/brotli/dec/decode.c:2266:19: error: argument 2 of type ‘const uint8_t[encoded_size]’ {aka ‘const unsigned char[encoded_size]’} declared as a variable length array [-Werror=vla-parameter]
# 2266 | const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
# | ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# In file included from /NEW-SSD/NREF-COMM-CENTRAL/mozilla/modules/brotli/dec/decode.c:7:
# /NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/brotli/decode.h:228:20: note: previously declared as a pointer ‘const uint8_t *’ {aka ‘const unsigned char *’}
# 228 | const uint8_t *encoded_buffer,
# | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
# /NEW-SSD/NREF-COMM-CENTRAL/mozilla/modules/brotli/dec/decode.c:2268:13: error: argument 4 of type ‘uint8_t[*decoded_size]’ {aka ‘unsigned char[*decoded_size]’} declared as a variable length array [-Werror=vla-parameter]
# 2268 | uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]) {
# | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# /NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/brotli/decode.h:230:14: note: previously declared as a pointer ‘uint8_t *’ {aka ‘unsigned char *’}
# 230 | uint8_t *decoded_buffer);
# | ~~~~~~~~~^~~~~~~~~~~~~~
# cc1: all warnings being treated as errors
Comment 11•2 years ago
|
||
One other thing. My patches were created for GCC-13 obviously. It compiles just fine, and about a month ago, I ran it through clang locally to avoid any problems where clang fails to grok GCC directives.
I conditionalize some patches so that pragmas became only effective when I am compiling the source tree with GCC, but
I may have created a few patches after that and these may not be well behaved when you try to compile it on treeherder.
Proper condionalization may have to be written.
Comment 12•2 years ago
|
||
BTW, I am now using gcc-14 for local compilation now.
I needed to add a few more patches (nine different patches) to fix the warnings once I began using gcc-14.2 a month or so ago.
Some are performance related and I think it would merit incorporation into the tree. I have no idea why clang did not notice these constructs.
Anyway, gcc-14.2 produced one additional dangling pointer warning which I needed to suppress using #pragma.
The current list of patches is as follows.
ishikawa@ip030:/NREF-COMM-CENTRAL/mozilla$ hg qser -vs
0 A ssl-uninitialized.patch: uninitialized ssl3 patch
1 A pickle-alignment.patch: patch to investigate strange sendmsg uninitialized memory access
2 A remove-an-assert-from-mochitest-runtest.patch: remove an assert for mochitest execution of c-c TB.
3 A check-if-owner-X-bit-is-there.patch: check if owner-X-bit is there for mkdir
4 A modules-unused-variable.patch: unused variable patch in modules/woff2/src/woff2_dec.cc
5 A nsprpub-sign-compare-rev01.patch: Bug 1307958 - -Werror=sign-compare in ./nsprpub subdirectory
6 A js-libffi-sign-compare.patch: Bug 1307958 - -Werror=sign-compare in ./js subdirectory
7 A intl-icu-sign-compare-rev02.patch: Bug 1307958 - -Werror=sign-compare under ./intl directory
8 A nss-security-sign-vs-unsigned-correction.patch: Bug 1307958 - -Werror=sign-compare under ./security/nss d...
9 A dump-seek-error-in-buffered-stream.patch: dump stream pointer, error, whence and offset when an seek erro...
10 A editorDom-assert.patch: Trying to nail down The offset is out of bounds: '\!mParent || mOffset.value() <=...
11 A ffvpx-config-unix64-remove-HAVE_VALGRIND_VALGRIND_H.patch: Avoid conflict of HAVE_VALGRIND_VALGRIND_H def...
12 A nss-freebl-mpi-unused-variable.patch: Removing unused variables.
13 A add-timeout-1200-runner.patch: Add longer timeout to runner.py
14 A dump-mutator-failure-path.patch: dump aSpec(i.e., path) for SetSpec failure
15 A netwerk-NS_NewBufferedOutputStream-debug-trace.patch: Tracing outputStream of NS_NewBufferedOutputStream ...
16 A m-c-snprintf-gcc-security-ssl-and-other.patch: Avoid the bug of strict snprintf format width check of gcc...
17 A gfx-ots-signed-vs-unsigned.patch: gfx/ots signed vs unsigned comparison fixes.#
18 A test-strict-javascript-issues.patch: remove an assert for mochitest execution of c-c TB.
19 A xpcshell-longer-timeout.patch: longer timeout for xpcshell test that runs under valgrind
20 A third-party-glsl.patch: bug 1633092, Strange bug - third party code suddenly getting compiled
21 A make-valgrind-work-for-TB-mochitest.patch: make valgrind work for TB mochitest |mach| change and timeout ...
22 A add-dump-BrowserElementParent.patch: add dump to BrowserElementParent.jsm
23 A runner-define-timeout.patch: define timeout patch
24 A 1700420-nsSafeFileOutputStream-Finish.patch: 1700420 Finish() ought to catch and propagate the error from...
25 A media-ffvpx-null-passed-to-string-format-specifier.patch: gcc warns that null is passed to %s in format s...
26 A valgrind-mach-and-cpp-timeout.patch: mach timeout patch
27 A valgrind-support-dont-probe-graphics-locally-support.patch: check return value of write()
28 A check-return-value-uxproces.patch: check return value of write()
29 A media-sign-compare-rev03.patch: Bug 1307958 - -Werror=sign-compare in ./media subdirectory
30 A unused-vars.patch: Unused variables, etc.
31 A libwebrtc-unused-and-sign-vs-unsigned-avoid-stringop-overflow-h264_profile_level_id.patch: temporary fix ...
32 A strange-configure-status-missing-error-invetigation.patch: Without |mach bootstrap| something went wrong.
33 A sslsock-unsigned-vs-sign-comparision.patch: type cast unsigned int vs int comparison
34 A checksum-fix.patch: fix checksum
35 A fgx-sign-vs-unsign.patch: gfx sign vs unsign comparison
36 A gettid-linux.patch: gettid undeclared? under linux, it is not a function.
37 A dump-original-unesapce.patch: assert check
38 A print-message-when-preferences-shutdown-is-called.patch: print a message when Preferences::Shutdown() is ...
39 A add-test-for-low-level-IO.patch: add dump messages for low-level I/O (seek, etc.)
40 A timestamp-now-has-leading-0-without-space.patch: Log timestamp now has leading 0 without leading space
41 A investigate-mOffset-mismatch.patch: investigate mOffset mismatch
42 A string-enumerator-dump.patch: dump key values when a strange situation occurs.
43 A Same-EAGAIN-EWOULDBLOCK.patch: EAGAIN and EWOULDBLOCK have the same value under linux and gcc warns
44 A plvrsion-version-string.patch: Need to keep rcsid, sccsid somehow
45 A UNISTR_FROM_STRING_EXPLICIT-avoid-redefinition.patch: avoid redefinition of macro
46 A avoid-multiline-comment-warning.patch: avoid multi-line comment warning
47 A gfx-avoid-fallthrough.patch: avoid fallthrough of switch case statements
48 A make-default-cant-happen-Types.h.patch: gcc warning - make default case cannot happen
49 A make-timeout-longer-by-3.patch: Make timeout longer by 3
50 A mark-non-reacheable-generated_message_tctable_impl.patch: mark non-reacheable code as such
51 A nss-signed-vs-unsigned.patch: nss signed vs unsigned patch
52 A strange-NS_SUCCEEDED.patch: strange use of NS_SUCCEEDED()
53 A use-after-free.patch: use after free (one is OK usage)
54 A ipc-chromium-libevent.patch: storing the address of a local variable to a global variable
55 A rtc_no_sanitize.patch: no sanitize patch
56 A xpcshell-exclude-due-to-buggy-libsqlite3.patch: Exclude some tests due to libsqlite3 issue
57 A handle-pragram-to-ignore-Wuse-after-free-for-older-gcc.patch: -Wuse-after-free has to be disabled in one ...
58 A runtests.py-patch-to-extend-timeout-rev02.patch: extend timeout for valgrind
59 A gcc-warning-shutup.patch: gcc warning suppression
60 A gcc++-warn-nonnull.patch: shut up gcc-11 warning
61 A glsl-macro-and-hash-fix.patch: Conditionalize MI_PI definitions
62 A strange-json-vs-text-error.patch: To debug json vs text return type
63 A gcc-12-G_CONST_RETURN.patch: gcc-12 G_CONST_RETURN -> const patch
64 A gcc-12-dangling-pointer.patch: GCC-12 warned dangling pointer ???
65 A gcc12-array-dangling-pointer.patch: GCC-12 warns dangaling pointer to a local variable
66 A gcc12-narrowing.patch: gcc-12 warns narrowing
67 A c12-stringop-overflow-TestAnnexB.patch: gcc12 warning stringop-overflow
68 A gcc12-stringop-overflow-nsTArray.patch: gcc-12 warning
69 A use-after-free-nsISupportsImpl.patch: gcc12 warns use-after-free
70 A use-after-free-ProxyAutoConfigChild.patch: gcc12 use-after-free warning
71 A memory-allocation-SPSCQueue.patch: strange malloc range warning/error
72 A gcc12-dangling-pointer-IntermRebuild.patch: gcc 12 warns dangling pointer
73 A gcc12-unused-result-evutil_rand.patch: gcc 12 warns unused result
74 A gcc12-use-after-free-ReadableByteStreamController.patch: gcc 12 warns use-after-free
75 A gcc12-stringop-overflow-SkPathRef.patch: gcc 12 warns stringop-overflow
76 A gcc13-use-after-free-nsHTTPCompressConv.patch: gcc13 use-after-free warning
77 A gcc13-dangling-pointer-RootingAPI.patch: shut up gcc13 dangling pointer warning
78 A gcc13-self-move.patch: gcc-13 warns self-move
79 A gcc13-alloc-size-larger-than.patch: gcc-13 alloc-size-larger-than warning
80 A gcc13-mismarched-new-delete.patch: gcc-13 warns mismatched-new-delete
81 A gcc13-dangling-pionter-Marking.patch: gcc-13 dangling pointer warning
82 A gcc13-dangling-pointer-generic.patch: shut up gcc13 dangling pointer warnings
83 A cairo-remove-static.patch: remove ignored static attribute
84 A avoid-macro-re-defintion-cairo-xlib-xrender-private.h: avoid macro redefinition by undefining it first
85 A avoid-macro-re-definition-cairo-output-stream.c: avoid macro re-definition
86 A avoid-macro-re-definition-cairo-path-stroke-polygon.c: avoid macro re-definition
87 A avoid-macro-re-definition-cairo-xlib-surface.patch: avoid macro-re-definition
88 A avoid-macro-re-definition-cairo-path-stroke-boxes.patch: avoid macro re-definition
89 A arg-type-fix-util-internal.patch: make arg type compatible
90 A avoid-macro-redefintion-InterpolateShannon.patch: avoid macro re-definition
91 A remove-unused-function-EbmlBufferWriter.patch: remove unused function
92 A proper-if-guard-vorbis_block.patch: proper if guard
93 A proper-size-to-memset-row_any.patch: Pass proper size to memset
94 A strange-assigment-convert.patch: fix strange assignment (typo)
95 A pass-strict-matching-type-decode.patch: pass strictly same type
96 A remove-unused-function-rtc_event_field_encoding_parser.patch: remove unused function
97 A parenthesis-matched_filter.patch: gcc warning - insert a pair of parentheis
98 A gcc-warning-default-case-cant-happen-transient_suppressor_impl.patch: gcc warning - default case and not ...
99 A not-reached-rnn_fc.patch: gcc warning - not reached
100 A return-at-the-end-of-non-void-function-bitrate_prober.patch: gcc warning - not reached
101 A wip-suppress-assertion-spew.patch: suppress assertion spew
102 A gcc-avoid-stringop-overflow-intl-icu-source-i18n-decNumber.patch: gcc13 - avoid stringop overflow (initia...
103 A media-ffvpx-libacvcodec-dct-type-mismatch.patch: int16_t * is not compatible with the usage in the code
104 A media-ffvpx-libavutil-hwcontext_vaapi.patch: Unused variables depending on the macors defined (!)
105 A remove-std-move-widget-gtk-nsClipboard.patch: Gcc warns std:;move is wasteful in terms of efficiency here
106 A change-runtests.patch: dump some values from test run scripts
107 A temp-runtests.patch: change timeout in runtests.py
108 A sslsecur-signed-vs-unsigned.patch: gcc signed vs unsigned warning
109 A gcc-function-signature.patch: gcc compilation issue
110 A always-false-softfloat.patch: always false expression is commented out
111 A sqlite3-not-reached-return.patch: return not reached
112 A gnuc_wrapper_for_gcc-diagnostic.patch: conditionalize based on __clang__
113 A spellcheck-clang.patch: remove unused var
114 A clang-unused-variable.patch: remove unreached statement
115 A clang-unreached-protobuf.patch: remove unused statement
116 A clang-gcc-clang-conditionalize-pragma.patch: Conditionalize pragma depending on __clang__ and __GNUC__
117 A clang-float-conversion.patch: unexact implicit float conversion
118 A clang-float-conversion-libavutil-eval.patch: unexact float conversion warning
119 A clang-wearnings-intl-icu.patch: suppress warning for strict -Wall clang compilation
120 A clang-warning-removal-media-ffvpx.patch: suppuress warnings for clang -Werror compilation
121 A clang-liborbis-dubious-code.patch: Unsued variables, but is the code correct?
122 A clang-class-varargs.patch: suppress warning for -Werror clang compilation
123 A clang-warning-aom.patch: suppress warning for clang -Werror compilation
124 A clang-non-executed-stmt-warning-sqlite3.patch: suppress warning by moving yylex generated statement for c...
125 A clang-warning-ipc-chromium-libevent.patch: suppress warning for clang -Werror compilation
126 A clang-sign-compare-pipeware.patch: suppress sign-compare warning for clang -Werror compilation
127 A clang-jxl.patch: suppress warning for clang -Werror compilation
128 A clang-skia-warning.patch: suppress warning for clang -Werror compilation
129 A clang-warning-davId-cpu.patch: suppress warning for clang -Werror compilation
130 A clang-warning-libopus-unreached.patch: suppress warning for clang -Werror compilation
131 A clang-macro-redefinition.patch: suppress warning for clang -Werror compilation
132 A clang-float-conversion-libvpx.patch: supprress warning for clang -Werror conversion
133 A remove-continued-comment.patch: remove continued comment which clang warns
134 A unreachable-congestion-control.patch: add __builtin_unreachable()
135 A ffvpx-unused-variable-due-to-macro-definition.patch: static const uint8_t simple_mmx_permutation[64] is n...
136 A ir_validate.patch: non-executed return after abort
137 A checksum-json.patch: fix checksum
138 A irregexp-warning.patch: Remove some warnings
139 A incorrect-NS_SUCCEEDED-use.patch: NS_SUCCEEDED needs to be used as rvalue.
140 A glslopt-clang-format-if-indention-issue.patch: clang format a couple of files to suppress indentation war...
141 A gcc-asan.patch: gcc asan attempt
142 A EAGAIN-EWOUDBLOCK-in-ptio..patch: move a macro definition outside #if DEBUG/#endif block for non-DEBUG build
143 A clang-non-DEBUG-unused-var-add-2.patch: unused variable in non-DEBUG build reporeted by clang during asan...
144 A make-setEOF-fail-at-build-time-if-not-implemented.patch: setEOF cannot be not implemented
145 A SHARPYUV_VERSION_MAJOR.patch: gcc warning - SHARPYUV_VERSION_MAJOR can be 0 and in that case we have
146 A media-ffvpx-pointer-cast.patch: pointer cast
147 A cairo-tag-attributes-duplicate-cond.patch: (! content_attrs->tag_name) repeated.
148 A gfx-pixman-sign-unsigned-comparision-v2.patch: always true cond in ASSERT()
149 A pixman-region-discarded-qualifier.patch: discarded qualifier patch, simply ignore
150 A security-nss-sign-vs-unsigned-add.patch: sign vs unsigned patch
151 A h26x_packet_buffer-unreacheable.patch: unreachable compilation error for gcc
152 A rtcp_packet.patch: shut up gcc-13 warning for returning nothing at the end of a function
153 A glsl-patch-add.patch:
154 A type_info_TEMP.patch: moved class type_info declaration to an earlier position
155 A spellcheck-ubsan-002.patch: UBSAN fix
156 A glsl-optimizer-unused-var-gcc-vs-clang.patch:
157 A trying-NEW-libevent.patch: trying new libevent patch
158 A gcc-14.2-warning-as-error.patch: gcc-14.2 avoid warning as treated error
159 A processhandler-modification-for-avoiding-stall-fallout.patch: 1915540 etc
160 A encoder_stream_factory-unsigned-zero.patch: 0 needs to written as 0L in one place.
161 A nss-freebl-mpi-sign-vs-unsigned-fix.patch:
162 A avoid-copying-according-to-gcc14.patch: Avoid copying
163 A not-reached-additional-cases.patch: avoid reaching at the end of non-void function
164 A self-move.patch: gcc-14
165 A gcc-14.2-dangling-pointer.patch: gcc-14.2
166 A process-handler-fix.patch:
167 U event-old-api-3-params.patch: Revert to patch
168 U reversed-event.patch: reversed libevent 2.2.0 and libevent-mozilla
ishikawa@ip030:/NREF-COMM-CENTRAL/mozilla$
167 and 168 are to be ignored. I have tried to incorporate the latest libevent from git repository to see if the newer code removes strange error issue reported by SANITIZED build (and it did).
The patches in 167 and 168 tried to apply and then revert the local changes which are no longer applicable.
I needed to cope with reformatting of files under nsprpub directory, which seemed to take place a few weeks ago, and many patches were modified accordingly.
Just FYI, if anyone cares to compile M-C/C-C TB using the latest GCC and wants to apply -Werror to weed out dubious constructs in their own patches.
The zip file contains files not listed in the hg qser patch list above. They are old or experimental patches, which I removed for later analysis.
Comment 13•2 years ago
|
||
With the patches in comment 12, I can run C-C TB mochitest and xpcshell-test successfully, no crashes.
Also I can run C-C xpcshell-test and mochitest using the sanitized version of C-C TB.
I do not find any serious errors except for some timeout issues obviously introduced by the slow down of ASAN build, I think.
So I think they are good as far as C-C TB is concerned.
valgrind run is another issue, since valgrind cannot run successfully due to strange timeout and other issues.
The performance issue I mentioned is
162 A avoid-copying-according-to-gcc14.patch: Avoid copying
...
164 A self-move.patch: gcc-14
These are micro optimizations noticed by gcc-14.2, but they could have been written such a way due to some
issues. So, I am not sure if they may cause problems for Firefox. (Maybe I should submit an M-C treeherder job instead of M-C/C-C TB job.)
There are empty patches which I have not removed yet. Some have been taken care of by the M-C tree lately or
the code has changed and the constructs no long exist any more, etc.
Description
•