Modify

Ticket #6659 (closed Bugs: fixed)

Opened 2 years ago

Last modified 22 months ago

Filesystem compilation broken on Solaris 9 and 10

Reported by: Vasily Sukhanov <basil@…> Owned by: bemandawes
Milestone: To Be Determined Component: filesystem
Version: Boost 1.49.0 Severity: Regression
Keywords: compilation Cc: duncanphilipnorman@…, chrmhoffmann@…

Description

Output message:

libs/filesystem/v3/src/operations.cpp: In function âvoid boost::filesystem3::detail::permissions(const boost::filesystem3::path&, boost::filesystem3::perms, boost::system::error_code*)â: libs/filesystem/v3/src/operations.cpp:1391:11: error: â::fchmodatâ has not been declared

This is regression, 1.48 compiles well. I reproduced on both sparc and x86 platforms.

Attachments

boost-1.49.0-dont_use_fchmodat.patch Download (1.0 KB) - added by Duncan Exon Smith <duncanphilipnorman@…> 2 years ago.
Patch to remove unnecessary use of poorly supported fchmodat().
test-fchmodat.cpp Download (727 bytes) - added by Duncan Exon Smith <duncanphilipnorman@…> 22 months ago.
Test program to demonstrate that Linux does not support AT_SYMLINK_NOFOLLOW.
linux-no_fchmod_at-r79468.patch Download (1.3 KB) - added by Duncan Exon Smith <duncanphilipnorman@…> 22 months ago.
Patch to stop using fchmodat() on Linux, with an explanation in the comments.

Change History

comment:1 follow-up: ↓ 3 Changed 2 years ago by bemandawes

  • Status changed from new to assigned

Here is the code:

Mac OS X Lion and some other platforms don't support fchmodat()

# if defined(AT_FDCWD) && defined(AT_SYMLINK_NOFOLLOW) \

&& (!defined(SUNPRO_CC)
SUNPRO_CC > 0x5100)

if (::fchmodat(AT_FDCWD, p.c_str(), mode_cast(prms),

!(prms & symlink_perms) ? 0 : AT_SYMLINK_NOFOLLOW))

# else fallback if fchmodat() not supported

if (::chmod(p.c_str(), mode_cast(prms)))

# endif

I'm guessing the SUNPRO_CC code is wrong. If you could submit a tested patch, I'd appreciate it.

Thanks,

--Beman

comment:2 Changed 2 years ago by Vasily Sukhanov <basil@…>

SUNPRO_CC has no relation to my case. I had to write that I am using GCC 4.5.3. I didn't find this function in Solaris headers. Documentation ( http://docs.oracle.com/cd/E19963-01/html/821-1463/chmod-2.html) says fchmodat is present in Solaris 11.

comment:3 in reply to: ↑ 1 Changed 2 years ago by Duncan Exon Smith <duncanphilipnorman@…>

  • Cc duncanphilipnorman@… added

Just a note that I ran into this on Linux, albeit in a strange circumstance.

  • We develop on modern Linux, but since we support RHEL4, we have some builds that link against RHEL4 glibc (and pthread, etc.).
    • Perhaps we should build Boost on RHEL4 and link against that, but we're lazy, and use the same build of Boost for all of our Linux builds.
  • To others with the same issue: we're planning to work around this by modifying the same code Beman mentions (in operations.cpp) to always use the fallback.

Replying to bemandawes:

Here is the code:

Beman, I think you broke the Trac renderer (it's adding funny italics because of the // comments). Perhaps you'd be willing to edit your comment:

  • If you add a line preceding the code snippet that contains just {{{ and a line following the snippet that contains just }}}, the renderer will do the right thing.
  • Bonus: if you add a line containing just #!cpp to the top of the code snippet, you'll get C++ highlighting.

comment:4 follow-up: ↓ 5 Changed 2 years ago by Duncan Exon Smith <duncanphilipnorman@…>

Another note: AT_SYMLINK_NOFOLLOW does not appear to be supported even in Linux.

  • See  http://www.kernel.org/doc/man-pages/online/pages/man2/fchmodat.2.html
  • Quote from flags description:
          AT_SYMLINK_NOFOLLOW
                 If pathname is a symbolic link, do not dereference it: instead operate
                 on the link itself.  This flag is not currently implemented.
    
  • Quote from ERRORS:
          ENOTSUP
                 flags specified AT_SYMLINK_NOFOLLOW, which is not supported.
    
  • Perhaps it's premature to use fchmodat(), especially since its main feature -- acting relative to dirfd -- is not even being used.

Changed 2 years ago by Duncan Exon Smith <duncanphilipnorman@…>

Patch to remove unnecessary use of poorly supported fchmodat().

comment:5 in reply to: ↑ 4 Changed 2 years ago by Duncan Exon Smith <duncanphilipnorman@…>

Replying to Duncan Exon Smith <duncanphilipnorman@…>:

Another note: AT_SYMLINK_NOFOLLOW does not appear to be supported even in Linux.

I've added attachment:boost-1.49.0-dont_use_fchmodat.patch Download as one way of fixing these problems (as well as the original problem on Solaris), although I admit it may be heavy-handed.

comment:6 follow-up: ↓ 7 Changed 22 months ago by Vasily Sukhanov <basil@…>

Why wasn't this patch integrated into boost 1.50?

comment:7 in reply to: ↑ 6 Changed 22 months ago by Duncan Exon Smith <duncanphilipnorman@…>

Replying to Vasily Sukhanov <basil@…>:

Why wasn't this patch integrated into boost 1.50?

I don't know. Beman?

comment:8 Changed 22 months ago by anonymous

In the Solaris case, testing the value of SUNPRO_CC won't tell us if fchmodat is supported. For that we need to test whether the system is Solaris 11 or not. The following will test for Solaris 11 if you are using Solaris Studio compilers

defined(__SUNPRO_CC) && defined(__SunOS_5_11)

comment:9 Changed 22 months ago by Vasily Sukhanov <basil@…>

This is a narrow case. I'm using gcc.

Changed 22 months ago by Duncan Exon Smith <duncanphilipnorman@…>

Test program to demonstrate that Linux does not support AT_SYMLINK_NOFOLLOW.

comment:10 Changed 22 months ago by chrmhoffmann@…

  • Cc chrmhoffmann@… added

comment:11 follow-up: ↓ 12 Changed 22 months ago by bemandawes

Test_fchmodat.cpp was quite helpful. It runs without asserts a fresh install of Ubuntu 12.04.

The problem with the patch is that, as you said, it is too heavy handed. It looses too much information, both in comments and in code.

On Linux, I'd prefer to try fchmodat with AT_SYMLINK_NOFOLLOW and then if it fails with ENOTSUP, try again with 0 instead of AT_SYMLINK_NOFOLLOW. That way the code will work correctly if Linux ever fixes AT_SYMLINK_NOFOLLOW. A comment is needed to explain why the code is written that way, too.

I'd prefer you rework the patch, test it, and resubmit.

Thanks,

--Beman

comment:12 in reply to: ↑ 11 Changed 22 months ago by Duncan Exon Smith <duncanphilipnorman@…>

Replying to bemandawes:

I'd prefer you rework the patch, test it, and resubmit.

I've put together another patch that avoids fchmodat() on Linux (but only Linux, this time). Let me argue why:

  • As documented in  http://man7.org/linux/man-pages/man7/symlink.7.html, Linux does not support changing permissions on symbolic links (look for the section "Symbolic link ownership, permissions, and timestamps"). Based on the documentation, I can't believe they plan to support this in the future.
  • I think you're right that if support for AT_SYMLINK_NOFOLLOW were coming to fchmodat(), it would be best to "future proof" by testing for support programatically (check for ENOTSUP and recall chmod()).
  • However, since the limitation of fchmodat() is caused by an operating system limitation that's unlikely to change, I think it's better to just avoid the issue entirely.
  • Let me know what you think.

This version of the patch (attachment:linux-no_fchmod_at-r79468.patch Download) applies cleanly to the trunk at r79568.

Finally, I don't have access to a Solaris system for testing.

  • If someone else can figure out the preprocessor code necessary to test for Solaris 11, please add it to the patch. I suspect you'll need some sort of sun-specific include.
  • Alternatively, if someone on a Solaris 11 or 12 box could run attachment:test-fchmodat.cpp Download to determine if AT_SYMLINK_NOFOLLOW is even supported, perhaps the fix suggested in #7051 would be appropriate. In that case a similar approach to the one I argue for above for Linux probably makes sense for Solaris as well (I think you'll just need to add || defined(sun) || defined(__sun) to my list, as well as some supporting documentation in the comments).

Changed 22 months ago by Duncan Exon Smith <duncanphilipnorman@…>

Patch to stop using fchmodat() on Linux, with an explanation in the comments.

comment:13 Changed 22 months ago by bemandawes

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [79484]) Fix #6659 and #7051, fchmodat supported only on Solaris 11. Disable fchmodat for both Sun and GCC compilers regardless of OS version; a runtime check is too much trouble.

comment:14 Changed 22 months ago by anonymous

Hi,

Shouldn't this be handled in the config.hpp rather than with ifdefs in the cpp code itself?

Otherwise thanks for looking into this.

Rgds, Chris

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.