Re: [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 2210 bytes --]

 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;
 			}