xfsprogs: 4.14 rollup

PATCH In review Darrick J. Wong 4 participants 29 messages Oct 26, 2017 – Oct 27, 2017

lore

Share & tools

lore

Darrick J. Wong

author

2017-10-26 22:14 UTC #0 lore raw

cover xfsprogs: 4.14 rollup

Hi all,

Here is a rollup of everything I still have queued for xfsprogs 4.14. This rollup supersedes all the previous xfsprogs patches I've sent, though the first three patches in this series haven't changed since then.

The first six patches fix various bugs I found while testing metadump and repair. The middle three patches enable developers to turn on the undefined behavior sanitizer, the address sanitizer, and the thread sanitizer that are now in gcc. The last patch fixes some ubsan problems.

--D

Darrick J. Wong

author

2017-10-26 22:15 UTC #1 lore raw

PATCH 1/10 db: increase metadump's default overly long extent discard threshold

From: Darrick J. Wong <[email protected]>

Back in 88b8e1d6d7 ("Make xfs_metadump more robust against bad data"), metadump grew the ability to ignore a directory extent if it was longer than 20 blocks. Presumably this was to protect metadump from dumping absurdly long extents resulting from bmbt corruption, but it's certainly possible to create a directory with an extent longer than 20 blocks. Hilariously, the discards happen with no warning unless the caller explicitly set -w.

This was raised to 1000 blocks in 7431d134fe8 ("Increase default maximum extent size for xfs_metadump when copying..."), but it's still possible to create a directory with an extent longer than 1000 blocks.

Increase the threshold to MAXEXTLEN blocks because it's totally valid for the filesystem to create extents up to that length.

Signed-off-by Darrick J. Wong <[email protected]>

Reviewed-by Carlos Maiolino <[email protected]>

---
 db/metadump.c           |    2 +-
 man/man8/xfs_metadump.8 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

db/metadump.c +1 -1

index 6dd06c3..8ffb90f 100644
@@ -32,7 +32,7 @@
 #include "field.h"
 #include "dir2.h"
 
-#define DEFAULT_MAX_EXT_SIZE	1000
+#define DEFAULT_MAX_EXT_SIZE	MAXEXTLEN
 
 /*
  * It's possible that multiple files in a directory (or attributes

man/man8/xfs_metadump.8 +1 -1

index 3731d6a..7207c20 100644
@@ -114,7 +114,7 @@ copied.
 .B \-m
 Set the maximum size of an allowed metadata extent.  Extremely large metadata
 extents are likely to be corrupt, and will be skipped if they exceed
-this value.  The default size is 1000 blocks.
+this value.  The default size is 2097151 blocks.
 .TP
 .B \-o
 Disables obfuscation of file names and extended attributes.

Eric Sandeen

2017-10-27 00:03 UTC #2 lore raw

Re: [PATCH 01/10] db: increase metadump's default overly long extent discard threshold

Quoted 14 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Back in 88b8e1d6d7 ("Make xfs_metadump more robust against bad data"), metadump grew the ability to ignore a directory extent if it was longer than 20 blocks. Presumably this was to protect metadump from dumping absurdly long extents resulting from bmbt corruption, but it's certainly possible to create a directory with an extent longer than 20 blocks. Hilariously, the discards happen with no warning unless the caller explicitly set -w.
This was raised to 1000 blocks in 7431d134fe8 ("Increase default maximum extent size for xfs_metadump when copying..."), but it's still possible to create a directory with an extent longer than 1000 blocks.

Quoted 5 lines

Increase the threshold to MAXEXTLEN blocks because it's totally valid for the filesystem to create extents up to that length.
Signed-off-by Darrick J. Wong <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>

Thanks, I'll pull this in.

Should we fix the noisiness so it doesn't require -w for warnings as well?

-Eric

Quoted 38 lines

---
 db/metadump.c           |    2 +-
 man/man8/xfs_metadump.8 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

db/metadump.c +1 -1

index 6dd06c3..8ffb90f 100644
@@ -32,7 +32,7 @@
 #include "field.h"
 #include "dir2.h"
 
-#define DEFAULT_MAX_EXT_SIZE	1000
+#define DEFAULT_MAX_EXT_SIZE	MAXEXTLEN
 
 /*
  * It's possible that multiple files in a directory (or attributes

man/man8/xfs_metadump.8 +1 -2

index 3731d6a..7207c20 100644
@@ -114,7 +114,7 @@ copied.
 .B \-m
 Set the maximum size of an allowed metadata extent.  Extremely large metadata
 extents are likely to be corrupt, and will be skipped if they exceed
-this value.  The default size is 1000 blocks.
+this value.  The default size is 2097151 blocks.
 .TP
 .B \-o
 Disables obfuscation of file names and extended attributes.
 
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 00:12 UTC #3 lore raw

Re: [PATCH 01/10] db: increase metadump's default overly long extent discard threshold

Quoted 18 lines · Eric Sandeen

Quoted 17 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Back in 88b8e1d6d7 ("Make xfs_metadump more robust against bad data"), metadump grew the ability to ignore a directory extent if it was longer than 20 blocks. Presumably this was to protect metadump from dumping absurdly long extents resulting from bmbt corruption, but it's certainly possible to create a directory with an extent longer than 20 blocks. Hilariously, the discards happen with no warning unless the caller explicitly set -w.
This was raised to 1000 blocks in 7431d134fe8 ("Increase default maximum extent size for xfs_metadump when copying..."), but it's still possible to create a directory with an extent longer than 1000 blocks.
Increase the threshold to MAXEXTLEN blocks because it's totally valid for the filesystem to create extents up to that length.

Quoted 7 lines

Quoted 2 lines
Signed-off-by Darrick J. Wong <[email protected]>
Reviewed-by Carlos Maiolino <[email protected]>
Thanks, I'll pull this in.
Should we fix the noisiness so it doesn't require -w for warnings as well?

Yeah. Seeing as we can't have an extent that long anyway we could possibly promote it to a flat out error message, at least if it's set to 2097151.

--D

Quoted 44 lines

-Eric
Quoted 34 lines
---
 db/metadump.c           |    2 +-
 man/man8/xfs_metadump.8 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
db/metadump.c +1 -1
index 6dd06c3..8ffb90f 100644
@@ -32,7 +32,7 @@
 #include "field.h"
 #include "dir2.h"
 
-#define DEFAULT_MAX_EXT_SIZE	1000
+#define DEFAULT_MAX_EXT_SIZE	MAXEXTLEN
 
 /*
  * It's possible that multiple files in a directory (or attributes
man/man8/xfs_metadump.8 +1 -2
index 3731d6a..7207c20 100644
@@ -114,7 +114,7 @@ copied.
 .B \-m
 Set the maximum size of an allowed metadata extent.  Extremely large metadata
 extents are likely to be corrupt, and will be skipped if they exceed
-this value.  The default size is 1000 blocks.
+this value.  The default size is 2097151 blocks.
 .TP
 .B \-o
 Disables obfuscation of file names and extended attributes.
 
--
Quoted 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-26 22:15 UTC #4 lore raw

PATCH 2/10 xfsprogs: explicitly cast troublesome types to match printf format specifiers

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Certain system-defined types (__u64, __s64, __nlink_t, __ino64_t, __off64_t, __blkcnt64_t) don't have a consistent definition across different architectures, so wherever we use a printf format specifier on such a variable, we have to typecast the variable or else the compiler will complain.

IOWs this fixes build warnings on ppc64le.

Signed-off-by Darrick J. Wong <[email protected]>

---
 io/fiemap.c          |   37 +++++++++++++++++++++----------------
 io/open.c            |    4 ++--
 io/stat.c            |   26 +++++++++++++-------------
 repair/attr_repair.c |    6 ++++--
 repair/dinode.c      |    3 ++-
 repair/phase6.c      |    4 ++--
 repair/scan.c        |    9 ++++++---
 7 files changed, 50 insertions(+), 39 deletions(-)

io/fiemap.c +21 ~~-16~~

index e6fd66d..bdcfacd 100644
@@ -67,16 +67,18 @@ print_hole(
 
 	   if (plain) {
 		printf("\t%d: [%llu..%llu]: hole", cur_extent,
-		       llast, lstart - 1ULL);
+		       (unsigned long long)llast, lstart - 1ULL);
 		if (lflag)
-			printf(_(" %llu blocks\n"), lstart - llast);
+			printf(_(" %llu blocks\n"),
+			       (unsigned long long)lstart - llast);
 		else
 			printf("\n");
 	   } else {
-		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", llast,
-			 lstart - 1ULL);
+		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:",
+			 (unsigned long long)llast, lstart - 1ULL);
 		printf("%4d: %-*s %-*s %*llu\n", cur_extent, foff_w, lbuf,
-		       boff_w, _("hole"), tot_w, lstart - llast);
+		       boff_w, _("hole"), tot_w,
+		       (unsigned long long)lstart - llast);
 	   }

@@ -125,12 +127,13 @@ print_verbose(
 	if (cur_extent == max_extents)
 		return 1;
 
-	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
-		 lstart + len - 1ULL);
-	snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block, block + len - 1ULL);
+	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:",
+		 (unsigned long long)lstart, lstart + len - 1ULL);
+	snprintf(bbuf, sizeof(bbuf), "%llu..%llu",
+		 (unsigned long long)block, block + len - 1ULL);
 	snprintf(flgbuf, sizeof(flgbuf), "0x%x", extent->fe_flags);
 	printf("%4d: %-*s %-*s %*llu %*s\n", cur_extent, foff_w, lbuf,
-	       boff_w, bbuf, tot_w, len, flg_w, flgbuf);
+	       boff_w, bbuf, tot_w, (unsigned long long)len, flg_w, flgbuf);
 
 	return 2;
 }

@@ -161,11 +164,11 @@ print_plain(
 		return 1;
 
 	printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
-	       lstart, lstart + len - 1ULL, block,
-	       block + len - 1ULL);
+	       (unsigned long long)lstart, lstart + len - 1ULL,
+	       (unsigned long long)block, block + len - 1ULL);
 
 	if (lflag)
-		printf(_(" %llu blocks\n"), len);
+		printf(_(" %llu blocks\n"), (unsigned long long)len);
 	else
 		printf("\n");
 	return 2;

@@ -198,10 +201,12 @@ calc_print_format(
 		len = BTOBBT(extent->fe_length);
 		block = BTOBBT(extent->fe_physical);
 
-		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]", logical,
-			 logical + len - 1);
-		snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block,
-			 block + len - 1);
+		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]",
+			 (unsigned long long)logical,
+			 (unsigned long long)logical + len - 1);
+		snprintf(bbuf, sizeof(bbuf), "%llu..%llu",
+			 (unsigned long long)block,
+			 (unsigned long long)block + len - 1);
 		*foff_w = max(*foff_w, strlen(lbuf));
 		*boff_w = max(*boff_w, strlen(bbuf));
 		*tot_w = max(*tot_w, numlen(len, 10));

io/open.c +2 -2

index f2ea7c3..2cce045 100644
@@ -762,14 +762,14 @@ inode_f(
 
 	if (verbose && result_ino) {
 		/* Requested verbose and we have an answer */
-		printf("%llu:%d\n", result_ino,
+		printf("%llu:%d\n", (unsigned long long)result_ino,
 			result_ino > XFS_MAXINUMBER_32 ? 64 : 32);
 	} else if (userino == NULLFSINO) {
 		/* Just checking 32 or 64 bit presence, non-verbose */
 		printf("%d\n", result_ino > XFS_MAXINUMBER_32 ? 1 : 0);
 	} else {
 		/* We asked about a specific inode, non-verbose */
-		printf("%llu\n", result_ino);
+		printf("%llu\n", (unsigned long long)result_ino);
 	}
 
 	return 0;

io/stat.c +13 ~~-13~~

index 060ff83..b97cced 100644
@@ -69,14 +69,14 @@ filetype(mode_t mode)
 static int
 dump_raw_stat(struct stat *st)
 {
-	printf("stat.blksize = %lu\n", st->st_blksize);
-	printf("stat.nlink = %lu\n", st->st_nlink);
+	printf("stat.blksize = %lu\n", (unsigned long)st->st_blksize);
+	printf("stat.nlink = %lu\n", (unsigned long)st->st_nlink);
 	printf("stat.uid = %u\n", st->st_uid);
 	printf("stat.gid = %u\n", st->st_gid);
 	printf("stat.mode: 0%o\n", st->st_mode);
-	printf("stat.ino = %lu\n", st->st_ino);
-	printf("stat.size = %lu\n", st->st_size);
-	printf("stat.blocks = %lu\n", st->st_blocks);
+	printf("stat.ino = %llu\n", (unsigned long long)st->st_ino);
+	printf("stat.size = %lld\n", (long long)st->st_size);
+	printf("stat.blocks = %lld\n", (long long)st->st_blocks);
 	printf("stat.atime.tv_sec = %ld\n", st->st_atim.tv_sec);
 	printf("stat.atime.tv_nsec = %ld\n", st->st_atim.tv_nsec);
 	printf("stat.ctime.tv_sec = %ld\n", st->st_ctim.tv_sec);

@@ -273,21 +273,21 @@ dump_raw_statx(struct statx *stx)
 {
 	printf("stat.mask = 0x%x\n", stx->stx_mask);
 	printf("stat.blksize = %u\n", stx->stx_blksize);
-	printf("stat.attributes = 0x%llx\n", stx->stx_attributes);
+	printf("stat.attributes = 0x%llx\n", (unsigned long long)stx->stx_attributes);
 	printf("stat.nlink = %u\n", stx->stx_nlink);
 	printf("stat.uid = %u\n", stx->stx_uid);
 	printf("stat.gid = %u\n", stx->stx_gid);
 	printf("stat.mode: 0%o\n", stx->stx_mode);
-	printf("stat.ino = %llu\n", stx->stx_ino);
-	printf("stat.size = %llu\n", stx->stx_size);
-	printf("stat.blocks = %llu\n", stx->stx_blocks);
-	printf("stat.atime.tv_sec = %lld\n", stx->stx_atime.tv_sec);
+	printf("stat.ino = %llu\n", (unsigned long long)stx->stx_ino);
+	printf("stat.size = %llu\n", (unsigned long long)stx->stx_size);
+	printf("stat.blocks = %llu\n", (unsigned long long)stx->stx_blocks);
+	printf("stat.atime.tv_sec = %lld\n", (long long)stx->stx_atime.tv_sec);
 	printf("stat.atime.tv_nsec = %d\n", stx->stx_atime.tv_nsec);
-	printf("stat.btime.tv_sec = %lld\n", stx->stx_btime.tv_sec);
+	printf("stat.btime.tv_sec = %lld\n", (long long)stx->stx_btime.tv_sec);
 	printf("stat.btime.tv_nsec = %d\n", stx->stx_btime.tv_nsec);
-	printf("stat.ctime.tv_sec = %lld\n", stx->stx_ctime.tv_sec);
+	printf("stat.ctime.tv_sec = %lld\n", (long long)stx->stx_ctime.tv_sec);
 	printf("stat.ctime.tv_nsec = %d\n", stx->stx_ctime.tv_nsec);
-	printf("stat.mtime.tv_sec = %lld\n", stx->stx_mtime.tv_sec);
+	printf("stat.mtime.tv_sec = %lld\n", (long long)stx->stx_mtime.tv_sec);
 	printf("stat.mtime.tv_nsec = %d\n", stx->stx_mtime.tv_nsec);
 	printf("stat.rdev_major = %u\n", stx->stx_rdev_major);
 	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);

repair/attr_repair.c +4 -2

index 9ec2231..8b1b8a7 100644
@@ -943,14 +943,16 @@ __check_attr_header(
 	if (be64_to_cpu(info->owner) != ino) {
 		do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, attr block %" PRIu64 "\n"),
-			ino, be64_to_cpu(info->owner), bp->b_bn);
+			ino, (unsigned long long)be64_to_cpu(info->owner),
+			bp->b_bn);
 		return 1;
 	}
 	/* verify block number */
 	if (be64_to_cpu(info->blkno) != bp->b_bn) {
 		do_warn(
 _("expected block %" PRIu64 ", got %llu, inode %" PRIu64 "attr block\n"),
-			bp->b_bn, be64_to_cpu(info->blkno), ino);
+			bp->b_bn, (unsigned long long)be64_to_cpu(info->blkno),
+			ino);
 		return 1;
 	}
 	/* verify uuid */

repair/dinode.c +2 -1

index 15ba8cc..e7de6d4 100644
@@ -2330,7 +2330,8 @@ process_dinode_int(xfs_mount_t *mp,
 			if (!uncertain)
 				do_warn(
 _("inode identifier %llu mismatch on inode %" PRIu64 "\n"),
-					be64_to_cpu(dino->di_ino), lino);
+					(unsigned long long)be64_to_cpu(dino->di_ino),
+					lino);
 			if (verify_mode)
 				return 1;
 			goto clear_bad_out;

repair/phase6.c +2 -2

index 4279d2a..37505a8 100644
@@ -1917,14 +1917,14 @@ __check_dir3_header(
 	if (be64_to_cpu(owner) != ino) {
 		do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, directory block %" PRIu64 "\n"),
-			ino, be64_to_cpu(owner), bp->b_bn);
+			ino, (unsigned long long)be64_to_cpu(owner), bp->b_bn);
 		return 1;
 	}
 	/* verify block number */
 	if (be64_to_cpu(blkno) != bp->b_bn) {
 		do_warn(
 _("expected block %" PRIu64 ", got %llu, directory inode %" PRIu64 "\n"),
-			bp->b_bn, be64_to_cpu(blkno), ino);
+			bp->b_bn, (unsigned long long)be64_to_cpu(blkno), ino);
 		return 1;
 	}
 	/* verify uuid */

repair/scan.c +6 -3

index 9c0f2d6..22c7331 100644
@@ -227,7 +227,9 @@ _("expected level %d got %d in inode %" PRIu64 ", (%s fork) bmbt block %" PRIu64
 		if (be64_to_cpu(block->bb_u.l.bb_owner) != ino) {
 			do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
-				ino, be64_to_cpu(block->bb_u.l.bb_owner), bno);
+				ino,
+				(unsigned long long)be64_to_cpu(block->bb_u.l.bb_owner),
+				bno);
 			return 1;
 		}
 		/* verify block number */

@@ -236,7 +238,8 @@ _("expected owner inode %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
 			do_warn(
 _("expected block %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
 				XFS_FSB_TO_DADDR(mp, bno),
-				be64_to_cpu(block->bb_u.l.bb_blkno), bno);
+				(unsigned long long)be64_to_cpu(block->bb_u.l.bb_blkno),
+				bno);
 			return 1;
 		}
 		/* verify uuid */

@@ -1587,7 +1590,7 @@ import_single_ino_chunk(
 _("ir_holemask/ir_free mismatch, %s chunk %d/%u, holemask 0x%x free 0x%llx\n"),
 					inobt_name, agno, ino,
 					be16_to_cpu(rp->ir_u.sp.ir_holemask),
-					be64_to_cpu(rp->ir_free));
+					(unsigned long long)be64_to_cpu(rp->ir_free));
 				suspect++;
 			}
 			if (!suspect && ino_rec)

Eric Sandeen

Reviewed-by on v1 2/10

2017-10-27 00:06 UTC #5 lore raw

Re: [PATCH 02/10] xfsprogs: explicitly cast troublesome types to match printf format specifiers

Quoted 11 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Certain system-defined types (__u64, __s64, __nlink_t, __ino64_t, __off64_t, __blkcnt64_t) don't have a consistent definition across different architectures, so wherever we use a printf format specifier on such a variable, we have to typecast the variable or else the compiler will complain.
IOWs this fixes build warnings on ppc64le.
Signed-off-by Darrick J. Wong <[email protected]>

Sigh, I hate this. ;)

Reviewed-by Eric Sandeen <[email protected]>

Quoted 262 lines

---
 io/fiemap.c          |   37 +++++++++++++++++++++----------------
 io/open.c            |    4 ++--
 io/stat.c            |   26 +++++++++++++-------------
 repair/attr_repair.c |    6 ++++--
 repair/dinode.c      |    3 ++-
 repair/phase6.c      |    4 ++--
 repair/scan.c        |    9 ++++++---
 7 files changed, 50 insertions(+), 39 deletions(-)

io/fiemap.c +21 ~~-16~~

index e6fd66d..bdcfacd 100644
@@ -67,16 +67,18 @@ print_hole(
 
 	   if (plain) {
 		printf("\t%d: [%llu..%llu]: hole", cur_extent,
-		       llast, lstart - 1ULL);
+		       (unsigned long long)llast, lstart - 1ULL);
 		if (lflag)
-			printf(_(" %llu blocks\n"), lstart - llast);
+			printf(_(" %llu blocks\n"),
+			       (unsigned long long)lstart - llast);
 		else
 			printf("\n");
 	   } else {
-		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", llast,
-			 lstart - 1ULL);
+		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:",
+			 (unsigned long long)llast, lstart - 1ULL);
 		printf("%4d: %-*s %-*s %*llu\n", cur_extent, foff_w, lbuf,
-		       boff_w, _("hole"), tot_w, lstart - llast);
+		       boff_w, _("hole"), tot_w,
+		       (unsigned long long)lstart - llast);
 	   }

@@ -125,12 +127,13 @@ print_verbose(
 	if (cur_extent == max_extents)
 		return 1;
 
-	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
-		 lstart + len - 1ULL);
-	snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block, block + len - 1ULL);
+	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:",
+		 (unsigned long long)lstart, lstart + len - 1ULL);
+	snprintf(bbuf, sizeof(bbuf), "%llu..%llu",
+		 (unsigned long long)block, block + len - 1ULL);
 	snprintf(flgbuf, sizeof(flgbuf), "0x%x", extent->fe_flags);
 	printf("%4d: %-*s %-*s %*llu %*s\n", cur_extent, foff_w, lbuf,
-	       boff_w, bbuf, tot_w, len, flg_w, flgbuf);
+	       boff_w, bbuf, tot_w, (unsigned long long)len, flg_w, flgbuf);
 
 	return 2;
 }

@@ -161,11 +164,11 @@ print_plain(
 		return 1;
 
 	printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
-	       lstart, lstart + len - 1ULL, block,
-	       block + len - 1ULL);
+	       (unsigned long long)lstart, lstart + len - 1ULL,
+	       (unsigned long long)block, block + len - 1ULL);
 
 	if (lflag)
-		printf(_(" %llu blocks\n"), len);
+		printf(_(" %llu blocks\n"), (unsigned long long)len);
 	else
 		printf("\n");
 	return 2;

@@ -198,10 +201,12 @@ calc_print_format(
 		len = BTOBBT(extent->fe_length);
 		block = BTOBBT(extent->fe_physical);
 
-		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]", logical,
-			 logical + len - 1);
-		snprintf(bbuf, sizeof(bbuf), "%llu..%llu", block,
-			 block + len - 1);
+		snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]",
+			 (unsigned long long)logical,
+			 (unsigned long long)logical + len - 1);
+		snprintf(bbuf, sizeof(bbuf), "%llu..%llu",
+			 (unsigned long long)block,
+			 (unsigned long long)block + len - 1);
 		*foff_w = max(*foff_w, strlen(lbuf));
 		*boff_w = max(*boff_w, strlen(bbuf));
 		*tot_w = max(*tot_w, numlen(len, 10));

io/open.c +2 -2

index f2ea7c3..2cce045 100644
@@ -762,14 +762,14 @@ inode_f(
 
 	if (verbose && result_ino) {
 		/* Requested verbose and we have an answer */
-		printf("%llu:%d\n", result_ino,
+		printf("%llu:%d\n", (unsigned long long)result_ino,
 			result_ino > XFS_MAXINUMBER_32 ? 64 : 32);
 	} else if (userino == NULLFSINO) {
 		/* Just checking 32 or 64 bit presence, non-verbose */
 		printf("%d\n", result_ino > XFS_MAXINUMBER_32 ? 1 : 0);
 	} else {
 		/* We asked about a specific inode, non-verbose */
-		printf("%llu\n", result_ino);
+		printf("%llu\n", (unsigned long long)result_ino);
 	}
 
 	return 0;

io/stat.c +13 ~~-13~~

index 060ff83..b97cced 100644
@@ -69,14 +69,14 @@ filetype(mode_t mode)
 static int
 dump_raw_stat(struct stat *st)
 {
-	printf("stat.blksize = %lu\n", st->st_blksize);
-	printf("stat.nlink = %lu\n", st->st_nlink);
+	printf("stat.blksize = %lu\n", (unsigned long)st->st_blksize);
+	printf("stat.nlink = %lu\n", (unsigned long)st->st_nlink);
 	printf("stat.uid = %u\n", st->st_uid);
 	printf("stat.gid = %u\n", st->st_gid);
 	printf("stat.mode: 0%o\n", st->st_mode);
-	printf("stat.ino = %lu\n", st->st_ino);
-	printf("stat.size = %lu\n", st->st_size);
-	printf("stat.blocks = %lu\n", st->st_blocks);
+	printf("stat.ino = %llu\n", (unsigned long long)st->st_ino);
+	printf("stat.size = %lld\n", (long long)st->st_size);
+	printf("stat.blocks = %lld\n", (long long)st->st_blocks);
 	printf("stat.atime.tv_sec = %ld\n", st->st_atim.tv_sec);
 	printf("stat.atime.tv_nsec = %ld\n", st->st_atim.tv_nsec);
 	printf("stat.ctime.tv_sec = %ld\n", st->st_ctim.tv_sec);

@@ -273,21 +273,21 @@ dump_raw_statx(struct statx *stx)
 {
 	printf("stat.mask = 0x%x\n", stx->stx_mask);
 	printf("stat.blksize = %u\n", stx->stx_blksize);
-	printf("stat.attributes = 0x%llx\n", stx->stx_attributes);
+	printf("stat.attributes = 0x%llx\n", (unsigned long long)stx->stx_attributes);
 	printf("stat.nlink = %u\n", stx->stx_nlink);
 	printf("stat.uid = %u\n", stx->stx_uid);
 	printf("stat.gid = %u\n", stx->stx_gid);
 	printf("stat.mode: 0%o\n", stx->stx_mode);
-	printf("stat.ino = %llu\n", stx->stx_ino);
-	printf("stat.size = %llu\n", stx->stx_size);
-	printf("stat.blocks = %llu\n", stx->stx_blocks);
-	printf("stat.atime.tv_sec = %lld\n", stx->stx_atime.tv_sec);
+	printf("stat.ino = %llu\n", (unsigned long long)stx->stx_ino);
+	printf("stat.size = %llu\n", (unsigned long long)stx->stx_size);
+	printf("stat.blocks = %llu\n", (unsigned long long)stx->stx_blocks);
+	printf("stat.atime.tv_sec = %lld\n", (long long)stx->stx_atime.tv_sec);
 	printf("stat.atime.tv_nsec = %d\n", stx->stx_atime.tv_nsec);
-	printf("stat.btime.tv_sec = %lld\n", stx->stx_btime.tv_sec);
+	printf("stat.btime.tv_sec = %lld\n", (long long)stx->stx_btime.tv_sec);
 	printf("stat.btime.tv_nsec = %d\n", stx->stx_btime.tv_nsec);
-	printf("stat.ctime.tv_sec = %lld\n", stx->stx_ctime.tv_sec);
+	printf("stat.ctime.tv_sec = %lld\n", (long long)stx->stx_ctime.tv_sec);
 	printf("stat.ctime.tv_nsec = %d\n", stx->stx_ctime.tv_nsec);
-	printf("stat.mtime.tv_sec = %lld\n", stx->stx_mtime.tv_sec);
+	printf("stat.mtime.tv_sec = %lld\n", (long long)stx->stx_mtime.tv_sec);
 	printf("stat.mtime.tv_nsec = %d\n", stx->stx_mtime.tv_nsec);
 	printf("stat.rdev_major = %u\n", stx->stx_rdev_major);
 	printf("stat.rdev_minor = %u\n", stx->stx_rdev_minor);

repair/attr_repair.c +4 -2

index 9ec2231..8b1b8a7 100644
@@ -943,14 +943,16 @@ __check_attr_header(
 	if (be64_to_cpu(info->owner) != ino) {
 		do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, attr block %" PRIu64 "\n"),
-			ino, be64_to_cpu(info->owner), bp->b_bn);
+			ino, (unsigned long long)be64_to_cpu(info->owner),
+			bp->b_bn);
 		return 1;
 	}
 	/* verify block number */
 	if (be64_to_cpu(info->blkno) != bp->b_bn) {
 		do_warn(
 _("expected block %" PRIu64 ", got %llu, inode %" PRIu64 "attr block\n"),
-			bp->b_bn, be64_to_cpu(info->blkno), ino);
+			bp->b_bn, (unsigned long long)be64_to_cpu(info->blkno),
+			ino);
 		return 1;
 	}
 	/* verify uuid */

repair/dinode.c +2 -1

index 15ba8cc..e7de6d4 100644
@@ -2330,7 +2330,8 @@ process_dinode_int(xfs_mount_t *mp,
 			if (!uncertain)
 				do_warn(
 _("inode identifier %llu mismatch on inode %" PRIu64 "\n"),
-					be64_to_cpu(dino->di_ino), lino);
+					(unsigned long long)be64_to_cpu(dino->di_ino),
+					lino);
 			if (verify_mode)
 				return 1;
 			goto clear_bad_out;

repair/phase6.c +2 -2

index 4279d2a..37505a8 100644
@@ -1917,14 +1917,14 @@ __check_dir3_header(
 	if (be64_to_cpu(owner) != ino) {
 		do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, directory block %" PRIu64 "\n"),
-			ino, be64_to_cpu(owner), bp->b_bn);
+			ino, (unsigned long long)be64_to_cpu(owner), bp->b_bn);
 		return 1;
 	}
 	/* verify block number */
 	if (be64_to_cpu(blkno) != bp->b_bn) {
 		do_warn(
 _("expected block %" PRIu64 ", got %llu, directory inode %" PRIu64 "\n"),
-			bp->b_bn, be64_to_cpu(blkno), ino);
+			bp->b_bn, (unsigned long long)be64_to_cpu(blkno), ino);
 		return 1;
 	}
 	/* verify uuid */

repair/scan.c +6 -4

index 9c0f2d6..22c7331 100644
@@ -227,7 +227,9 @@ _("expected level %d got %d in inode %" PRIu64 ", (%s fork) bmbt block %" PRIu64
 		if (be64_to_cpu(block->bb_u.l.bb_owner) != ino) {
 			do_warn(
 _("expected owner inode %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
-				ino, be64_to_cpu(block->bb_u.l.bb_owner), bno);
+				ino,
+				(unsigned long long)be64_to_cpu(block->bb_u.l.bb_owner),
+				bno);
 			return 1;
 		}
 		/* verify block number */

@@ -236,7 +238,8 @@ _("expected owner inode %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
 			do_warn(
 _("expected block %" PRIu64 ", got %llu, bmbt block %" PRIu64 "\n"),
 				XFS_FSB_TO_DADDR(mp, bno),
-				be64_to_cpu(block->bb_u.l.bb_blkno), bno);
+				(unsigned long long)be64_to_cpu(block->bb_u.l.bb_blkno),
+				bno);
 			return 1;
 		}
 		/* verify uuid */

@@ -1587,7 +1590,7 @@ import_single_ino_chunk(
 _("ir_holemask/ir_free mismatch, %s chunk %d/%u, holemask 0x%x free 0x%llx\n"),
 					inobt_name, agno, ino,
 					be16_to_cpu(rp->ir_u.sp.ir_holemask),
-					be64_to_cpu(rp->ir_free));
+					(unsigned long long)be64_to_cpu(rp->ir_free));
 				suspect++;
 			}
 			if (!suspect && ino_rec)
 
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-26 22:15 UTC #6 lore raw

PATCH 3/10 xfs_io: add new error injection knobs to inject command

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Bring xfs_io's inject command up to date with the newest knobs.

Signed-off-by Darrick J. Wong <[email protected]>

---
 io/inject.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

io/inject.c +7 -1

index 25c7021..964ebfe 100644
@@ -86,7 +86,13 @@ error_tag(char *name)
 		{ XFS_ERRTAG_BMAP_FINISH_ONE,		"bmap_finish_one" },
 #define XFS_ERRTAG_AG_RESV_CRITICAL			27
 		{ XFS_ERRTAG_AG_RESV_CRITICAL,		"ag_resv_critical" },
-#define XFS_ERRTAG_MAX                                  28
+#define XFS_ERRTAG_DROP_WRITES				28
+		{ XFS_ERRTAG_DROP_WRITES,		"drop_writes" },
+#define XFS_ERRTAG_LOG_BAD_CRC				29
+		{ XFS_ERRTAG_LOG_BAD_CRC,		"log_bad_crc" },
+#define XFS_ERRTAG_LOG_ITEM_PIN				30
+		{ XFS_ERRTAG_LOG_ITEM_PIN,		"log_item_pin" },
+#define XFS_ERRTAG_MAX                                  31
 		{ XFS_ERRTAG_MAX,			NULL }
 	};
 	int	count;

Eric Sandeen

Reviewed-by on v1 3/10

2017-10-27 00:09 UTC #7 lore raw

Re: [PATCH 03/10] xfs_io: add new error injection knobs to inject command

Quoted 5 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Bring xfs_io's inject command up to date with the newest knobs.
Signed-off-by Darrick J. Wong <[email protected]>

Ok, but should we get this into shared libxfs files eventually?

Reviewed-by Eric Sandeen <[email protected]>

Quoted 30 lines

---
 io/inject.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

io/inject.c +7 -2

index 25c7021..964ebfe 100644
@@ -86,7 +86,13 @@ error_tag(char *name)
 		{ XFS_ERRTAG_BMAP_FINISH_ONE,		"bmap_finish_one" },
 #define XFS_ERRTAG_AG_RESV_CRITICAL			27
 		{ XFS_ERRTAG_AG_RESV_CRITICAL,		"ag_resv_critical" },
-#define XFS_ERRTAG_MAX                                  28
+#define XFS_ERRTAG_DROP_WRITES				28
+		{ XFS_ERRTAG_DROP_WRITES,		"drop_writes" },
+#define XFS_ERRTAG_LOG_BAD_CRC				29
+		{ XFS_ERRTAG_LOG_BAD_CRC,		"log_bad_crc" },
+#define XFS_ERRTAG_LOG_ITEM_PIN				30
+		{ XFS_ERRTAG_LOG_ITEM_PIN,		"log_item_pin" },
+#define XFS_ERRTAG_MAX                                  31
 		{ XFS_ERRTAG_MAX,			NULL }
 	};
 	int	count;
 
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-26 22:15 UTC #8 lore raw

PATCH 4/10 xfs_repair: fix bag memory overwrite problems

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

There's an off by one error in the bag_remove code such that we end up copying memory from beyond the end of the array into the array. Not a serious problem since we have counters to prevent us from reading that garbage, but AddressSanitizer complained so let's fix it.

Signed-off-by Darrick J. Wong <[email protected]>

---
 repair/slab.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

repair/slab.c +1 -1

index 8609270..d47448a 100644
@@ -469,7 +469,7 @@ bag_remove(
 {
 	ASSERT(nr < bag->bg_inuse);
 	memmove(&bag->bg_ptrs[nr], &bag->bg_ptrs[nr + 1],
-		(bag->bg_inuse - nr) * sizeof(void *));
+		(bag->bg_inuse - nr - 1) * sizeof(void *));
 	bag->bg_inuse--;
 	return 0;
 }

Eric Sandeen

Reviewed-by on v1 4/10

2017-10-27 00:49 UTC #9 lore raw

Re: [PATCH 04/10] xfs_repair: fix bag memory overwrite problems

Quoted 8 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
There's an off by one error in the bag_remove code such that we end up copying memory from beyond the end of the array into the array. Not a serious problem since we have counters to prevent us from reading that garbage, but AddressSanitizer complained so let's fix it.
Signed-off-by Darrick J. Wong <[email protected]>

Reviewed-by Eric Sandeen <[email protected]>

Quoted 19 lines

---
 repair/slab.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

repair/slab.c +1 -1

index 8609270..d47448a 100644
@@ -469,7 +469,7 @@ bag_remove(
 {
 	ASSERT(nr < bag->bg_inuse);
 	memmove(&bag->bg_ptrs[nr], &bag->bg_ptrs[nr + 1],
-		(bag->bg_inuse - nr) * sizeof(void *));
+		(bag->bg_inuse - nr - 1) * sizeof(void *));
 	bag->bg_inuse--;
 	return 0;
 }

Darrick J. Wong

author

2017-10-26 22:15 UTC #10 lore raw

PATCH 5/10 xfs_repair: clear DAX flag from non-file inodes

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

The DAX flag should only be set for files and directories.

Signed-off-by Darrick J. Wong <[email protected]>

---
 repair/dinode.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

repair/dinode.c +12 -0

index e7de6d4..e62ec33 100644
@@ -2510,6 +2510,18 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
 			flags2 &= XFS_DIFLAG2_ANY;
 		}
 
+		if (flags2 & XFS_DIFLAG2_DAX) {
+			/* must be a file or dir */
+			if (di_mode && !(S_ISREG(di_mode) || S_ISDIR(di_mode))) {
+				if (!uncertain) {
+					do_warn(
+	_("DAX flag set on special inode %" PRIu64 "\n"),
+						lino);
+				}
+				flags2 &= ~XFS_DIFLAG2_DAX;
+			}
+		}
+
 		if ((flags2 & XFS_DIFLAG2_REFLINK) &&
 		    !xfs_sb_version_hasreflink(&mp->m_sb)) {
 			if (!uncertain) {

Eric Sandeen

Reviewed-by on v1 5/10

2017-10-27 02:01 UTC #11 lore raw

Re: [PATCH 05/10] xfs_repair: clear DAX flag from non-file inodes

Quoted 9 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
The DAX flag should only be set for files and directories.
Signed-off-by Darrick J. Wong <[email protected]>
---
 repair/dinode.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Reviewed-by Eric Sandeen <[email protected]>

Darrick J. Wong

author

2017-10-26 22:15 UTC #12 lore raw

PATCH 6/10 xfs_repair: fix cowextsize field checking and repairing

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Make sure that we never leave the filesystem with a zero cowextsize hint while the cowextsize inode flag is set.

Signed-off-by Darrick J. Wong <[email protected]>

---
 repair/dinode.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

repair/dinode.c +19 -0

index e62ec33..32cc769 100644
@@ -2694,6 +2694,7 @@ _("Cannot have non-zero CoW extent size %u on non-cowextsize inode %" PRIu64 ",
 					be32_to_cpu(dino->di_cowextsize), lino);
 			if (!no_modify)  {
 				do_warn(_("resetting to zero\n"));
+				dino->di_flags2 &= ~cpu_to_be64(XFS_DIFLAG2_COWEXTSIZE);
 				dino->di_cowextsize = 0;
 				*dirty = 1;
 			} else

@@ -2702,6 +2703,24 @@ _("Cannot have non-zero CoW extent size %u on non-cowextsize inode %" PRIu64 ",
 	}
 
 	/*
+	 * Can't have the COWEXTSIZE flag set with no hint.
+	 */
+	if (dino->di_version >= 3 &&
+	    be32_to_cpu(dino->di_cowextsize) == 0 &&
+	    (be64_to_cpu(dino->di_flags2) & XFS_DIFLAG2_COWEXTSIZE)) {
+		do_warn(
+_("Cannot have CoW extent size of zero on cowextsize inode %" PRIu64 ", "),
+				lino);
+		if (!no_modify)  {
+			do_warn(_("clearing cowextsize flag\n"));
+			dino->di_flags2 &= ~cpu_to_be64(XFS_DIFLAG2_COWEXTSIZE);
+			*dirty = 1;
+		} else {
+			do_warn(_("would clear cowextsize flag\n"));
+		}
+	}
+
+	/*
 	 * general size/consistency checks:
 	 */
 	if (process_check_inode_sizes(mp, dino, lino, type) != 0)

Eric Sandeen

Reviewed-by on v1 6/10

2017-10-27 02:06 UTC #13 lore raw

Re: [PATCH 06/10] xfs_repair: fix cowextsize field checking and repairing

Quoted 6 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Make sure that we never leave the filesystem with a zero cowextsize hint while the cowextsize inode flag is set.
Signed-off-by Darrick J. Wong <[email protected]>

Ok. Is there anything that catches cowextsize set on di_version < 3?

Reviewed-by Eric Sandeen <[email protected]>

Darrick J. Wong

author

2017-10-27 16:17 UTC #14 lore raw

Re: [PATCH 06/10] xfs_repair: fix cowextsize field checking and repairing

Quoted 9 lines · Eric Sandeen

Quoted 6 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Make sure that we never leave the filesystem with a zero cowextsize hint while the cowextsize inode flag is set.
Signed-off-by Darrick J. Wong <[email protected]>
Ok. Is there anything that catches cowextsize set on di_version < 3?

No, because the inode core structure isn't large enough to have cowextsize if version < 3.

--D

Quoted 7 lines

Reviewed-by Eric Sandeen <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric Sandeen

2017-10-27 16:27 UTC #15 lore raw

Re: [PATCH 06/10] xfs_repair: fix cowextsize field checking and repairing

Quoted 7 lines · Darrick J. Wong

Quoted 6 lines · Eric Sandeen
Quoted 5 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Make sure that we never leave the filesystem with a zero cowextsize hint while the cowextsize inode flag is set.

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Darrick J. Wong <[email protected]>
Ok. Is there anything that catches cowextsize set on di_version < 3?
No, because the inode core structure isn't large enough to have cowextsize if version < 3.

Oh right - no di_flags2 to hold the XFS_DIFLAG2_COWEXTSIZE flag, and no di_cowextsize to hold a value. Derp, sorry for the noise.

-Eric

Darrick J. Wong

author

2017-10-26 22:15 UTC #16 lore raw

PATCH 7/10 misc: enable ubsan if it's available

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Enable the undefined behavior sanitizer (ubsan) if it's available or the builder requests it.

Signed-off-by Darrick J. Wong <[email protected]>

---
 configure.ac            |   13 +++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    6 ++++--
 include/buildmacros     |    2 +-
 m4/Makefile             |    1 +
 m4/package_sanitizer.m4 |   19 +++++++++++++++++++
 6 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 m4/package_sanitizer.m4

configure.ac +13 -0

index 4161c3b..a5b2c65 100644
@@ -72,6 +72,12 @@ AC_ARG_ENABLE(librt,
 	enable_librt=yes)
 AC_SUBST(enable_librt)
 
+# Enable UBSAN; set enable_ubsan=yesdefault to enable autoprobe.
+AC_ARG_ENABLE(ubsan,
+[ --enable-ubsan=[yes/no] Enable Undefined Behavior Sanitizer (UBSAN) [default=yes]],,
+	enable_ubsan=no)
+AC_SUBST(enable_ubsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -148,6 +154,13 @@ if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
 fi
 
+if test "$enable_ubsan" = "yes" || test "$enable_ubsan" = "yesdefault"; then
+        AC_PACKAGE_CHECK_UBSAN
+fi
+if test "$enable_ubsan" = "yes" && test "$have_ubsan" != "yes"; then
+        AC_MSG_ERROR([UBSAN not supported by compiler.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index c673380..9dcaf52 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +4 -2

index ec630bd..6b9d6c2 100644
@@ -155,6 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
+SANITIZER_CFLAGS += @ubsan_cflags@
+SANITIZER_LDFLAGS += @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

@@ -165,8 +167,8 @@ GCFLAGS += -DENABLE_GETTEXT
 endif
 
 BUILD_CFLAGS += $(GCFLAGS) $(PCFLAGS)
-# First, Global, Platform, Local CFLAGS
-CFLAGS += $(FCFLAGS) $(OPTIMIZER) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
+# First, Sanitizer, Global, Platform, Local CFLAGS
+CFLAGS += $(FCFLAGS) $(SANITIZER_CFLAGS) $(OPTIMIZER) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
 
 include $(TOPDIR)/include/buildmacros

include/buildmacros +1 -1

index a7c5d8a..178e2ed 100644
@@ -9,7 +9,7 @@ BUILDRULES = $(TOPDIR)/include/buildrules
 # $(CXXFILES), or $(HFILES) and is used to construct the manifest list
 # during the "dist" phase (packaging).
 
-LDFLAGS += $(LOADERFLAGS) $(LLDFLAGS)
+LDFLAGS += $(SANITIZER_LDFLAGS) $(LOADERFLAGS) $(LLDFLAGS)
 LTLDFLAGS += $(LOADERFLAGS)
 LDLIBS = $(LLDLIBS) $(PLDLIBS) $(MALLOCLIB)

m4/Makefile +1 -0

index d282f0a..4706121 100644
@@ -18,6 +18,7 @@ LSRCFILES = \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
+	package_sanitizer.m4 \
 	package_types.m4 \
 	package_utilies.m4 \
 	package_uuiddev.m4 \

m4/package_sanitizer.m4 new file +19 -0

new file mode 100644
index 0000000..a6673f3
@@ -0,0 +1,19 @@
+AC_DEFUN([AC_PACKAGE_CHECK_UBSAN],
+  [ AC_MSG_CHECKING([if C compiler supports UBSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    UBSAN_FLAGS="-fsanitize=undefined"
+    CFLAGS="$CFLAGS $UBSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $UBSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [ubsan_cflags=$UBSAN_FLAGS]
+        [ubsan_ldflags=$UBSAN_FLAGS]
+        [have_ubsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_ubsan)
+    AC_SUBST(ubsan_cflags)
+    AC_SUBST(ubsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:23 UTC #17 lore raw

PATCH v2 1/1 misc: enable ubsan if the builder wants it

From: Darrick J. Wong <[email protected]>

Enable the undefined behavior sanitizer (ubsan) if the builder requests it and it's available.

Signed-off-by Darrick J. Wong <[email protected]>

---
 configure.ac            |   13 +++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    6 ++++--
 include/buildmacros     |    2 +-
 m4/Makefile             |    1 +
 m4/package_sanitizer.m4 |   19 +++++++++++++++++++
 6 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 m4/package_sanitizer.m4

configure.ac +13 -0

index 4161c3b..5838e08 100644
@@ -72,6 +72,12 @@ AC_ARG_ENABLE(librt,
 	enable_librt=yes)
 AC_SUBST(enable_librt)
 
+# Enable UBSAN; set enable_ubsan=probe below to enable autoprobe.
+AC_ARG_ENABLE(ubsan,
+[ --enable-ubsan=[yes/no] Enable Undefined Behavior Sanitizer (UBSAN) [default=no]],,
+	enable_ubsan=no)
+AC_SUBST(enable_ubsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -148,6 +154,13 @@ if test "$enable_blkid" = yes; then
 AC_HAVE_BLKID_TOPO
 fi
 
+if test "$enable_ubsan" = "yes" || test "$enable_ubsan" = "probe"; then
+        AC_PACKAGE_CHECK_UBSAN
+fi
+if test "$enable_ubsan" = "yes" && test "$have_ubsan" != "yes"; then
+        AC_MSG_ERROR([UBSAN not supported by compiler.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index c673380..9dcaf52 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +4 -2

index ec630bd..6b9d6c2 100644
@@ -155,6 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
+SANITIZER_CFLAGS += @ubsan_cflags@
+SANITIZER_LDFLAGS += @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

@@ -165,8 +167,8 @@ GCFLAGS += -DENABLE_GETTEXT
 endif
 
 BUILD_CFLAGS += $(GCFLAGS) $(PCFLAGS)
-# First, Global, Platform, Local CFLAGS
-CFLAGS += $(FCFLAGS) $(OPTIMIZER) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
+# First, Sanitizer, Global, Platform, Local CFLAGS
+CFLAGS += $(FCFLAGS) $(SANITIZER_CFLAGS) $(OPTIMIZER) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
 
 include $(TOPDIR)/include/buildmacros

include/buildmacros +1 -1

index a7c5d8a..178e2ed 100644
@@ -9,7 +9,7 @@ BUILDRULES = $(TOPDIR)/include/buildrules
 # $(CXXFILES), or $(HFILES) and is used to construct the manifest list
 # during the "dist" phase (packaging).
 
-LDFLAGS += $(LOADERFLAGS) $(LLDFLAGS)
+LDFLAGS += $(SANITIZER_LDFLAGS) $(LOADERFLAGS) $(LLDFLAGS)
 LTLDFLAGS += $(LOADERFLAGS)
 LDLIBS = $(LLDLIBS) $(PLDLIBS) $(MALLOCLIB)

m4/Makefile +1 -0

index d282f0a..4706121 100644
@@ -18,6 +18,7 @@ LSRCFILES = \
 	package_globals.m4 \
 	package_libcdev.m4 \
 	package_pthread.m4 \
+	package_sanitizer.m4 \
 	package_types.m4 \
 	package_utilies.m4 \
 	package_uuiddev.m4 \

m4/package_sanitizer.m4 new file +19 -0

new file mode 100644
index 0000000..a6673f3
@@ -0,0 +1,19 @@
+AC_DEFUN([AC_PACKAGE_CHECK_UBSAN],
+  [ AC_MSG_CHECKING([if C compiler supports UBSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    UBSAN_FLAGS="-fsanitize=undefined"
+    CFLAGS="$CFLAGS $UBSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $UBSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [ubsan_cflags=$UBSAN_FLAGS]
+        [ubsan_ldflags=$UBSAN_FLAGS]
+        [have_ubsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_ubsan)
+    AC_SUBST(ubsan_cflags)
+    AC_SUBST(ubsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:15 UTC #18 lore raw

PATCH 8/10 misc: enable gcc/clang address sanitizer

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Enable AddressSanitizer to look for memory usage errors if requested.

Signed-off-by Darrick J. Wong <[email protected]>

---
 configure.ac            |   13 +++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    4 ++--
 m4/package_sanitizer.m4 |   20 ++++++++++++++++++++
 4 files changed, 37 insertions(+), 4 deletions(-)

configure.ac +13 -0

index a5b2c65..b101e05 100644
@@ -78,6 +78,12 @@ AC_ARG_ENABLE(ubsan,
 	enable_ubsan=no)
 AC_SUBST(enable_ubsan)
 
+# Enable ADDRSAN; set enable_addrsan=yesdefault to enable autoprobe.
+AC_ARG_ENABLE(addrsan,
+[ --enable-addrsan=[yes/no] Enable Address Sanitizer (ADDRSAN) [default=yes]],,
+	enable_addrsan=no)
+AC_SUBST(enable_addrsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -161,6 +167,13 @@ if test "$enable_ubsan" = "yes" && test "$have_ubsan" != "yes"; then
         AC_MSG_ERROR([UBSAN not supported by compiler.])
 fi
 
+if test "$enable_addrsan" = "yes" || test "$enable_addrsan" = "yesdefault"; then
+        AC_PACKAGE_CHECK_ADDRSAN
+fi
+if test "$enable_addrsan" = "yes" && test "$have_addrsan" != "yes"; then
+        AC_MSG_ERROR([ADDRSAN not supported by compiler.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index 9dcaf52..6b6f45b 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +2 -2

index 6b9d6c2..7c78d4b 100644
@@ -155,8 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
-SANITIZER_CFLAGS += @ubsan_cflags@
-SANITIZER_LDFLAGS += @ubsan_ldflags@
+SANITIZER_CFLAGS += @addrsan_cflags@ @ubsan_cflags@
+SANITIZER_LDFLAGS += @addrsan_ldflags@ @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

m4/package_sanitizer.m4 +20 -0

index a6673f3..06031ad 100644
@@ -17,3 +17,23 @@ AC_DEFUN([AC_PACKAGE_CHECK_UBSAN],
     AC_SUBST(ubsan_cflags)
     AC_SUBST(ubsan_ldflags)
   ])
+
+AC_DEFUN([AC_PACKAGE_CHECK_ADDRSAN],
+  [ AC_MSG_CHECKING([if C compiler supports ADDRSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    ADDRSAN_FLAGS="-fsanitize=address"
+    CFLAGS="$CFLAGS $ADDRSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $ADDRSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [addrsan_cflags=$ADDRSAN_FLAGS]
+        [addrsan_ldflags=$ADDRSAN_FLAGS]
+        [have_addrsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_addrsan)
+    AC_SUBST(addrsan_cflags)
+    AC_SUBST(addrsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:24 UTC #19 lore raw

PATCH v2 8/10 misc: enable gcc/clang address sanitizer

From: Darrick J. Wong <[email protected]>

Enable AddressSanitizer to look for memory usage errors if the builder asks for it and it's available.

Signed-off-by Darrick J. Wong <[email protected]>

---
 configure.ac            |   13 +++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    4 ++--
 m4/package_sanitizer.m4 |   20 ++++++++++++++++++++
 4 files changed, 37 insertions(+), 4 deletions(-)

configure.ac +13 -0

index 5838e08..764b22b 100644
@@ -78,6 +78,12 @@ AC_ARG_ENABLE(ubsan,
 	enable_ubsan=no)
 AC_SUBST(enable_ubsan)
 
+# Enable ADDRSAN; set enable_addrsan=probe below to enable autoprobe.
+AC_ARG_ENABLE(addrsan,
+[ --enable-addrsan=[yes/no] Enable Address Sanitizer (ADDRSAN) [default=no]],,
+	enable_addrsan=no)
+AC_SUBST(enable_addrsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -161,6 +167,13 @@ if test "$enable_ubsan" = "yes" && test "$have_ubsan" != "yes"; then
         AC_MSG_ERROR([UBSAN not supported by compiler.])
 fi
 
+if test "$enable_addrsan" = "yes" || test "$enable_addrsan" = "probe"; then
+        AC_PACKAGE_CHECK_ADDRSAN
+fi
+if test "$enable_addrsan" = "yes" && test "$have_addrsan" != "yes"; then
+        AC_MSG_ERROR([ADDRSAN not supported by compiler.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index 9dcaf52..6b6f45b 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +2 -2

index 6b9d6c2..7c78d4b 100644
@@ -155,8 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
-SANITIZER_CFLAGS += @ubsan_cflags@
-SANITIZER_LDFLAGS += @ubsan_ldflags@
+SANITIZER_CFLAGS += @addrsan_cflags@ @ubsan_cflags@
+SANITIZER_LDFLAGS += @addrsan_ldflags@ @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

m4/package_sanitizer.m4 +20 -0

index a6673f3..06031ad 100644
@@ -17,3 +17,23 @@ AC_DEFUN([AC_PACKAGE_CHECK_UBSAN],
     AC_SUBST(ubsan_cflags)
     AC_SUBST(ubsan_ldflags)
   ])
+
+AC_DEFUN([AC_PACKAGE_CHECK_ADDRSAN],
+  [ AC_MSG_CHECKING([if C compiler supports ADDRSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    ADDRSAN_FLAGS="-fsanitize=address"
+    CFLAGS="$CFLAGS $ADDRSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $ADDRSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [addrsan_cflags=$ADDRSAN_FLAGS]
+        [addrsan_ldflags=$ADDRSAN_FLAGS]
+        [have_addrsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_addrsan)
+    AC_SUBST(addrsan_cflags)
+    AC_SUBST(addrsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:15 UTC #20 lore raw

PATCH 9/10 misc: enable thread

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

---
 configure.ac            |   17 +++++++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    4 ++--
 m4/package_sanitizer.m4 |   20 ++++++++++++++++++++
 4 files changed, 41 insertions(+), 4 deletions(-)

configure.ac +17 -0

index b101e05..947392c 100644
@@ -84,6 +84,12 @@ AC_ARG_ENABLE(addrsan,
 	enable_addrsan=no)
 AC_SUBST(enable_addrsan)
 
+# Enable THREADSAN; set enable_threadsan=yesdefault to enable autoprobe.
+AC_ARG_ENABLE(threadsan,
+[ --enable-threadsan=[yes/no] Enable Thread Sanitizer (THREADSAN) [default=no]],,
+	enable_threadsan=no)
+AC_SUBST(enable_threadsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -174,6 +180,17 @@ if test "$enable_addrsan" = "yes" && test "$have_addrsan" != "yes"; then
         AC_MSG_ERROR([ADDRSAN not supported by compiler.])
 fi
 
+if test "$enable_threadsan" = "yes" || test "$enable_threadsan" = "yesdefault"; then
+        AC_PACKAGE_CHECK_THREADSAN
+fi
+if test "$enable_threadsan" = "yes" && test "$have_threadsan" != "yes"; then
+        AC_MSG_ERROR([THREADSAN not supported by compiler.])
+fi
+
+if test "$have_threadsan" = "yes" && test "$have_addrsan" = "yes"; then
+        AC_MSG_WARN([ADDRSAN and THREADSAN are not known to work together.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index 6b6f45b..f186d79 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +2 -2

index 7c78d4b..2df12a7 100644
@@ -155,8 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
-SANITIZER_CFLAGS += @addrsan_cflags@ @ubsan_cflags@
-SANITIZER_LDFLAGS += @addrsan_ldflags@ @ubsan_ldflags@
+SANITIZER_CFLAGS += @addrsan_cflags@ @threadsan_cflags@ @ubsan_cflags@
+SANITIZER_LDFLAGS += @addrsan_ldflags@ @threadsan_ldflags@ @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

m4/package_sanitizer.m4 +20 -0

index 06031ad..41b7299 100644
@@ -37,3 +37,23 @@ AC_DEFUN([AC_PACKAGE_CHECK_ADDRSAN],
     AC_SUBST(addrsan_cflags)
     AC_SUBST(addrsan_ldflags)
   ])
+
+AC_DEFUN([AC_PACKAGE_CHECK_THREADSAN],
+  [ AC_MSG_CHECKING([if C compiler supports THREADSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    THREADSAN_FLAGS="-fsanitize=thread"
+    CFLAGS="$CFLAGS $THREADSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $ADRSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [threadsan_cflags=$THREADSAN_FLAGS]
+        [threadsan_ldflags=$THREADSAN_FLAGS]
+        [have_threadsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_threadsan)
+    AC_SUBST(threadsan_cflags)
+    AC_SUBST(threadsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:24 UTC #21 lore raw

PATCH v2 9/10 misc: enable thread sanitizer if requested

From: Darrick J. Wong <[email protected]>

Enable the gcc/clang thread data corruption sanitizer if the builder requests it and it's available.

Signed-off-by Darrick J. Wong <[email protected]>

---
 configure.ac            |   17 +++++++++++++++++
 debian/rules            |    4 ++--
 include/builddefs.in    |    4 ++--
 m4/package_sanitizer.m4 |   20 ++++++++++++++++++++
 4 files changed, 41 insertions(+), 4 deletions(-)

configure.ac +17 -0

index 764b22b..210e606 100644
@@ -84,6 +84,12 @@ AC_ARG_ENABLE(addrsan,
 	enable_addrsan=no)
 AC_SUBST(enable_addrsan)
 
+# Enable THREADSAN; set enable_threadsan=probe to enable autoprobe.
+AC_ARG_ENABLE(threadsan,
+[ --enable-threadsan=[yes/no] Enable Thread Sanitizer (THREADSAN) [default=no]],,
+	enable_threadsan=no)
+AC_SUBST(enable_threadsan)
+
 #
 # If the user specified a libdir ending in lib64 do not append another
 # 64 to the library names.

@@ -174,6 +180,17 @@ if test "$enable_addrsan" = "yes" && test "$have_addrsan" != "yes"; then
         AC_MSG_ERROR([ADDRSAN not supported by compiler.])
 fi
 
+if test "$enable_threadsan" = "yes" || test "$enable_threadsan" = "probe"; then
+        AC_PACKAGE_CHECK_THREADSAN
+fi
+if test "$enable_threadsan" = "yes" && test "$have_threadsan" != "yes"; then
+        AC_MSG_ERROR([THREADSAN not supported by compiler.])
+fi
+
+if test "$have_threadsan" = "yes" && test "$have_addrsan" = "yes"; then
+        AC_MSG_WARN([ADDRSAN and THREADSAN are not known to work together.])
+fi
+
 AC_CHECK_SIZEOF([long])
 AC_CHECK_SIZEOF([char *])
 AC_TYPE_UMODE_T

debian/rules +2 -2

index 6b6f45b..f186d79 100755
@@ -20,9 +20,9 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan" ;
 diopts  = $(options) \
-	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan" ;
+	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan" ;
 checkdir = test -f debian/rules
 
 build: built

include/builddefs.in +2 -2

index 7c78d4b..2df12a7 100644
@@ -155,8 +155,8 @@ ifeq ($(HAVE_GETFSMAP),yes)
 PCFLAGS+= -DHAVE_GETFSMAP
 endif
 
-SANITIZER_CFLAGS += @addrsan_cflags@ @ubsan_cflags@
-SANITIZER_LDFLAGS += @addrsan_ldflags@ @ubsan_ldflags@
+SANITIZER_CFLAGS += @addrsan_cflags@ @threadsan_cflags@ @ubsan_cflags@
+SANITIZER_LDFLAGS += @addrsan_ldflags@ @threadsan_ldflags@ @ubsan_ldflags@
 
 GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \

m4/package_sanitizer.m4 +20 -0

index 06031ad..41b7299 100644
@@ -37,3 +37,23 @@ AC_DEFUN([AC_PACKAGE_CHECK_ADDRSAN],
     AC_SUBST(addrsan_cflags)
     AC_SUBST(addrsan_ldflags)
   ])
+
+AC_DEFUN([AC_PACKAGE_CHECK_THREADSAN],
+  [ AC_MSG_CHECKING([if C compiler supports THREADSAN])
+    OLD_CFLAGS="$CFLAGS"
+    OLD_LDFLAGS="$LDFLAGS"
+    THREADSAN_FLAGS="-fsanitize=thread"
+    CFLAGS="$CFLAGS $THREADSAN_FLAGS"
+    LDFLAGS="$LDFLAGS $ADRSAN_FLAGS"
+    AC_LINK_IFELSE([AC_LANG_PROGRAM([])],
+        [AC_MSG_RESULT([yes])]
+        [threadsan_cflags=$THREADSAN_FLAGS]
+        [threadsan_ldflags=$THREADSAN_FLAGS]
+        [have_threadsan=yes],
+        [AC_MSG_RESULT([no])])
+    CFLAGS="${OLD_CFLAGS}"
+    LDFLAGS="${OLD_LDFLAGS}"
+    AC_SUBST(have_threadsan)
+    AC_SUBST(threadsan_cflags)
+    AC_SUBST(threadsan_ldflags)
+  ])

Darrick J. Wong

author

2017-10-26 22:16 UTC #22 lore raw

PATCH 10/10 misc: fix ubsan warnings

In reply to Darrick J. Wong

From: Darrick J. Wong <[email protected]>

Fix all the things ubsan warned about.

Signed-off-by Darrick J. Wong <[email protected]>

---
 include/bitops.h   |    8 ++++----
 include/command.h  |    2 +-
 include/xfs_arch.h |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

include/bitops.h +4 -4

index 44599a7..53a5aa0 100644
@@ -13,19 +13,19 @@ static inline int fls(int x)
 	if (!x)
 		return 0;
 	if (!(x & 0xffff0000u)) {
-		x <<= 16;
+		x = (x & 0xFFFFU) << 16;
 		r -= 16;
 	}
 	if (!(x & 0xff000000u)) {
-		x <<= 8;
+		x = (x & 0xFFFFFFU) << 8;
 		r -= 8;
 	}
 	if (!(x & 0xf0000000u)) {
-		x <<= 4;
+		x = (x & 0xFFFFFFFU) << 4;
 		r -= 4;
 	}
 	if (!(x & 0xc0000000u)) {
-		x <<= 2;
+		x = (x & 0x3FFFFFFFU) << 2;
 		r -= 2;
 	}
 	if (!(x & 0x80000000u)) {

include/command.h +1 -1

index fb3f5c7..59eab96 100644
@@ -25,7 +25,7 @@
  * not iterate the command args function callout and so can be used
  * for functions like "help" that should only ever be run once.
  */
-#define CMD_FLAG_ONESHOT	(1<<31)
+#define CMD_FLAG_ONESHOT	(1U<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 #define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */

include/xfs_arch.h +1 -1

index 186cadb..34fcd4d 100644
@@ -253,7 +253,7 @@ static inline uint16_t get_unaligned_be16(void *p)
 static inline uint32_t get_unaligned_be32(void *p)
 {
 	uint8_t *__p = p;
-        return __p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
+        return (uint32_t)__p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
 }
 
 static inline uint64_t get_unaligned_be64(void *p)

Eric Sandeen

2017-10-27 13:48 UTC #23 lore raw

Re: [PATCH 10/10] misc: fix ubsan warnings

Quoted 17 lines · Darrick J. Wong

From: Darrick J. Wong <[email protected]>
Fix all the things ubsan warned about.
Signed-off-by Darrick J. Wong <[email protected]>
---
 include/bitops.h   |    8 ++++----
 include/command.h  |    2 +-
 include/xfs_arch.h |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)
include/bitops.h +0 -0
index 44599a7..53a5aa0 100644

include/bitops.h static inline int fls(int x) · line 16

Quoted 5 lines

include/bitops.h +1 -1

@@ -13,19 +13,19 @@ static inline int fls(int x)
 	if (!x)
 		return 0;
 	if (!(x & 0xffff0000u)) {
-		x <<= 16;
+		x = (x & 0xFFFFU) << 16;

xfsprogs almost exclusively uses lowercase hex - as it does in the line above your new code. ;) I can change it on the way in unless you object.

Quoted 23 lines

include/bitops.h +3 -3

 		r -= 16;
 	}
 	if (!(x & 0xff000000u)) {
-		x <<= 8;
+		x = (x & 0xFFFFFFU) << 8;
 		r -= 8;
 	}
 	if (!(x & 0xf0000000u)) {
-		x <<= 4;
+		x = (x & 0xFFFFFFFU) << 4;
 		r -= 4;
 	}
 	if (!(x & 0xc0000000u)) {
-		x <<= 2;
+		x = (x & 0x3FFFFFFFU) << 2;
 		r -= 2;
 	}
 	if (!(x & 0x80000000u)) {

include/command.h +0 -0

index fb3f5c7..59eab96 100644

include/command.h line 30

Quoted 7 lines

include/command.h +1 -1

@@ -25,7 +25,7 @@
  * not iterate the command args function callout and so can be used
  * for functions like "help" that should only ever be run once.
  */
-#define CMD_FLAG_ONESHOT	(1<<31)
+#define CMD_FLAG_ONESHOT	(1U<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 #define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */

What complained about this? cmd flags is a signed int, right, so why is this a problem? (And why are we only using the top 3 bits? And let's just do 1U on every flag if it's really needed, but why is it needed? I must be missing something.)

Quoted 19 lines

include/xfs_arch.h +1 -2

index 186cadb..34fcd4d 100644
@@ -253,7 +253,7 @@ static inline uint16_t get_unaligned_be16(void *p)
 static inline uint32_t get_unaligned_be32(void *p)
 {
 	uint8_t *__p = p;
-        return __p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
+        return (uint32_t)__p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
 }
 
 static inline uint64_t get_unaligned_be64(void *p)
 
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-10-27 16:14 UTC #24 lore raw

Re: [PATCH 10/10] misc: fix ubsan warnings

Quoted 13 lines · Eric Sandeen

Quoted 12 lines · Darrick J. Wong
From: Darrick J. Wong <[email protected]>
Fix all the things ubsan warned about.
Signed-off-by Darrick J. Wong <[email protected]>
---
 include/bitops.h   |    8 ++++----
 include/command.h  |    2 +-
 include/xfs_arch.h |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

Quoted 13 lines

include/bitops.h static inline int fls(int x) · line 16
Quoted 10 lines
include/bitops.h +1 -1
index 44599a7..53a5aa0 100644
@@ -13,19 +13,19 @@ static inline int fls(int x)
 	if (!x)
 		return 0;
 	if (!(x & 0xffff0000u)) {
-		x <<= 16;
+		x = (x & 0xFFFFU) << 16;
xfsprogs almost exclusively uses lowercase hex - as it does in the line above your new code. ;) I can change it on the way in unless you object.

Ok, sounds good to me.

Quoted 27 lines

include/bitops.h +3 -3

 		r -= 16;
 	}
 	if (!(x & 0xff000000u)) {
-		x <<= 8;
+		x = (x & 0xFFFFFFU) << 8;
 		r -= 8;
 	}
 	if (!(x & 0xf0000000u)) {
-		x <<= 4;
+		x = (x & 0xFFFFFFFU) << 4;
 		r -= 4;
 	}
 	if (!(x & 0xc0000000u)) {
-		x <<= 2;
+		x = (x & 0x3FFFFFFFU) << 2;
 		r -= 2;
 	}
 	if (!(x & 0x80000000u)) {

include/command.h +0 -1

index fb3f5c7..59eab96 100644
@@ -25,7 +25,7 @@
  * not iterate the command args function callout and so can be used
  * for functions like "help" that should only ever be run once.
  */
-#define CMD_FLAG_ONESHOT	(1<<31)

Quoted 8 lines

include/command.h line 30
Quoted 3 lines
include/command.h +1 -0
+#define CMD_FLAG_ONESHOT	(1U<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 #define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */
What complained about this? cmd flags is a signed int, right, so why is this a problem? (And why are we only using the top 3 bits? And let's just do 1U on every flag if it's really needed, but why is it needed? I must be missing something.)

I think the complaint is that "1" is signed int, and ubsan doesn't like us shifting a value bit into the sign bit.

But yeah, I suppose if we 'U'ize one of them we could do it for all.

(Though really, why not start with the lower bits?)

--D

Quoted 23 lines

Quoted 15 lines
include/xfs_arch.h +1 -2
index 186cadb..34fcd4d 100644
@@ -253,7 +253,7 @@ static inline uint16_t get_unaligned_be16(void *p)
 static inline uint32_t get_unaligned_be32(void *p)
 {
 	uint8_t *__p = p;
-        return __p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
+        return (uint32_t)__p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
 }
 
 static inline uint64_t get_unaligned_be64(void *p)
 
--
Quoted 4 lines
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Eric Sandeen

2017-10-27 16:24 UTC #25 lore raw

Re: [PATCH 10/10] misc: fix ubsan warnings

Quoted 1 line · Darrick J. Wong

On Fri, Oct 27, 2017 at 08:48:07AM -0500, Eric Sandeen wrote:

...

Quoted 18 lines

Quoted 9 lines
Quoted 9 lines
include/command.h +0 -1
index fb3f5c7..59eab96 100644
@@ -25,7 +25,7 @@
  * not iterate the command args function callout and so can be used
  * for functions like "help" that should only ever be run once.
  */
-#define CMD_FLAG_ONESHOT	(1<<31)
Quoted 8 lines
include/command.h line 30
Quoted 3 lines
include/command.h +1 -0
+#define CMD_FLAG_ONESHOT	(1U<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 #define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */
What complained about this? cmd flags is a signed int, right, so why is this a problem? (And why are we only using the top 3 bits? And let's just do 1U on every flag if it's really needed, but why is it needed? I must be missing something.)

Quoted 2 lines

I think the complaint is that "1" is signed int, and ubsan doesn't like us shifting a value bit into the sign bit.

Oh right, duh.

Quoted 3 lines

But yeah, I suppose if we 'U'ize one of them we could do it for all.
(Though really, why not start with the lower bits?)

Heh, so:

io/init.h:

#define CMD_NOFILE_OK   (1<<0)  /* command doesn't need an open file    */
#define CMD_NOMAP_OK    (1<<1)  /* command doesn't need a mapped region */
#define CMD_FOREIGN_OK  CMD_FLAG_FOREIGN_OK

include/command.h:

#define CMD_FLAG_ONESHOT        (1<<31)
#define CMD_FLAG_FOREIGN_OK     (1<<30) /* command not restricted to XFS */
#define CMD_FLAG_LIBRARY        (1<<29) /* command provided by libxcmd */

What could possibly go wrong?

-Eric

Darrick J. Wong

author

2017-10-27 16:19 UTC #26 lore raw

PATCH v2 10/10 misc: fix ubsan warnings

In reply to Darrick J. Wong

Fix all the things ubsan warned about.

Signed-off-by Darrick J. Wong <[email protected]>

---
 include/bitops.h   |    8 ++++----
 include/command.h  |    6 +++---
 include/xfs_arch.h |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

include/bitops.h +4 -4

index 44599a7..7774950 100644
@@ -13,19 +13,19 @@ static inline int fls(int x)
 	if (!x)
 		return 0;
 	if (!(x & 0xffff0000u)) {
-		x <<= 16;
+		x = (x & 0xffffu) << 16;
 		r -= 16;
 	}
 	if (!(x & 0xff000000u)) {
-		x <<= 8;
+		x = (x & 0xffffffu) << 8;
 		r -= 8;
 	}
 	if (!(x & 0xf0000000u)) {
-		x <<= 4;
+		x = (x & 0xfffffffu) << 4;
 		r -= 4;
 	}
 	if (!(x & 0xc0000000u)) {
-		x <<= 2;
+		x = (x & 0x3fffffffu) << 2;
 		r -= 2;
 	}
 	if (!(x & 0x80000000u)) {

include/command.h +3 -3

index fb3f5c7..a7fe6eb 100644
@@ -25,9 +25,9 @@
  * not iterate the command args function callout and so can be used
  * for functions like "help" that should only ever be run once.
  */
-#define CMD_FLAG_ONESHOT	(1<<31)
-#define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
-#define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */
+#define CMD_FLAG_ONESHOT	(1u << 31)
+#define CMD_FLAG_FOREIGN_OK	(1u << 30) /* command not restricted to XFS */
+#define CMD_FLAG_LIBRARY	(1u << 29) /* command provided by libxcmd */
 
 typedef int (*cfunc_t)(int argc, char **argv);
 typedef void (*helpfunc_t)(void);

include/xfs_arch.h +1 -1

index 186cadb..34fcd4d 100644
@@ -253,7 +253,7 @@ static inline uint16_t get_unaligned_be16(void *p)
 static inline uint32_t get_unaligned_be32(void *p)
 {
 	uint8_t *__p = p;
-        return __p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
+        return (uint32_t)__p[0] << 24 | __p[1] << 16 | __p[2] << 8 | __p[3];
 }
 
 static inline uint64_t get_unaligned_be64(void *p)

Goldwyn Rodrigues

2017-10-26 22:32 UTC #27 lore raw

In reply to Darrick J. Wong

Quoted 6 lines · Darrick J. Wong

Hi all,
Here is a rollup of everything I still have queued for xfsprogs 4.14. This rollup supersedes all the previous xfsprogs patches I've sent, though the first three patches in this series haven't changed since then.

Any idea when the xfs_io patches for RWF_NOWAIT and write-once I/O will be incorporated? There are a couple of tests waiting for it.

https://patchwork.kernel.org/patch/9995815/
https://patchwork.kernel.org/patch/9995811/
https://patchwork.kernel.org/patch/9995811/

Signature · 1 line

Goldwyn

Eric Sandeen

2017-10-27 00:01 UTC #28 lore raw

Quoted 11 lines · Goldwyn Rodrigues

Quoted 6 lines · Darrick J. Wong
Hi all,
Here is a rollup of everything I still have queued for xfsprogs 4.14. This rollup supersedes all the previous xfsprogs patches I've sent, though the first three patches in this series haven't changed since then.

Quoted 6 lines

Any idea when the xfs_io patches for RWF_NOWAIT and write-once I/O will be incorporated? There are a couple of tests waiting for it.
https://patchwork.kernel.org/patch/9995815/
https://patchwork.kernel.org/patch/9995811/
https://patchwork.kernel.org/patch/9995811/

https://patchwork.kernel.org/patch/9995813/

Apologies, i've been juggling too many things and xfsprogs has been on the back burner for too long. Looks like these all have reviews, I can pull them in for the 4.14 release.

Thanks,
-Eric