Project

General

Profile

Actions

Bug #14102

closed

cpcgen fails due to 64-bit inodes on NFS

Added by Ryan Goodfellow about 1 month ago. Updated 28 days ago.

Status:
Closed
Priority:
Normal
Category:
tools - gate/build tools
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
cpcgen NFS
Gerrit CR:

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.

Actions #1

Updated by Robert Mustacchi about 1 month 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?

Actions #2

Updated by Ryan Goodfellow about 1 month ago

Yes the errno does get set. The result is

errno: 79
perror: Value too large for defined data type

Actions #3

Updated by Robert Mustacchi about 1 month 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.

Actions #4

Updated by Robert Mustacchi about 1 month 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
Actions #5

Updated by Robert Mustacchi about 1 month 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.
Actions #6

Updated by Robert Mustacchi about 1 month ago

  • Gerrit CR set to 1728
Actions #7

Updated by Electric Monk 28 days 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>

Actions

Also available in: Atom PDF