Open Bug 1633092 Opened 4 years ago Updated 1 month ago

Thunderbird build failure: GLSL optimizer output causes a compiler error (GCC-9) error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]

Categories

(Core :: Graphics, defect, P3)

x86_64
Linux
defect

Tracking

()

Tracking Status
firefox77 --- affected

People

(Reporter: ishikawa, Assigned: ishikawa, NeedInfo)

Details

Attachments

(5 files, 1 obsolete file)

I upgraded the source tree of M-C and C-C.

When I tried to build TB on local PC running linux , I got the following error .

running: "/usr/bin/ccache" "/usr/bin/gcc-9" "-std=gnu99" "-O1" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-m64" "-fno-builtin-strlen" "-Wl,--gdb-index" "-Dfdatasync=fdatasync" "-DDEBUG_4GB_CHECK" "-DUSEHELGRIND=1" "-DUSEVALGRIND=1" "-DDEBUG" "-g" "-gsplit-dwarf" "-Werror=sign-compare" "-Werror=unused-result" "-Werror=unused-variable" "-Werror=format" "-I" "glsl-optimizer/include" "-I" "glsl-optimizer/src/mesa" "-I" "glsl-optimizer/src/mapi" "-I" "glsl-optimizer/src/compiler" "-I" "glsl-optimizer/src/compiler/glsl" "-I" "glsl-optimizer/src/gallium/auxiliary" "-I" "glsl-optimizer/src/gallium/include" "-I" "glsl-optimizer/src" "-I" "glsl-optimizer/src/util" "-D__STDC_FORMAT_MACROS" "-DHAVE_ENDIAN_H" "-DHAVE_PTHREAD" "-DHAVE_TIMESPEC_GET" "-o" "/NEW-SSD/moz-obj-dir/objdir-tb3/debug/build/glslopt-2267117f733e84f5/out/glsl-optimizer/src/util/blob.o" "-c" "glsl-optimizer/src/util/blob.c"
cargo:warning=glsl-optimizer/src/util/blob.c: In function ‘ensure_can_read’:
cargo:warning=glsl-optimizer/src/util/blob.c:260:64: error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
cargo:warning=  260 |    if (blob->current <= blob->end && blob->end - blob->current >= size)
cargo:warning=      |                                                                ^~
cargo:warning=cc1: some warnings being treated as errors
exit code: 1
--- stderr
error occurred: Command "/usr/bin/ccache" "/usr/bin/gcc-9" "-std=gnu99" "-O1" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-m64" "-fno-builtin-strlen" "-Wl,--gdb-index" "-Dfdatasync=fdatasync" "-DDEBUG_4GB_CHECK" "-DUSEHELGRIND=1" "-DUSEVALGRIND=1" "-DDEBUG" "-g" "-gsplit-dwarf" "-Werror=sign-compare" "-Werror=unused-result" "-Werror=unused-variable" "-Werror=format" "-I" "glsl-optimizer/include" "-I" "glsl-optimizer/src/mesa" "-I" "glsl-optimizer/src/mapi" "-I" "glsl-optimizer/src/compiler" "-I" "glsl-optimizer/src/compiler/glsl" "-I" "glsl-optimizer/src/gallium/auxiliary" "-I" "glsl-optimizer/src/gallium/include" "-I" "glsl-optimizer/src" "-I" "glsl-optimizer/src/util" "-D__STDC_FORMAT_MACROS" "-DHAVE_ENDIAN_H" "-DHAVE_PTHREAD" "-DHAVE_TIMESPEC_GET" "-o" "/NEW-SSD/moz-obj-dir/objdir-tb3/debug/build/glslopt-2267117f733e84f5/out/glsl-optimizer/src/util/blob.o" "-c" "glsl-optimizer/src/util/blob.c" with args "gcc-9" did not execute successfully (status code exit code: 1).

Please note that I let the rather strict check of "-Werror=sign-compare" in place.
(This was due to a stricter check on mozilla compilation farm some years ago for a particular directory than on the local PC. Thus I got build error on the tryserver while I could build TB locally. Ever since, I simply have decided to let the stricter check on ALL the directories during local build. )

Given that the sign vs unsigned comparison has been a source of many security bugs and general bugs, I would rather see the unsafe expression of sign vs. unsigned comparison GO AWAY (go away was missing in the origina post.!)

But for now, I want to avoid the build error by proper type casting. (short of disabling -Werror=sign-compare")

I looked for blob.c under relevant directories and found blob.c as below.
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/rust/glslopt/glsl-optimizer/src/util/

/NEW-SSD/NREF-COMM-CENTRAL/mozilla is my M-C source directory.

But when I tried to modify it and build, I got the following error. Note the warning about "[replace]".

/usr/bin/ccache /usr/bin/g++-9 -std=gnu++17 -o OSPreferences_gtk.o -c  -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/stl_wrappers -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/system_wrappers -include /NEW-SSD/NREF-COMM-CENTRAL/mozilla/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DDEBUG=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/intl/locale/gtk -I/NEW-SSD/moz-obj-dir/objdir-tb3/intl/locale/gtk -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/intl/locale -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nspr -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-overflow=2 -fno-sized-deallocation -fno-aligned-new -fno-builtin-strlen -Wl,--gdb-index -Dfdatasync=fdatasync -DDEBUG_4GB_CHECK -DUSEHELGRIND=1 -DUSEVALGRIND=1 -DDEBUG -g -gsplit-dwarf -Werror=sign-compare -Werror=unused-result -Werror=unused-variable -Werror=format -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -g -O2 -fvar-tracking -gdwarf-4 -fvar-tracking-assignments -freorder-blocks -fno-omit-frame-pointer -funwind-tables -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include  -MD -MP -MF .deps/OSPreferences_gtk.o.pp  -fdiagnostics-color  /NEW-SSD/NREF-COMM-CENTRAL/mozilla/intl/locale/gtk/OSPreferences_gtk.cpp
error: the listed checksum of `/NEW-SSD/NREF-COMM-CENTRAL/mozilla/third_party/rust/glslopt/glsl-optimizer/src/util/blob.c` has changed:
expected: 214fea1499bc25eed6eb560651f3358cadbaf507b4ec8bdb8f894c13010ab3f5
actual:   9abe0f8715df6bca8f44eafafe1d9ded8d321a8044c2d6ca518d27e3b6eec099
directory sources are not intended to be edited, if modifications are required then it is recommended that [replace] is used with a forked copy of the source
make[4]: *** [/NEW-SSD/NREF-COMM-CENTRAL/mozilla/config/makefiles/rust.mk:294: force-cargo-library-build] Error 101

Obviously, there seems to be a built-in check for unintended modification.
I am new to this code.

How can I change the source code to insert proper type casting and still compile it without the consistency check kicking in?

   if (blob->current <= blob->end && (size_t) (blob->end - blob->current) >= size)

TIA

Flags: needinfo?(jnicol)

Something is fishy here.

I checked my previous build, and sure enough, this file, blob.c was not compiled then.
This explains why the issue(s) in the source file has not been noticed by this occasional patch contributor who uses GCC locally on his linux PC.

There were other "sign vs. unsigned comparison" warnings and one unused variable declaration which were handled as errors in the source code.

The latest error that prompted me to realize there is something very wrong with the local configure (which I have done countless times before) is that I get the following error after using the patch I post here to deal with the sign vs. unsigned compare and unused variable declaration.

/usr/bin/ccache /usr/bin/g++-9 -std=gnu++17 -o crypto.o -c  -DDEBUG=1 -D_GNU_SOURCE -DHAVE_BZLIB_H -DHAVE_ZLIB_H '-DMOZ_RNP_DIST_INFO=Thunderbird 77.0a1 rnp' -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/botan/build/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/bzip2 -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/json-c -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/zlib -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include -fPIC -DMOZILLA_CLIENT -include /NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/src/lib -iquote /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/../json-c -g -g -O2 -fvar-tracking -gdwarf-4 -fvar-tracking-assignments -freorder-blocks -fno-omit-frame-pointer -funwind-tables -fcxx-exceptions  -MD -MP -MF .deps/crypto.o.pp  -fdiagnostics-color  /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib/crypto.cpp
g++-9: error: unrecognized command line option ‘-fcxx-exceptions’; did you mean ‘-fno-exceptions’?

The error basically states that -fcxx-exceptions not recognized by g++-9 is passed to it.

As I explain, these files probably should not be compiled under Linux x86_64 (not sure about this).
They were not compiled before. (I checked the previous build about a week ago on April 19 did not show the compilation of blob.c and others in the patch.

Or the configure process for the third_party directory may not be quire correct?
(Maybe the latest tree has decided to compile third_party tree by default.)

I am a bit confused as to why these inconsistencies seem to happen.
I only pass |configure| to |mach| as I have done many times before this time again.

TIA

Assignee: nobody → ishikawa

The [replace] warning is because this code comes from a rust dependency which gets "vendored" in to m-c. It's saying changes should be made upstream rather than directly in mozilla-central.

This dependency was only recently added which is why you've just started to see this.

Can you please post your .mozconfig file, and the commands you use to configure and build, along with the full log of the output? Thanks.

Flags: needinfo?(jnicol) → needinfo?(ishikawa)

Thank you for your information regarding the check sum.

Here is my mozconfig.

I will upload the configuration log.
I looked at it, and did not notice anything particular...

Still it is a bit strange that blob.c and other files are suddenly compiled.

Flags: needinfo?(ishikawa)
Attached file configure log.

I invoke configure from a shell script. I set "set -vx" which may be a bit confusing, but this makes it sure that every thing is recorded.

Hope this helps.

Do you need the full build log until the failure is seen. (I have patched the source tree with the earlier patch, and thus it fails with a different error now.).

Yes please. The full build log, without the patch so it includes the original failure, would be very useful.

Attachment #9143496 - Attachment mime type: application/octet-stream → text/plain
Attached file build failure log

Here is the build failure log. I am afraid there was another warning (not an error) near the end which I did not realize. (something is wrong with my configure, I think.)

cc1plus: warning: unrecognized command line option ‘-Wno-inconsistent-missing-override’

As I explained earlier, I have no idea why the third_party directory suddenly gets compiled, but I now have a feeling that this is probably done by rust library or something.

In that case, the rust framework ought to properly pickup the flags, etc. that are passed to the compiler the local developer has chosen.

I know I am using GCC which is not the default, but I do believe there is a value of some people using non-standard compiler so that any unintended dependency on |clang| would be caught rather early in development cycle. And that is a good thing IMHO. If |clang| people decide to fix any non-standard features of clang which mozilla code base has come to depend without even realizing it, we would be in a big trouble.

Hope the log file helps.

Thanks, that's very helpful.

The third_party directory would always have been compiled, but third_party/rust/glslopt was only added as a dependency a few days ago, hence why you are just seeing this now.

Rust code usually is built with the flags you specify, which is why we are hitting this error. This particular rust library, however, is a build-time dependency, and therefore should ignore the flags, otherwise it won't compile or link successfully. The library's build script just needs to be updated to ignore some more flags.

This isn't to do with clang or gcc, it will be built with whichever you choose.

Thank you for the explanation.

Then maybe a little hacking is in order.

The library's build script just needs to be updated to ignore some more flags.

Do you know where the library's build script resides, especially the one that compiles this directory?

I searched for the string "cxx" at the top level of ./mozilla directory and noticed that there are files called
old-configure.in and old-configure which were updated at the time of last M-C update(?). So these may be where the c++/g++ option handling is done despite its "old-" moniker.

Any hint to where the options are assembled and passed to this rust third party glsl compilation during build will be appreciated.

TIA

With the patch in https://bugzilla.mozilla.org/attachment.cgi?id=9143376
I could get past the blob.c and other sign vs. unsign comparison issues, etc.

However, then I got hit with -fcxx-exceptions error as follows. (gcc does not understand -fcxx-exceptions).

/usr/bin/ccache /usr/bin/g++-9 -std=gnu++17 -o crypto.o -c  -DDEBUG=1 -D_GNU_SOURCE -DHAVE_BZLIB_H -DHAVE_ZLIB_H '-DMOZ_RNP_DIST_INFO=Thunderbird 77.0a1 rnp' -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/botan/build/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/bzip2 -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/json-c -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/zlib -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include -fPIC -DMOZILLA_CLIENT -include /NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/src/lib -iquote /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/../json-c -g -g -O2 -fvar-tracking -gdwarf-4 -fvar-tracking-assignments -freorder-blocks -fno-omit-frame-pointer -funwind-tables -fcxx-exceptions  -MD -MP -MF .deps/crypto.o.pp  -fdiagnostics-color  /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib/crypto.cpp
comm/third_party/rnp/bn.o
toolkit/library/buildid.o
/usr/bin/ccache /usr/bin/g++-9 -std=gnu++17 -o buildid.o -c  -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/stl_wrappers -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/system_wrappers -include /NEW-SSD/NREF-COMM-CENTRAL/mozilla/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DDEBUG=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/toolkit/library -I/NEW-SSD/moz-obj-dir/objdir-tb3/toolkit/library -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/config -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/widget/windows -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nspr -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-overflow=2 -fno-sized-deallocation -fno-aligned-new -fno-builtin-strlen -Wl,--gdb-index -Dfdatasync=fdatasync -DDEBUG_4GB_CHECK -DUSEHELGRIND=1 -DUSEVALGRIND=1 -DDEBUG -g -gsplit-dwarf -Werror=sign-compare -Werror=unused-result -Werror=unused-variable -Werror=format -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -g -O2 -fvar-tracking -gdwarf-4 -fvar-tracking-assignments -freorder-blocks -fno-omit-frame-pointer -funwind-tables  -MD -MP -MF .deps/buildid.o.pp  -fdiagnostics-color  buildid.cpp
g++-9: error: unrecognized command line option ‘-fcxx-exceptions’; did you mean ‘-fno-exceptions’?

OK, since I was not sure where this -fcxx-exceptions came from, and without solving this issue, I cannot build TB for local testing, I manually
tried to compile the source file by removng -fcxx-exceptions from the command line that triggered the error. Success.
Except that there are other files with the same unrecognized command line option error.
After doing manual compilation of a few such files, I looked around and found the following file.

/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/backend.mk

My MOZOBJ is /NEW-SSD/moz-obj-dir/objedir-tb3/.
In it I found the following line.:

COMPUTED_CXXFLAGS += -DDEBUG=1 -D_GNU_SOURCE -DHAVE_BZLIB_H -DHAVE_ZLIB_H '-DMOZ_RNP_DIST_INFO=Thunderbird 77.0a1 rnp' -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp -I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/botan/build/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/bzip2 -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/json-c -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/zlib -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/include -I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src -I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include -fPIC -DMOZILLA_CLIENT -include /NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/src/lib -iquote /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib -iquote /NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp/../json-c -g -g -O2 -fvar-tracking -gdwarf-4 -fvar-tracking-assignments -freorder-blocks -fno-omit-frame-pointer -funwind-tables  -fcxx-exceptions

I removed the "-fcxx-exceptions" option at the end of this line.
Voila. The build succeeded (!).

So whatever script that generates this backend.mk dynamically during build needs to be modified to make sure
incorrect option is not passed to CC and CXX compilers.

However, I am afraid that sign vs. unsigned compare issue is orthogonal to this issue because I am overriding CFLAGS and CXXFLAGS with environment variable settings before invoking |./mach build|. :

WARNFLAGS="-Werror=sign-compare -Werror=unused-result -Werror=unused-variable -Werror=format"

 ...
export CFLAGS="$CFLAGS $MEMORYMODEL $ASAN -fno-builtin-strlen $SPLITDWARF -Dfdatasync=fdatasync  -DDEBUG_4GB_CHECK -DUSEHELGRIND=1 -DUSEVALGRIND=1 -DDEBUG  $NULLPTRCHK -g -gsplit-dwarf ${WARNFLAGS}"
export CXXFLAGS="$CXXFLAGS $MEMORYMODEL $ASAN  -fno-builtin-strlen $SPLITDWARF -Dfdatasync=fdatasync -DDEBUG_4GB_CHECK -DUSEHELGRIND=1 -DUSEVALGRIND=1 -DDEBUG $NULLPTRCHK  -g -gsplit-dwarf ${WARNFLAGS}"

That I might want to report to INTEL (?) because the source seems to have originated from INTEL?

In any case, |-fcxx-exceptions| now needs to be handled.
Should I file a new bugzilla?

TIA

I fixed the typo of -fcxx-exceptions in the previous post. I forgot the 's' at the end.

I notice in my config log the following lines.
Note the "warning: overriding recipe for target 'librnp.so'.
So |rnp| directory may be handled in a somewhat incorrect manner by the current configure script?

make[4]: Entering directory '/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp'
Makefile:18: warning: overriding recipe for target 'librnp.so'
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/config/rules.mk:608: warning: ignoring old recipe for target 'librnp.so'
comm/third_party/rnp/config.h.stub
/NEW-SSD/moz-obj-dir/objdir-tb3/_virtualenvs/init_py3/bin/python -m mozbuild.action.file_generate /NEW-SSD/NREF-COMM-CENTRAL/mozilla/python/mozbuild/mozbuild/action/process_define_files.py process_define_file src/lib/config.h src/lib/.deps/config.h.pp src/lib/.deps/config.h.stub /NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/third_party/rnp/src/lib/config.h.in
make[4]: Leaving directory '/NEW-SSD/moz-obj-dir/objdir-tb3/comm/third_party/rnp'

I found a bugzilla related to rnp and so the recent addition needs some shaking down yet.

Bug 1631627
RNP does not build with linux64-clang-win-cross toolchain since April 15

Yes there is some work still to do on the RNP libraries.
I plan to address the recipe error in Bug 1634209.
Bug 1634887 for the gcc build problem.

Severity: -- → S4
Flags: needinfo?(jbonisteel)
Priority: -- → P3

What is the next step?

Flags: needinfo?(ishikawa)

This is the most up-to-date patch and I have split the cargo check sum change to another patch.
This is to accommodate other changes to some other third party changes.

Attachment #9143376 - Attachment is obsolete: true

I need to modify the checksum for successful local build after modifying the gls file as in the previous patch.

There are two issues.

  1. The checksum issue is caused by the fact that this is the third party code.
    So where should I report the issue for signed vs unsigned comparison?

  2. The checksum file that contains hash values is so difficult to edit because there is NO newline inside the JSON file
    I know this is not to be touched on by the mere mortals (unless one needs to modify the file for successful local build as in my case, say).
    Where should I report that the file ought to have a new line after every name, value pair as a jason file for easy manual editing?

(not sure your asking Rob, or Jamie, so NI both)

Flags: needinfo?(rob)
Flags: needinfo?(jnicol)
Flags: needinfo?(rob)

Hi Chiaki,

The glslopt rust repository can be found here, however the interesting bit is the C++ library here. However, glsl-optimizer is based on mesa source code, and we want to avoid making modifications to it as much as possible so that we can continue to merge mesa updates easily. It might be worth checking if the issues you see exist in the upstream mesa source code.

You're right that the checksum json file is not intended to be modified by a human. If you edit the Cargo.toml file in the root of the gecko repository and add the following lines:

[patch.crates-io.glslopt]
path = "third_party/rust/glslopt"

that should disable the checksum and allow it to use your local modifications to the glslopt folder. Let me know if you have any issues with that. Thanks.

Flags: needinfo?(jnicol)

(In reply to Jamie Nicol [:jnicol] from comment #20)

Hi Chiaki,

The glslopt rust repository can be found here, however the interesting bit is the C++ library here. However, glsl-optimizer is based on mesa source code, and we want to avoid making modifications to it as much as possible so that we can continue to merge mesa updates easily. It might be worth checking if the issues you see exist in the upstream mesa source code.

Thank you for the pointers.
I will check the original source of mesa.

You're right that the checksum json file is not intended to be modified by a human. If you edit the Cargo.toml file in the root of the gecko repository and add the following lines:

[patch.crates-io.glslopt]
path = "third_party/rust/glslopt"

that should disable the checksum and allow it to use your local modifications to the glslopt folder. Let me know if you have any issues with that. Thanks.

I will try this modification of Cargo.toml.

Flags: needinfo?(ishikawa)

Re modification of source files.
Let me check that the source files touched by my patch are found in the c++ library first.

According to the description under the heading "Dev Notes" at https://github.com/jamienicol/glsl-optimizer,
code seems to be merged as follows.

Pulling Mesa upstream:

git fetch upstream
git merge upstream/master
sh removeDeletedByUs.sh
# inspect files, git rm unneeded ones, fix conflicts etc.
# git commit

So, I first found the files in jamienicol github.
Then I found the files in corresponding mesa site.

They are listed as below.

third_party/rust/glslopt/glsl-optimizer/src/compiler/glsl/ir_print_glsl_visitor.cpp
this is in
https://github.com/jamienicol/glsl-optimizer/blob/master/src/compiler/glsl/ir_print_glsl_visitor.cpp
(touched 6 days ago. So, I better have the modification done in upstream code, indeed.)
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/glsl/ir_print_visitor.cpp
(The strange URL seems to be that mesa URL is partially re-directed to freedesktop.org )

third_party/rust/glslopt/glsl-optimizer/src/compiler/shader_enums.c
this is in
https://github.com/jamienicol/glsl-optimizer/blob/master/src/compiler/shader_enums.c
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/compiler/shader_enums.c

third_party/rust/glslopt/glsl-optimizer/src/util/blob.c
this is in
https://github.com/jamienicol/glsl-optimizer/blob/master/src/util/blob.c
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/util/blob.c

third_party/rust/glslopt/glsl-optimizer/src/util/softfloat.c
this is in
https://github.com/jamienicol/glsl-optimizer/blob/master/src/util/softfloat.c
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/util/softfloat.c

third_party/rust/glslopt/glsl-optimizer/src/util/string_buffer.c
this is in
https://github.com/jamienicol/glsl-optimizer/blob/master/src/util/string_buffer.c
https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/util/string_buffer.c


Well, I am not sure how to report the issues. I will probably only report the affected files used in mozilla code.
The reason being that I am fetching the mesa upstream and it turns out the git clone produces probably 400MB of data and simply compiling the whole library would be too time consuming.
OTOH, I am not sure if devlopers would be happy to receive only the partial modifications.
But anyway, I will try.

I hope they would agree that silent sign extension is hairy especially in softfloat.c [I am not even sure if the original code was OK at all. Maybe we don't use softfloat anymore on modern CPUs on PC and smartphones.]

(Having so many local mqueue patches under ./mozilla directory including mesa GLSL optimizer code
has become too bothersome when I need to update ./mozilla directory.)

ir_print_glsl_visitor.cpp is specific to glsl-optimizer and does not exist in mesa. (ir_print_visitor.cpp is a different file). So please file a bug/pull request in the glsl-optimizer repository for that specific issue. We can easily fix that.

If you want you can file a pull request for glsl-optimizer for the other issues too (shader_enums, blob, softloat, and string_buffer), and then I can file a mesa bug for that.

(In reply to Jamie Nicol [:jnicol] from comment #23)

ir_print_glsl_visitor.cpp is specific to glsl-optimizer and does not exist in mesa. (ir_print_visitor.cpp is a different file). So please file a bug/pull request in the glsl-optimizer repository for that specific issue. We can easily fix that.

If you want you can file a pull request for glsl-optimizer for the other issues too (shader_enums, blob, softloat, and string_buffer), and then I can file a mesa bug for that.

It sounds much easier to work with your repo then.
I just found out the missing file (different name issue) for ir_print_visitor.cpp and ir_print_glsl_visitor.cpp since the patch would not apply to the original mesa repository.

I will pull glsl-optimizer repo and file a bug to it so that you can hopefully make the change upstream.

(Besides, I was not sure how to test mesa with my modification after I figured out how to compile this gigantic source. 2300+ compilation....)

TIA

I filed an issue at https://github.com/jamienicol/glsl-optimizer/issues/7
(I simply attached the git-generated diff as text file, let me know if that is not quite suitable.)

Flags: needinfo?(jnicol)

(In reply to ISHIKAWA, Chiaki from comment #25)

I filed an issue at https://github.com/jamienicol/glsl-optimizer/issues/7
(I simply attached the git-generated diff as text file, let me know if that is not quite suitable.)

glsl-optimizer land seems to be pretty dead

(In reply to Wayne Mery (:wsmwk) from comment #26)

(In reply to ISHIKAWA, Chiaki from comment #25)

I filed an issue at https://github.com/jamienicol/glsl-optimizer/issues/7
(I simply attached the git-generated diff as text file, let me know if that is not quite suitable.)

glsl-optimizer land seems to be pretty dead

Hmm... Let me see what I can do.

Flags: needinfo?(ishikawa)
Summary: TB build failure: GLSL optimizer output causes a compiler error (GCC-9) error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] → Thunderbird build failure: GLSL optimizer output causes a compiler error (GCC-9) error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]

I have not made any progress on this front. I got busy due to an annual technology exhibition as part of the organizer back then. :-(

Flags: needinfo?(ishikawa)
Flags: needinfo?(ishikawa)
Flags: needinfo?(jnicol)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: