On Sun, 23 Feb 2003, Terry Lambert wrote:

> David Syphers wrote:
> > Okay, I've verified that the problem is due to rev. 1.39 of
> > /usr/src/sys/ufs/ffs/fs.h. Peter Wemm pointed out that the problem is not the
> > commit, but gcc's bad handling of 64-bit operations. Nonetheless, this commit
> > does break world for a lot of people... is there some official solution? The
> > make.conf line only works for UFS1 - if it's set to UFS2, buildworld still
> > fails. (Am I correct in assuming a 5.0-R install defaults to UFS2?)
>
> Yes.  And 4.x upgrades, which are far more common, default to UFS.
>
> Personally, I think the changes should be #ifdef'ed the current
> version of GCC; when GCC rev's, hopefully its 64 bit operations
> handling will have improved.

This would wrong, since ufs2 depends on the changes to actually work
for file systems larger than about 1TB.

Blaming gcc's 64-bit operations seems to be wrong anyway.  gcc actually
has good handling of (32, 32) -> 64 bit operations which are the only
types involved here in at least fs.h, boot2 still fits for me when it
is compiled the previous version of gcc.  It has 19 bytes to spare:

%%%
   text    data     bss     dec     hex filename
    512       0       0     512     200 obj-UFS1_AND_UFS2/boot1.o
    512       0       0     512     200 obj-UFS1_AND_UFS2/boot1.out
   5228      25    2120    7373    1ccd obj-UFS1_AND_UFS2/boot2.o
   5439      25    2196    7660    1dec obj-UFS1_AND_UFS2/boot2.out
     91       0       0      91      5b obj-UFS1_AND_UFS2/sio.o
    512       0       0     512     200 obj-UFS1_ONLY/boot1.o
    512       0       0     512     200 obj-UFS1_ONLY/boot1.out
   4999      25    1864    6888    1ae8 obj-UFS1_ONLY/boot2.o
   5211      25    1940    7176    1c08 obj-UFS1_ONLY/boot2.out
     91       0       0      91      5b obj-UFS1_ONLY/sio.o
    512       0       0     512     200 obj-UFS2_ONLY/boot1.o
    512       0       0     512     200 obj-UFS2_ONLY/boot1.out
   5046      25    1992    7063    1b97 obj-UFS2_ONLY/boot2.o
   5255      25    2068    7348    1cb4 obj-UFS2_ONLY/boot2.out
     91       0       0      91      5b obj-UFS2_ONLY/sio.o
%%%

The fixes in fs.h are:

% Index: fs.h
% ===================================================================
% RCS file: /home/ncvs/src/sys/ufs/ffs/fs.h,v
% retrieving revision 1.38
% retrieving revision 1.39
% diff -u -1 -r1.38 -r1.39
% --- fs.h      10 Jan 2003 06:59:34 -0000      1.38
% +++ fs.h      22 Feb 2003 00:19:26 -0000      1.39
% @@ -33,3 +33,3 @@
%   *   @(#)fs.h        8.13 (Berkeley) 3/21/95
% - * $FreeBSD: src/sys/ufs/ffs/fs.h,v 1.38 2003/01/10 06:59:34 marcel Exp $
% + * $FreeBSD: src/sys/ufs/ffs/fs.h,v 1.39 2003/02/22 00:19:26 mckusick Exp $
%   */
% @@ -498,3 +498,3 @@
%   */
% -#define      cgbase(fs, c)   ((ufs2_daddr_t)((fs)->fs_fpg * (c)))
% +#define      cgbase(fs, c)   (((ufs2_daddr_t)(fs)->fs_fpg) * (c))
%  #define      cgdmin(fs, c)   (cgstart(fs, c) + (fs)->fs_dblkno)      /* 1st data */

This changes a (32, 32) -> 32 bit (possibly overflowing) operation to a
(32, 32) -> 64 bit (never overflowing) operation.  The 1386 hardware
already does the wider operation so all gcc has to do is not discard the
high 32-bits, which it does reasonably well.

% @@ -543,5 +543,5 @@
%  #define lfragtosize(fs, frag)        /* calculates ((off_t)frag * fs->fs_fsize) */ \
% -     ((off_t)(frag) << (fs)->fs_fshift)
% +     (((off_t)(frag)) << (fs)->fs_fshift)
%  #define lblktosize(fs, blk)  /* calculates ((off_t)blk * fs->fs_bsize) */ \
% -     ((off_t)(blk) << (fs)->fs_bshift)
% +     (((off_t)(blk)) << (fs)->fs_bshift)
%  /* Use this only when `blk' is known to be small, e.g., < NDADDR. */

These changes have no effect except to add style bugs (see below).
The casts were inserted in the correct place in rev.1.7 to fix similar
overflow bugs for _files_ larger than 2GB.  Now we're fixing overflow
for block numbers larger than 2G.

% @@ -573,3 +573,3 @@
%       (fs)->fs_cstotal.cs_nffree - \
% -     ((off_t)((fs)->fs_dsize) * (percentreserved) / 100))
% +     (((off_t)((fs)->fs_dsize)) * (percentreserved) / 100))
%

This change has no effect for many reasons:
- it just adds style bugs (see below).
- the macro is not used in the boot blocks.
- fs_dsize already  happens to have the same type as off_t (both have type
  int64_t).  off_t is a bogus type to cast to anyway.  We start with a
  block count and multiply by a percentage.  This is unrelated to file
  sizes, but overflow happens to be prevented by the maxiumum percentage
  (100) being smaller than the minimum block size (4096).

All of these changes add style bugs IMO.  Except for the change to
cgbase(), they add redundant parentheses around "(cast)(foo)->bar",
but properly parenthesized macros are hard enough to read with only
non-redundant parentheses.  The change to cgbase() moves parentheses
so that they are redundant instead of just wrong.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to