Closed Bug 1513236 Opened 3 years ago Closed 2 years ago

Update in-tree libpng to version 1.6.37


(Core :: ImageLib, defect, P3)




Tracking Status
firefox68 --- fixed


(Reporter: RyanVM, Assigned: RyanVM)


(Blocks 1 open bug)


(Whiteboard: [gfx-noted])


(1 file)

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
* 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.
Comment on attachment 9033042 [details]
Bug 1513236 - Update libpng to version 1.6.37. r=aosmond

This looks good on Try:

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:

And a MOZCHANGES file that looks like:

And finally, our APNG patch now contains only the relevant code changes without any of the license/version changes it used to contain:

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.
Attachment #9033042 - Flags: feedback?(mhoye)
Attachment #9033042 - Flags: feedback?(ctruta)
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.
Attachment #9033042 - Flags: feedback?(ctruta)
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.

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.
Blocks: 1515602

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.

Attachment #9033042 - Flags: feedback?(mhoye) → feedback+

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?

Flags: needinfo?(ryanvm)

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

Flags: needinfo?(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.
Summary: Update in-tree libpng to version 1.6.36 → Update in-tree libpng to version 1.6.37

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:

Attachment #9033042 - Attachment description: Bug 1513236 - Update libpng to version 1.6.36. r=aosmond → Bug 1513236 - Update libpng to version 1.6.37. r=aosmond
Pushed by
Update libpng to version 1.6.37. r=aosmond
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.