Skip to content

Conversation

rokuyama
Copy link

Forcibly enable use64bitint for NetBSD 4.0 and later, where ino_t is uint64_t.

Otherwise, miniperl cannot open files whose inode numbers are not within
32-bit integer. This results in random build failures.

Forcibly enable use64bitint for NetBSD 4.0 and later, where ino_t is uint64_t.

Otherwise, miniperl cannot open files whose inode numbers are not within
32-bit integer. This results in random build failures.
@jkeenan
Copy link
Contributor

jkeenan commented May 12, 2021

Forcibly enable use64bitint for NetBSD 4.0 and later, where ino_t is uint64_t.

Otherwise, miniperl cannot open files whose inode numbers are not within
32-bit integer. This results in random build failures.

I very much doubt that we should adopt the approach we are advocating.

I don't claim any particular expertise with NetBSD. I'm more familiar with FreeBSD and OpenBSD. But we have a long history of successfully smoke-testing Perl 5 blead on NetBSD in a variety of configurations without specifying anything one way or the other about -Duse64bitint. See, e.g., https://perl5.test-smoke.org/report/5005449.

For you to convince us we would need some more evidence of the miniperl failure you claim. We would also need some references to the official NetBSD documentation where this problem is discussed.

Thank you very much.
Jim Keenan

@rokuyama
Copy link
Author

Thank you very much for your quick reply.

The problem is tracked as pkg/55997 for NetBSD:
http://gnats.netbsd.org/55997

Perl successfully builds on NetBSD for most cases; for LP64 architectures, and for ILP32 architectures with real disk drives. For ILP32 architectures, this is because inode number is normally within 32-bit integer for real storage. However, typically for tmpfs on COMPAT_NETBSD32 (*; see below), inode number larger than UINT32_MAX is assigned to a file frequently. This case, miniperl built for ILP32 without -Duse64bitint fails to open that file like:

LD_LIBRARY_PATH=/build/pkgsrc/lang/perl5/work.earmv6/perl-5.32.1 ./miniperl -Ilib make_ext.pl lib/auto/Encode/Encode.so  MAKE="/usr/bin/make" LIBPERL_A=libperl.so LINKTYPE=dynamic
ERROR from evaluation of /build/pkgsrc/lang/perl5/work.earmv6/perl-5.32.1/cpan/Encode/Byte/Makefile.PL: Can't locate File/Spec/Functions.pm in @INC (you may need to install the File::Spec::Functions module) (@INC contains: / / /.extract_makevars.mk/cv.h/PathTools /.extract_makevars.mk/cv.h/PathTools/lib / /.extract_makevars.mk/charclass_invlists.h/ExtUtils-MakeMaker/lib /.extract_makevars.mk/charclass_invlists.h/ExtUtils-Manifest/lib / /.extract_makevars.mk/embedvar.h/re / / / / / / /.extract_makevars.mk/charclass_invlists.h/Filter-Util-Call/lib / /.extract_makevars.mk/lib /.extract_makevars.mk/charclass_invlists.h/Encode .) at ./Makefile.PL line 4.
BEGIN failed--compilation aborted at ./Makefile.PL line 4.
Unsuccessful Makefile.PL(cpan/Encode): code=512 at make_ext.pl line 536.
*** [lib/auto/Encode/Encode.so] Error code 2

(*) NetBSD provides COMPAT_NETBSD32 framework, that enables to run ILP32 userland on LP64 kernel.

@oodler577
Copy link

oodler577 commented May 12, 2021

use64bitint is mandatory for NetBSD 4.0 and later, where ino_t is uint64_t.

What happens when perl is being built on a 32 bit architecture under netbsd with this change? Would it be "nothing"? Just thought I'd see some check on the actual architecture. NetBSD 4.0 is from 2008, so it's reasonable to consider this case.

@rokuyama
Copy link
Author

With this patch, Perl is configured as if -Duse64bitint is specified. Nothing changes if at least one of -Duse64bitint, -Dusemorebits, or -Duse64bitall options are already specified. There should be no penalty except for small overhead and increase in binary size for ILP32 architectures.

Although -Duse64bitint and friends have never been specified by default for NetBSD package system, I've personally enabled -Duse64bitint option over a year for ARM, PowerPC, and m68k. And there is no fallout from it as far as I can see.

Please tell me how, if I can/should do some regression tests.

@tonycoz
Copy link
Contributor

tonycoz commented May 12, 2021

I'm curious about what's going wrong at the system level, is stat() failing?

-Duselargefiles (typically on by default) should be preventing that, and pp_stat should be handling large inode numbers sanely (either storing as a double if they're exactly representable, or as a string).

The strange @inc is the problem, and I've reproduced it here, for anyone else:

  • check /tmp is tmpfs
  • check you have 64-bit NetBSD and plenty of swap or memory (I had 1G swap + RAM and ran out, expanding memory to 8GB let me build)
  • clone/extract the perl sources under /tmp somewhere
  • configure with: -Dcc='cc -m32' -Dld='cc -m32'
  • try to build

@rokuyama
Copy link
Author

Thank you very much for reproducing the problem as well as kind explanation.

As you pointed out, a large inode numbers are stored as strings. However, they are compared as integers here:

https://github.com/Perl/perl5/blob/blead/dist/PathTools/Cwd.pm#L278

perl5/dist/PathTools/Cwd.pm 
278	last if $odev == $cdev && $oino == $cino;

This is impossible for ILP32 architectures when use64bitint is disabled. As a minimal test case, this does not work properly:

#!/usr/bin/env perl

$a = "82350279496700219";
$b = "82350279496700220";

if ($a == $b) {
        print "Equal\n";
} else {
        print "Not equal\n";
}

Here, values of $a and $b are taken from inode numbers observed for tmpfs.

@tonycoz
Copy link
Contributor

tonycoz commented May 13, 2021

The issue is that this isn't just a NetBSD issue - other systems have large inode numbers too:

# Linux subsystem on windows
tony@GANYMEDE:~$ stat .
  File: .
  Size: 4096            Blocks: 0          IO Block: 4096   directory
Device: 2h/2d   Inode: 562949953975388  Links: 1
Access: (0755/drwxr-xr-x)  Uid: ( 1000/    tony)   Gid: ( 1000/    tony)
Access: 2019-11-12 17:04:20.387599800 +1100
Modify: 2021-01-04 15:47:46.947753400 +1100
Change: 2021-01-04 15:47:46.947753400 +1100
 Birth: -

So we need to fix this generally, not just for NetBSD.

The simplest fix is for Cwd (and File::Find and Porting/bisect.pl, and probably a lot of CPAN, including IO::Async::File, AnyEvent, but not File::Path, File::Temp) to start using eq instead of == for comparing inode numbers.

Another solution might be for us to return a Math::BigInt or some other overloaded object for large inode numbers (or for all inode numbers to avoid surprises),

@rokuyama
Copy link
Author

Thank you for your comment.

At least Perl successfully builds on tmpfs with this patch:

https://gist.github.com/rokuyama/499f7bd1c02630d1e0859ebf020d54ba

by which inode numbers are compared as strings.

tonycoz added a commit to tonycoz/ExtUtils-MakeMaker that referenced this pull request May 17, 2021
On a 32-bit system with 64-bit inodes, recent-ish versions of perl
will return very large inode numbers as strings, and their numeric
value may be too large to be exactly represented as an NV.

This means comparing such inodes numerically may return them as equal
even when the actual inode number is different.

See Perl/perl5#18788 for a case where this
has caused a problem.
@tonycoz
Copy link
Contributor

tonycoz commented May 24, 2021

Still need the MM changes

@tonycoz tonycoz reopened this May 24, 2021
bingos pushed a commit to Perl-Toolchain-Gang/ExtUtils-MakeMaker that referenced this pull request May 25, 2021
On a 32-bit system with 64-bit inodes, recent-ish versions of perl
will return very large inode numbers as strings, and their numeric
value may be too large to be exactly represented as an NV.

This means comparing such inodes numerically may return them as equal
even when the actual inode number is different.

See Perl/perl5#18788 for a case where this
has caused a problem.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 29, 2021
PR pkg/55997

Internal stat() function for perl stores inode number as string, if it
cannot be represented by host's integer. However, unfortunately, some
components compare them as integer.

Therefore, if 64-bit integers are not supported, files cannot be handled,
whose inode number is larger than UINT32_MAX.

Usually, inode numbers on real filesystems are well below UINT32_MAX. But,
inode numbers larger than UINT32_MAX are assigned for tmpfs on LP64 kernels.
This results in build failures for perl on COMPAT_NETBSD32 if working
directory is tmpfs, and perl-64bitint and friends are not specified.

Now, inode numbers are compared as string, which works just fine even if
64-bit integers are not supported.

Cherry-picked from upstream. See Perl/perl5#18788
and related pull-requests for more details.
@jkeenan
Copy link
Contributor

jkeenan commented Jun 20, 2021

Still need the MM changes

Am I correct in thinking that if an ExtUtils-MakeMaker 7.63 is released to CPAN, we can then proceed with this p.r.?

Thank you very much.
Jim Keenan

@tonycoz tonycoz added the do not merge Don't merge this PR, at least for now label Jun 21, 2021
@tonycoz
Copy link
Contributor

tonycoz commented Jun 21, 2021

Still need the MM changes

Am I correct in thinking that if an ExtUtils-MakeMaker 7.63 is released to CPAN, we can then proceed with this p.r.?

No, this PR is obsoleted by the #18798 and MM 7.63 to-be changes.

This PR is effectively now an issue, but github has no mechanism to convert a PR to an issue.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 21, 2021

Still need the MM changes

Am I correct in thinking that if an ExtUtils-MakeMaker 7.63 is released to CPAN, we can then proceed with this p.r.?

No, this PR is obsoleted by the #18798 and MM 7.63 to-be changes.

This PR is effectively now an issue, but github has no mechanism to convert a PR to an issue.

Can you or @rokuyama or someone else propose some language to open a new issue about this?

@hvds
Copy link
Contributor

hvds commented Mar 3, 2022

I've pulled all the information I can discover into #19489, so I think this PR can be closed now. I'll do that in a day or two if nobody dissents.

@jkeenan
Copy link
Contributor

jkeenan commented Mar 3, 2022

I've pulled all the information I can discover into #19489, so I think this PR can be closed now. I'll do that in a day or two if nobody dissents.

What specific steps, if any, should we then take with respect to NetBSD?

@hvds
Copy link
Contributor

hvds commented Mar 3, 2022

I've pulled all the information I can discover into #19489, so I think this PR can be closed now. I'll do that in a day or two if nobody dissents.

What specific steps, if any, should we then take with respect to NetBSD?

No specific steps should be required, it is a general problem; but see also above where "netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue on May 29, 2021" - I expect that resolved the immediate problem that prompted this PR in the first place. As Tony said, "this PR is effectively now an issue, but github has no mechanism to convert a PR to an issue".

@hvds
Copy link
Contributor

hvds commented Mar 5, 2022

Closing per discussion; thanks @rokuyama for bringing this to our attention.

@hvds hvds closed this Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

distro-netbsd do not merge Don't merge this PR, at least for now

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants