Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Mon, 20 Sep 2021 10:26:21 -0700 | Subject | Re: [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again |
| |
On Mon, Sep 20, 2021 at 5:12 AM Arnd Bergmann <arnd@kernel.org> wrote: > > + /* > + * work around gcc-11.x using the first field it observes > + * to determing the actual length > + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 > + */ > + char __empty[0]; > + char de_name[];
Ugh. That looks _really_ hacky.
It sounds like we can avoid the gcc bug if we just always use "de->de_name[]". Then we don't need to depend on magical behavior about one particular gcc version and a strange empty array in front of it.
IOW, something like the attached simpler thing that just does that "always use de_name[]" and has a comment about why we don't do the natural thing
Also, just what version of gcc is the broken one? You say "gcc-11", but I certainly don't see it with _my_ version of gcc-11, so can we (just for that comment) document more precisely what version you have (or possibly what config you use to trigger it).
Linus fs/qnx4/dir.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c index 2a66844b7ff8..d60806efe090 100644 --- a/fs/qnx4/dir.c +++ b/fs/qnx4/dir.c @@ -20,12 +20,24 @@ * depending on the status field in the last byte. The * first byte is where the name start either way, and a * zero means it's empty. + * + * Also, due to a bug in gcc, we don't want to use the + * real (differently sized) name arrays in the inode and + * link entries, but always the 'de_name[]' one in the + * fake struct entry. + * + * See + * + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c6 + * + * for details - but basically gcc will take the size of + * the 'name' array from one of the used entries randomly. */ union qnx4_directory_entry { struct { - char de_name; - char de_pad[62]; - char de_status; + const char de_name[48]; + u8 de_pad[15]; + u8 de_status; }; struct qnx4_inode_entry inode; struct qnx4_link_info link; @@ -53,29 +65,26 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx) ix = (ctx->pos >> QNX4_DIR_ENTRY_SIZE_BITS) % QNX4_INODES_PER_BLOCK; for (; ix < QNX4_INODES_PER_BLOCK; ix++, ctx->pos += QNX4_DIR_ENTRY_SIZE) { union qnx4_directory_entry *de; - const char *name; offset = ix * QNX4_DIR_ENTRY_SIZE; de = (union qnx4_directory_entry *) (bh->b_data + offset); - if (!de->de_name) + if (!de->de_name[0]) continue; if (!(de->de_status & (QNX4_FILE_USED|QNX4_FILE_LINK))) continue; if (!(de->de_status & QNX4_FILE_LINK)) { size = sizeof(de->inode.di_fname); - name = de->inode.di_fname; ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1; } else { size = sizeof(de->link.dl_fname); - name = de->link.dl_fname; ino = ( le32_to_cpu(de->link.dl_inode_blk) - 1 ) * QNX4_INODES_PER_BLOCK + de->link.dl_inode_ndx; } - size = strnlen(name, size); + size = strnlen(de->de_name, size); QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, name)); - if (!dir_emit(ctx, name, size, ino, DT_UNKNOWN)) { + if (!dir_emit(ctx, de->de_name, size, ino, DT_UNKNOWN)) { brelse(bh); return 0; } | |