Bug #14102
closedcpcgen fails due to 64-bit inodes on NFS
100%
Description
I attempted to compile illumos from an NFS mounted directory today. I ran into a failure coming from cpcgen
that stated
cpcgen: no platforms found or matched
After taking a look at the source code I found that this loop was only finding the dir entrires .
and ..
for the directory usr/src/data/amdpc even though this directory contains two json files. Because of this cpcgen_maps
would remain null causing cpcgen
to kick back a error and derail the build.
To try and narrow down the issue I created the following minimum reproducible program.
#include <sys/types.h>
#include <dirent.h>
#include <stdio.h>
void run() {
DIR *dir = opendir(".");
if (dir == NULL)
printf("could not open current dir\n");
struct dirent *d;
while (d = readdir(dir))
printf("%s\n", d->d_name);
}
When compiled as a 64-bit binary, this program sees all the expected files in the amdpc
directory referenced above. However, when compiled as a 32-bit binary, it only sees the ..
and .
entries. Peeling back another layer to look at readdir, I immediately noticed that someone had encountered this pain and solved it in the function that directly follows readdir64.
Modifying the above program to the following shows that using readdir64
instead of readdir
does produce the expected result from a 32 bit binary on a 64 bit system.
#include <sys/types.h>
#include <dirent.h>
#include <stdio.h>
void run() {
DIR *dir = opendir(".");
if (dir == NULL)
printf("could not open current dir\n");
struct dirent *d;
while (d = readdir(dir))
printf("%s\n", d->d_name);
}
void run64() {
DIR *dir = opendir(".");
if (dir == NULL)
printf("could not open current dir\n");
struct dirent64 *d;
while (d = readdir64(dir))
printf("%s\n", d->d_name);
}
int main() {
printf("run\n");
run();
printf("run64\n");
run64();
return 0;
}
ry@masaka:/kdev/illumos/usr/src/data/amdpmc$ /kdev/readdir-fail/readdir-fail32 run . .. run64 . .. f19h_zen3_core.json f17h_zen2_core.json .make.state Makefile f17h_zen1_core.json README
The following patch allows a build from an NFS mounted directory to get further on my system.
diff --git a/usr/src/tools/cpcgen/cpcgen.c b/usr/src/tools/cpcgen/cpcgen.c
index 779a782cc6..320cba52f7 100644
--- a/usr/src/tools/cpcgen/cpcgen.c
+++ b/usr/src/tools/cpcgen/cpcgen.c
@@ -642,7 +642,7 @@ static void
cpcgen_read_amd(const char *datadir, const char *platform)
{
DIR *dir;
- struct dirent *d;
+ struct dirent64 *d;
const char *suffix = ".json";
const size_t slen = strlen(suffix);
@@ -650,7 +650,7 @@ cpcgen_read_amd(const char *datadir, const char *platform)
err(EXIT_FAILURE, "failed to open directory %s", datadir);
}
- while ((d = readdir(dir)) != NULL) {
+ while ((d = readdir64(dir)) != NULL) {
char *name, *c;
cpc_map_t *map;
nvlist_t *parsed;
However, I'm not sure if this would break things on an actual 32 bit system.
Updated by Robert Mustacchi 8 months ago
So the purpose of readdir64
isn't 32-bit vs. 64-bit binaries exactly, but rather the 64-bit file offsets and related, which are the default on the LP64 ABI. Historically this is done by most programs that need to access 64-bit offsets to use the compilation macros that allow this. While we aren't accessing 64-bit offsets by any means, I guess that we've ended up in the place where somehow we've ended up getting 64-bit inodes from the server, though I'm not sure how that leads to the readdir failure. If we need 64-bit file offsets we should use the large file source macros to do this rather than changing the one call site so that way future changes to this program can deal with that.
I can put something together to do to that; however, it'd help to understand why exactly readdir is failing in this case as I wouldn't expect that. Is errno set in the case where we get NULL?
Updated by Ryan Goodfellow 8 months ago
Yes the errno does get set. The result is
errno: 79 perror: Value too large for defined data type
Updated by Robert Mustacchi 8 months ago
OK, then this is pretty clearly the check in the ILP32 readdir around the inode or offset. So we probably have a 64-bit inode. We should just fix this. I've wsdiff'd a fix to this.
Updated by Robert Mustacchi 8 months ago
- Assignee changed from Ryan Goodfellow to Robert Mustacchi
- Subject changed from cpcgen fails to iterate NFS mounted directory on 64 bit systems to cpcgen fails due to 64-bit inodes on NFS
Updated by Robert Mustacchi 8 months ago
Just to add a bit more context, what's happening here is we encountered a 64-bit indoe, which caused a normal readdir to fail. As such I went through and added the classic LFS flags to this. With this in place I did the following testing:
- Did a wsdiff across both versions, including all the intermediate generated files. These all came back clean.
- Worked with Ryan to confirm that the updated versions works in his environment that was generating this.
Updated by Electric Monk 8 months ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 1b09309c5ebed5c0bf14e2b396bf626c5aa30034
commit 1b09309c5ebed5c0bf14e2b396bf626c5aa30034 Author: Robert Mustacchi <rm@fingolfin.org> Date: 2021-09-27T14:57:53.000Z 14102 cpcgen fails due to 64-bit inodes on NFS Reviewed by: Joshua M. Clulow <josh@sysmgr.org> Reviewed by: Peter Tribble <peter.tribble@gmail.com> Reviewed by: Ryan Goodfellow <ryan.goodfellow@oxide.computer> Approved by: Richard Lowe <richlowe@richlowe.net>