diff mbox

[Linux] fcntl: add new F_OFD_*32 constants and handle them appropriately

Message ID 01b37ffd-4ec7-ce69-c7e1-891162f3e4d8@cs.ucla.edu
State New, archived
Headers show

Commit Message

Paul Eggert Aug. 18, 2016, 7:36 p.m. UTC
Zack Weinberg wrote:
> We could change the libc headers used on old-ILP32 ABIs so that
> _FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
> headers).  This would break the ABI of every shared library that
> exports a structure (transitively) containing a field of type off_t,
> ino_t, fsblkcnt_t, fsfilcnt_t, or rlim_t.

As I understand it, most (all important?) such libraries are already compiled 
with _FILE_OFFSET_BITS=64 anyway, so their ABIs wouldn't break.

How about if we start the transition by deprecating the use of 32-bit off_t in 
user or library code on platforms with 32-bit long? The attached patch plus lots 
of similar patches, say (this is just a sketch). Really, it's long since time 
that file offsets were 64 bits.

Comments

Zack Weinberg Aug. 19, 2016, 1:20 p.m. UTC | #1
On 08/18/2016 03:36 PM, Paul Eggert wrote:
> Zack Weinberg wrote:
>> We could change the libc headers used on old-ILP32 ABIs so that
>> _FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
>> headers).  This would break the ABI of every shared library that
>> exports a structure (transitively) containing a field of type off_t,
>> ino_t, fsblkcnt_t, fsfilcnt_t, or rlim_t.
> 
> As I understand it, most (all important?) such libraries are already
> compiled with _FILE_OFFSET_BITS=64 anyway, so their ABIs wouldn't break.

I'd feel a lot safer about changing the default if we had some way of
tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have
that?  It seems like something that could probably be done with existing
object-annotation goo, if we don't.

> How about if we start the transition by deprecating the use of 32-bit
> off_t in user or library code on platforms with 32-bit long? The
> attached patch plus lots of similar patches, say (this is just a
> sketch). Really, it's long since time that file offsets were 64 bits.

I'd support this patch (better with __attribute_deprecated_msg__() and
an explanation that it's not off_t that's deprecated, it's *32-bit* off_t).

zw
Joseph Myers Aug. 19, 2016, 3:02 p.m. UTC | #2
On Fri, 19 Aug 2016, Zack Weinberg wrote:

> I'd feel a lot safer about changing the default if we had some way of
> tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
> and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have

But combining such files is fine if _FILE_OFFSET_BITS is not part of the 
ABI for those libraries (and there are plenty of libraries for which 
that's the case, even if they should be using _FILE_OFFSET_BITS=64 
internally).

My view is that changing the default is better then the status quo where 
distributions provide libraries compiled with a mixture of 
_FILE_OFFSET_BITS settings and users need to match the setting for a given 
library where it's part of the ABI.  The change should rapidly lead to 
very few people using _FILE_OFFSET_BITS=32 (but making that into just a 
glibc-internal "don't redirect functions in headers" mode for compiling 
the glibc compat versions of functions for old binaries, rather than 
supporting _FILE_OFFSET_BITS=32 for users, would be harder).
Zack Weinberg Aug. 19, 2016, 3:45 p.m. UTC | #3
On Fri, Aug 19, 2016 at 11:02 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 19 Aug 2016, Zack Weinberg wrote:
>> I'd feel a lot safer about changing the default if we had some way of
>> tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
>> and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have
>
> But combining such files is fine if _FILE_OFFSET_BITS is not part of the
> ABI for those libraries (and there are plenty of libraries for which
> that's the case, even if they should be using _FILE_OFFSET_BITS=64
> internally).

(I'm taking this as an implied statement that we *don't* have any such
tagging now.)

It's true that tagging would over-estimate incompatibility, but if
doing it reduces the risk of invisible breakage enough that we can
feel confident about changing the default, maybe we should do it
anyway.

(I'd still worry about invisible breakage of _applications_ due to
e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
nothing the _library_ can do about that.)

zw
Joseph Myers Aug. 19, 2016, 4:58 p.m. UTC | #4
On Fri, 19 Aug 2016, Zack Weinberg wrote:

> (I'd still worry about invisible breakage of _applications_ due to
> e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
> nothing the _library_ can do about that.)

We have the statement from the previous discussion that the change would 
break INN but distributions already dealt with LFS issues for it 
<https://sourceware.org/ml/libc-alpha/2014-03/msg00354.html>, but I expect 
that's the rare case, and not a significant argument against making the 
change.
Zack Weinberg Aug. 19, 2016, 7:50 p.m. UTC | #5
On Fri, Aug 19, 2016 at 12:58 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 19 Aug 2016, Zack Weinberg wrote:
>
>> (I'd still worry about invisible breakage of _applications_ due to
>> e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
>> nothing the _library_ can do about that.)
>
> We have the statement from the previous discussion that the change would
> break INN but distributions already dealt with LFS issues for it
> <https://sourceware.org/ml/libc-alpha/2014-03/msg00354.html>, but I expect
> that's the rare case, and not a significant argument against making the
> change.

INN was on-disk data structures containing off_t quantities, wasn't
it?  I was thinking more like

   struct stat st;
   fstat(fd, &st);
   buf = malloc(st.st_size);
   read(fd, buf, st.st_size);

which is catastrophically wrong with 64-bit off_t *but only when
size_t is smaller*, i.e. this won't have been flushed out by testing
in an LP64 environment.  Neither gcc nor clang issues any diagnostic
for the truncation in the call to malloc, even at -std=c11 -Wall
-Wextra -pedantic, and nothing bad will happen at runtime unless the
program is actually supplied with a >2GB file.  So it's a lurking
at-least-crash-perhaps-exploit.

(You *can* get a warning with -Wconversion, but who uses that?  I
wonder if there's something glibc can do in its headers to make
warnings happen in this circumstance.  Nothing comes to mind.)

zw
diff mbox

Patch

diff --git a/libio/stdio.h b/libio/stdio.h
index e37f901..a5f65e9 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -88,7 +88,11 @@  typedef _G_va_list va_list;
 #if defined __USE_UNIX98 || defined __USE_XOPEN2K
 # ifndef __off_t_defined
 # ifndef __USE_FILE_OFFSET64
+#  if !_SUPPRESS_FILE_OFFSET_DEPRECATION && __LONG_MAX__ < __LONG_LONG_MAX__
+typedef __off_t off_t __attribute_deprecated__;
+#  else
 typedef __off_t off_t;
+#  endif
 # else
 typedef __off64_t off_t;
 # endif