1513236 - Update in-tree libpng to version 1.6.37

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Ryan VanderMeulen [:RyanVM]]

I probably won't have time to work on this for a few days while dealing with some release work and other post-All Hands digging out, but getting it filed anyway.

Changes since the last public release (1.6.35):
* Optimized png_do_expand_palette for ARM processors.
* Improved performance by around 10-22% on a recent ARM Chromebook.
* Fixed manipulation of machine-specific optimization options.
* Used memcpy instead of manual pointer arithmetic on Intel SSE2.
* Fixed build errors with MSVC on ARM64.
* Fixed detection of libm in CMakeLists.
* Fixed incorrect creation of pkg-config file in CMakeLists.
* Fixed the CMake build on Windows MSYS by avoiding symlinks.
* Fixed a build warning on OpenBSD.
* Fixed various typos in comments.
* Raised the minimum required CMake version from 3.0.2 to 3.1.
* Removed yet more of the vestigial support for pre-ANSI C compilers.
* Removed ancient makefiles for ancient systems that have been broken
  across all previous libpng-1.6.x versions.
* Removed the Y2K compliance statement and the export control
  information.
* Applied various code style and documentation fixes.

I've already got a green Try push the nearly-final release from a couple weeks ago, but need to do some final clean-ups around the license changes.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=650269bf52ebce91fe397906deb8574540dd3792

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1513236 - Update libpng to version 1.6.37. r=aosmond

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9033042 [details]
Bug 1513236 - Update libpng to version 1.6.37. r=aosmond

This looks good on Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c96311ba87b5c2b92d5f362e16b550f36ecc6487

From a non-technical standpoint, the most notable changes in this patch are the ones around licensing. With the 1.6.36 release, libpng has updated to a newer OSI-approved license. I had Mike Hoye look it over and there wasn't anything objectionable in there he could see, however it did change the structure of the license around including the removal of some verbiage around making modifications to it which bitrotted some of the changes we'd been making previously.

After discussing it with Cosmin, we reached the conclusion that this was an opportunity to clean things up a bit with respect to how libpng is managed in our tree. In prior releases, we had to make a number of changes to the license, version string, etc for the APNG changes we patch into our copy of libpng. However, the intent of those changes was more for situations where the modified library was being redistributed elsewhere as a standalone library, not just being built into a shipping app. For that reason, it was felt that we could simplify life greatly and just include the notes about our changes in the already-existing MOZCHANGES file and leave the APNG patch to just the actual code changes needed. This will also make future updates a lot easier since it means that our patch is less-likely to bitrot between releases.

That way forward is what I've submitted in this patch. The result is a pristine LICENSE file:
Before: https://hg.mozilla.org/mozilla-central/raw-file/tip/media/libpng/LICENSE
After:  https://hg.mozilla.org/try/file/f473506a4d19/media/libpng/LICENSE

And a MOZCHANGES file that looks like:
Before: https://hg.mozilla.org/mozilla-central/raw-file/tip/media/libpng/MOZCHANGES
After:  https://hg.mozilla.org/try/file/f473506a4d19/media/libpng/MOZCHANGES

And finally, our APNG patch now contains only the relevant code changes without any of the license/version changes it used to contain:
Before: https://hg.mozilla.org/mozilla-central/raw-file/tip/media/libpng/apng.patch
After:  https://hg.mozilla.org/try/file/f473506a4d19/media/libpng/apng.patch

Asking feedback from Cosmin and Mike on the above just to ensure that we're all on the same page with this. I think these changes get us to a much nicer place than where we used to be and will make future maintenance much easier.

Comment 3

[Cosmin Truta]

Reviewed. LGTM.

One little correction to Ryan's message: neither the new nor the old libpng license are OSI-approved. I applied for OSI review for the new license, but some specific things need to be done first, and it will take a while. For that reason, I decided not to postpone the release of libpng-1.6.36.

Comment 4

[Cosmin Truta]

RyanVM, FYI: you may want to delay the landing until the issue mentioned below is sorted out.

A Debian developer noticed that a memory leak showed up in the testing of the ARM64 optimization. I didn't have a chance to run extended tests on ARM64 myself yet.
https://github.com/glennrp/libpng/issues/266

At this point, I can't say for sure if the leak is in the libpng code, or is caused by the test driver.

The ARM developer who wrote the code in question will look into it also.

Comment 5

[Mike Hoye [:mhoye]]

Comment on attachment 9033042 [details]
Bug 1513236 - Update libpng to version 1.6.37. r=aosmond

As mentioned in Phabricator, r+ for this but I need a followup bug to update About:Licenses with the new license.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:RyanVM, could you have a look please?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

We're holding off on updating until comment 4 is resolved upstream.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

1.6.37 fixes the issues noted in 1.6.36.

Version 1.6.37 [April 14, 2019]

• Fixed a use-after-free vulnerability (CVE-2019-7317) in png_image_free.
• Fixed a memory leak in the ARM NEON implementation of png_do_expand_palette.
• Fixed a memory leak in pngtest.c.
• Fixed two vulnerabilities (CVE-2018-14048, CVE-2018-14550) in
  contrib/pngminus; refactor.
• Changed the license of contrib/pngminus to MIT; refresh makefile and docs.
  (Contributed by Willem van Schaik)
• Fixed a typo in the libpng license v2.
  (Contributed by Miguel Ojeda)
• Added makefiles for AddressSanitizer-enabled builds.
• Cleaned up various makefiles.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

I needed to patch libpng to avoid Windows aarch64 errors caused by using the wrong header for Neon intrinsics. I've submitted the patch upstream as PR #285.

Otherwise, things look good on Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=18c33df1aa9a169f2c1b49b43fbf403e68bab337

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/179c0370dfa1
Update libpng to version 1.6.37. r=aosmond

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/179c0370dfa1