FTBFS in Eoan due to gcc9 "Deprecated pre-processor symbol" on GStaticMutex

Bug #1842301 reported by Christian Ehrhardt 
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
open-vm-tools (Ubuntu)
Fix Released
Undecided
Unassigned
Bionic
Fix Released
Undecided
Unassigned
Disco
Fix Released
Undecided
Unassigned

Bug Description

GStaticMutex is deprecated (a long time) [1][2]

gcc-9 with Werror detectas and breaks on this as an error nowadays:

fileLogger.c: In function ‘FileLoggerLog’:
fileLogger.c:351:13: error: Deprecated pre-processor symbol [-Werror]
  351 | g_static_mutex_lock(&logger->lock);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~

sysLogger.c: In function ‘SysLoggerUnref’:
sysLogger.c:108:13: error: Deprecated pre-processor symbol [-Werror]
  108 | g_static_mutex_lock(&gSysLoggerLock);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Upstream is close to v11, but the github content has nothing yet :-/.
This should be mostly a global search and replace with some extra thought on the places the locks are initialized.

[1]: https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-static-mutex-lock
[2]: https://developer.gnome.org/glib/stable/glib-Threads.html#g-mutex-lock

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Old init had multiple ocurrances which were not static anyway.
Two places had ... = G_STATIC_MUTEX_INIT

Per [2] just declaring them static will do it with the new GMutex.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

It seems I have a patch that resolves the above, but there are more gcc-9 errors that show up later:

hgfsServer.c:6541:43: error: taking address of packed member of ‘struct HgfsReplyReadV3’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
 | &reply->actualSize);
 | ^~~~~~~~~~~~~~~~~~

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This is a bit more tricky, obviously one should pass the struct and dereference in the target.
But since the source struct changes based on versions between HgfsReplyRead and HgfsReplyReadV3 type that isn't too easy.
I think the next obvious choice is to pass a local var from the top function and then dereference there.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

ok done and LGTM, and another one ...

3235 util_misc.c: In function ‘Util_ExpandString’:
3236 util_misc.c:722:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
3237 722 | Log("%s: Cannot allocate memory to expand \"%s\" in \"%s\".\n",
3238 | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3239 723 | __FUNCTION__, expand, fileName);
3240 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

And another deprecated function ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Bad feeling: v11 is nowhere published yet, but I expect they have fixed it there :-/
All work done twice ?

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

And another packed struct pointer access ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

And one more for gthread initialization ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Ok, a few patches further in I think this is upstreams task to be resolved.
We can (if there is no timely response) instead disable the new warnings.

There is too much potential that we spend effort and all we get is deriving from upstream too much.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Finally found https://github.com/vmware/open-vm-tools/issues/330
Maybe I can pick up more from there.

It confirms that fixes will be in the next major version (v11) therefore we might disable remainign warnings for 10.3.10

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Commits (among others) would be:
553d128348f311080e156519e9ac26a518c8579d
c68172ef7f2d4f116078e2aba82986a8cab0b16e
19ca3e36f6f16842a54f6f297f3cafcabde2f384
a7c141fc5278146b86c71502f60767962b752af7

Filed https://github.com/vmware/open-vm-tools/issues/367 for what we have seen on top.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Too much noise to reasonable use the commits I found for 10.3.10 as-is.
-Wno-error=address-of-packed-member
-Wno-error=deprecated-declarations

But these category of errors seems not to go away:
sysLogger.c: In function ‘SysLoggerUnref’:
sysLogger.c:108:13: error: Deprecated pre-processor symbol [-Werror]
  108 | g_static_mutex_lock(&gSysLoggerLock);

OTOH as already stated the full glib change backport seems wrong for Eoan either.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

After checkign with Bzed he already had worked on c68172ef7f2d4f116078e2aba82986a8cab0b16e.
I added on top of his commit in the master branch what I had found so far and finally it can build with gcc 9.2.1.

=> https://github.com/bzed/pkg-open-vm-tools/pull/22

Waiting for Bzed to upload to Debian to sync it.

Changed in open-vm-tools (Ubuntu):
status: New → Triaged
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Was uploaded to Debian as planned and now synced.
Built in https://launchpad.net/ubuntu/+source/open-vm-tools/2:10.3.10-3 successfully now.

Waiting for the proposed-migration to hopefully now present other hickups.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package open-vm-tools - 2:10.3.10-3

---------------
open-vm-tools (2:10.3.10-3) unstable; urgency=medium

  [ Bernd Zeimetz ]
  * [19c646a] gcc9 compatibility.
    Upstream commit c68172ef7f2d4f116078e2aba82986a8cab0b16e (Closes: #925794)

  [ Christian Ehrhardt ]
  * [865763e] Fix other ftbfs with GCC-9
    * d/rules: disable address-of-packed-member gcc-9 warnings for pre 11.0 code
    (LP: #1842301)
    * d/rules: use modern syntax for disabling deprecated-declarations
    * d/p/gcc9-Remove-GLib-2.32-deprecated-APIs-from-tools.patch: stop using outdated GLib features
    Upstream commit a7c141fc
    * d/p/gcc9-drop-obsolete-G_INLINE_FUNC.patch: stop using deprecated GLib Macro
    * d/p/gcc9-GStaticRecMutex.patch: stop using deprecated GStaticRecMutex
    Upstream commit 19ca3e36
    * d/p/gcc9-build-error-in-vmblocktest.c.patch: avoid error due to stringop-truncation
    Upstream commit 553d1283

  [ Bernd Zeimetz ]
  * [0ce2ba2] Policy 4.0.1: The extra priority has been deprecated
  * [c8760c6] Bumping Standards-Version to 4.4.0
  * [a6ed8ce] Don't override dh_builddeb.
    debian-rules-should-not-use-custom-compression-settings
  * [bdfd8b5] Remove add_patch script
  * [be4d889] Update copyright years.
  * [9ac710e] Remove autotools-dev dependency.
  * [4296cf4] Fix permissions of udev rules file
  * [ed11c19] A new lintian override

 -- Bernd Zeimetz <email address hidden> Tue, 03 Sep 2019 18:06:54 +0200

Changed in open-vm-tools (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Christian, or anyone else affected,

Accepted open-vm-tools into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/open-vm-tools/2:11.0.1-2ubuntu0.19.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in open-vm-tools (Ubuntu Disco):
status: New → Fix Committed
tags: added: verification-needed verification-needed-disco
Changed in open-vm-tools (Ubuntu Bionic):
status: New → Fix Committed
tags: added: verification-needed-bionic
Revision history for this message
Robie Basak (racb) wrote :

Hello Christian, or anyone else affected,

Accepted open-vm-tools into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/open-vm-tools/2:11.0.1-2ubuntu0.18.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

That is only part of the SRU because of the compiler differences between our releases and due to the full version backport being part of the upload - Not really a change that was added/removed.
Hence this never got an SRU template on its own.

The verification is just about checking if it FTBFS on the backport.
https://launchpad.net/ubuntu/+source/open-vm-tools/2:11.0.1-2ubuntu0.19.04.2
https://launchpad.net/ubuntu/+source/open-vm-tools/2:11.0.1-2ubuntu0.18.04.2

Both built fine as expected, marking verified.

tags: added: verification-done verification-done-bionic verification-done-disco
removed: verification-needed verification-needed-bionic verification-needed-disco
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (4.3 KiB)

This bug was fixed in the package open-vm-tools - 2:11.0.1-2ubuntu0.19.04.2

---------------
open-vm-tools (2:11.0.1-2ubuntu0.19.04.2) disco; urgency=medium

  * d/p/lp-1855686-Avoid-vmtoolsd-crash-in-HostInfo.patch: fix crash with
    uncommon lsb_output behavior (LP: #1855686)

open-vm-tools (2:11.0.1-2ubuntu0.19.04.1) disco; urgency=medium

  * Update to latest release v11 (LP: #1844834)
    - d/rules: Revert "Enable all compiler errors again" for the older
      compiler

open-vm-tools (2:11.0.1-2) unstable; urgency=medium

  * [76c600f] Fix segfault for fs devices without /
    See https://github.com/vmware/open-vm-tools/issues/378 for details.
    Thanks to Mo Zhou (Closes: #942692)

open-vm-tools (2:11.0.1-1) unstable; urgency=medium

  * [bb36e10] Update upstream source from tag 'upstream/11.0.1'
    Update to upstream version '11.0.1'
    with Debian dir 60c0d512096774b9a2a7cc9e4e94556b2893ae8a

open-vm-tools (2:11.0.0-2) unstable; urgency=medium

  * [4cfe383] Update Vcs-Git/Browser to point to salsa.
  * [bc253ad] Remove .travis.yml, add debian/.gitlab-ci.yml
  * [c92ca3a] Add add_patch.sh script to add patches from upstream.
  * [1d9b491] Add patch to remove deprecated inline functions
  * [3e2e307] Rename lintian-override file properly

open-vm-tools (2:11.0.0-1) unstable; urgency=medium

  [ goldstar611 ]
  * [c138871] Ensure VGAuthService starts after AppArmor
    https://gitlab.com/apparmor/apparmor/issues/13

  [ Bernd Zeimetz ]
  * [28ef841] New upstream version 11.0.0~0
  * [f78ed2d] New upstream version 11.0.0
    Closes: #940853
  * [19efc80] Revert "Revert "Removing libdumbnet-dev.""
    This reverts commit 31177fab964d92687501ab81774440a9b8d09e39.
  * [bc14a8b] snapshot changelog
  * [1c5e9ea] Dropping patches that were picked from upstream

open-vm-tools (2:10.3.10-3) unstable; urgency=medium

  [ Bernd Zeimetz ]
  * [19c646a] gcc9 compatibility.
    Upstream commit c68172ef7f2d4f116078e2aba82986a8cab0b16e (Closes: #925794)

  [ Christian Ehrhardt ]
  * [865763e] Fix other ftbfs with GCC-9
    * d/rules: disable address-of-packed-member gcc-9 warnings for pre 11.0 code
    (LP: #1842301)
    * d/rules: use modern syntax for disabling deprecated-declarations
    * d/p/gcc9-Remove-GLib-2.32-deprecated-APIs-from-tools.patch: stop using outdated GLib features
    Upstream commit a7c141fc
    * d/p/gcc9-drop-obsolete-G_INLINE_FUNC.patch: stop using deprecated GLib Macro
    * d/p/gcc9-GStaticRecMutex.patch: stop using deprecated GStaticRecMutex
    Upstream commit 19ca3e36
    * d/p/gcc9-build-error-in-vmblocktest.c.patch: avoid error due to stringop-truncation
    Upstream commit 553d1283

  [ Bernd Zeimetz ]
  * [0ce2ba2] Policy 4.0.1: The extra priority has been deprecated
  * [c8760c6] Bumping Standards-Version to 4.4.0
  * [a6ed8ce] Don't override dh_builddeb.
    debian-rules-should-not-use-custom-compression-settings
  * [bdfd8b5] Remove add_patch script
  * [be4d889] Update copyright years.
  * [9ac710e] Remove autotools-dev dependency.
  * [4296cf4] Fix permissions of udev rules file
  * [ed11c19] A new lintian override

open-vm-tools (2:10.3.10-2) unstable; urgency=medium

  [ Christian Ehrhardt ]
  * [d79cc9d] d/...

Read more...

Changed in open-vm-tools (Ubuntu Disco):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for open-vm-tools has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (6.0 KiB)

This bug was fixed in the package open-vm-tools - 2:11.0.1-2ubuntu0.18.04.2

---------------
open-vm-tools (2:11.0.1-2ubuntu0.18.04.2) bionic; urgency=medium

  * d/p/lp-1855686-Avoid-vmtoolsd-crash-in-HostInfo.patch: fix crash with
    uncommon lsb_output behavior (LP: #1855686)

open-vm-tools (2:11.0.1-2ubuntu0.18.04.1) bionic; urgency=medium

  * Update to latest release v11 (LP: #1844834)
    - d/rules: Revert "Enable all compiler errors again" for the older
      compiler

open-vm-tools (2:11.0.1-2) unstable; urgency=medium

  * [76c600f] Fix segfault for fs devices without /
    See https://github.com/vmware/open-vm-tools/issues/378 for details.
    Thanks to Mo Zhou (Closes: #942692)

open-vm-tools (2:11.0.1-1) unstable; urgency=medium

  * [bb36e10] Update upstream source from tag 'upstream/11.0.1'
    Update to upstream version '11.0.1'
    with Debian dir 60c0d512096774b9a2a7cc9e4e94556b2893ae8a

open-vm-tools (2:11.0.0-2) unstable; urgency=medium

  * [4cfe383] Update Vcs-Git/Browser to point to salsa.
  * [bc253ad] Remove .travis.yml, add debian/.gitlab-ci.yml
  * [c92ca3a] Add add_patch.sh script to add patches from upstream.
  * [1d9b491] Add patch to remove deprecated inline functions
  * [3e2e307] Rename lintian-override file properly

open-vm-tools (2:11.0.0-1) unstable; urgency=medium

  [ goldstar611 ]
  * [c138871] Ensure VGAuthService starts after AppArmor
    https://gitlab.com/apparmor/apparmor/issues/13

  [ Bernd Zeimetz ]
  * [28ef841] New upstream version 11.0.0~0
  * [f78ed2d] New upstream version 11.0.0
    Closes: #940853
  * [19efc80] Revert "Revert "Removing libdumbnet-dev.""
    This reverts commit 31177fab964d92687501ab81774440a9b8d09e39.
  * [bc14a8b] snapshot changelog
  * [1c5e9ea] Dropping patches that were picked from upstream

open-vm-tools (2:10.3.10-3) unstable; urgency=medium

  [ Bernd Zeimetz ]
  * [19c646a] gcc9 compatibility.
    Upstream commit c68172ef7f2d4f116078e2aba82986a8cab0b16e (Closes: #925794)

  [ Christian Ehrhardt ]
  * [865763e] Fix other ftbfs with GCC-9
    * d/rules: disable address-of-packed-member gcc-9 warnings for pre 11.0 code
    (LP: #1842301)
    * d/rules: use modern syntax for disabling deprecated-declarations
    * d/p/gcc9-Remove-GLib-2.32-deprecated-APIs-from-tools.patch: stop using outdated GLib features
    Upstream commit a7c141fc
    * d/p/gcc9-drop-obsolete-G_INLINE_FUNC.patch: stop using deprecated GLib Macro
    * d/p/gcc9-GStaticRecMutex.patch: stop using deprecated GStaticRecMutex
    Upstream commit 19ca3e36
    * d/p/gcc9-build-error-in-vmblocktest.c.patch: avoid error due to stringop-truncation
    Upstream commit 553d1283

  [ Bernd Zeimetz ]
  * [0ce2ba2] Policy 4.0.1: The extra priority has been deprecated
  * [c8760c6] Bumping Standards-Version to 4.4.0
  * [a6ed8ce] Don't override dh_builddeb.
    debian-rules-should-not-use-custom-compression-settings
  * [bdfd8b5] Remove add_patch script
  * [be4d889] Update copyright years.
  * [9ac710e] Remove autotools-dev dependency.
  * [4296cf4] Fix permissions of udev rules file
  * [ed11c19] A new lintian override

open-vm-tools (2:10.3.10-2) unstable; urgency=medium

  [ Christian Ehrhardt ]
  * [d79cc9d] ...

Read more...

Changed in open-vm-tools (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers