Skip to content

release/20.x: [libcxx] Use _ftelli64/_fseeki64 on Windows (#123128) #124922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jan 29, 2025

Backport 86e20b0

Requested by: @mstorsjo

@llvmbot llvmbot requested a review from a team as a code owner January 29, 2025 13:31
@llvmbot llvmbot added this to the LLVM 20.X Release milestone Jan 29, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jan 29, 2025

@philnik777 What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from philnik777 January 29, 2025 13:31
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 29, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jan 29, 2025

@llvm/pr-subscribers-libcxx

Author: None (llvmbot)

Changes

Backport 86e20b0

Requested by: @mstorsjo


Full diff: https://github.com/llvm/llvm-project/pull/124922.diff

2 Files Affected:

  • (modified) libcxx/include/fstream (+29-26)
  • (modified) libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/offset_range.pass.cpp (-6)
diff --git a/libcxx/include/fstream b/libcxx/include/fstream
index f0e9425e0a53d9..de5c07035dba9c 100644
--- a/libcxx/include/fstream
+++ b/libcxx/include/fstream
@@ -216,12 +216,6 @@ typedef basic_fstream<wchar_t> wfstream;
 _LIBCPP_PUSH_MACROS
 #  include <__undef_macros>
 
-#  if !defined(_LIBCPP_MSVCRT) && !defined(_NEWLIB_VERSION)
-#    define _LIBCPP_HAS_OFF_T_FUNCTIONS 1
-#  else
-#    define _LIBCPP_HAS_OFF_T_FUNCTIONS 0
-#  endif
-
 #  if _LIBCPP_HAS_FILESYSTEM && _LIBCPP_HAS_LOCALIZATION
 
 _LIBCPP_BEGIN_NAMESPACE_STD
@@ -362,6 +356,9 @@ private:
   bool __read_mode();
   void __write_mode();
 
+  _LIBCPP_HIDE_FROM_ABI static int __fseek(FILE* __file, pos_type __offset, int __whence);
+  _LIBCPP_HIDE_FROM_ABI static pos_type __ftell(FILE* __file);
+
   _LIBCPP_EXPORTED_FROM_ABI friend FILE* __get_ostream_file(ostream&);
 
   // There are multiple (__)open function, they use different C-API open
@@ -936,31 +933,42 @@ basic_filebuf<_CharT, _Traits>::seekoff(off_type __off, ios_base::seekdir __way,
   default:
     return pos_type(off_type(-1));
   }
-#    if !_LIBCPP_HAS_OFF_T_FUNCTIONS
-  if (fseek(__file_, __width > 0 ? __width * __off : 0, __whence))
+  if (__fseek(__file_, __width > 0 ? __width * __off : 0, __whence))
     return pos_type(off_type(-1));
-  pos_type __r = ftell(__file_);
-#    else
-  if (::fseeko(__file_, __width > 0 ? __width * __off : 0, __whence))
-    return pos_type(off_type(-1));
-  pos_type __r = ftello(__file_);
-#    endif
+  pos_type __r = __ftell(__file_);
   __r.state(__st_);
   return __r;
 }
 
+template <class _CharT, class _Traits>
+int basic_filebuf<_CharT, _Traits>::__fseek(FILE* __file, pos_type __offset, int __whence) {
+#    if defined(_LIBCPP_MSVCRT_LIKE)
+  return _fseeki64(__file, __offset, __whence);
+#    elif defined(_NEWLIB_VERSION)
+  return fseek(__file, __offset, __whence);
+#    else
+  return ::fseeko(__file, __offset, __whence);
+#    endif
+}
+
+template <class _CharT, class _Traits>
+typename basic_filebuf<_CharT, _Traits>::pos_type basic_filebuf<_CharT, _Traits>::__ftell(FILE* __file) {
+#    if defined(_LIBCPP_MSVCRT_LIKE)
+  return _ftelli64(__file);
+#    elif defined(_NEWLIB_VERSION)
+  return ftell(__file);
+#    else
+  return ftello(__file);
+#    endif
+}
+
 template <class _CharT, class _Traits>
 typename basic_filebuf<_CharT, _Traits>::pos_type
 basic_filebuf<_CharT, _Traits>::seekpos(pos_type __sp, ios_base::openmode) {
   if (__file_ == nullptr || sync())
     return pos_type(off_type(-1));
-#    if !_LIBCPP_HAS_OFF_T_FUNCTIONS
-  if (fseek(__file_, __sp, SEEK_SET))
+  if (__fseek(__file_, __sp, SEEK_SET))
     return pos_type(off_type(-1));
-#    else
-  if (::fseeko(__file_, __sp, SEEK_SET))
-    return pos_type(off_type(-1));
-#    endif
   __st_ = __sp.state();
   return __sp;
 }
@@ -1007,13 +1015,8 @@ int basic_filebuf<_CharT, _Traits>::sync() {
         }
       }
     }
-#    if !_LIBCPP_HAS_OFF_T_FUNCTIONS
-    if (fseek(__file_, -__c, SEEK_CUR))
+    if (__fseek(__file_, -__c, SEEK_CUR))
       return -1;
-#    else
-    if (::fseeko(__file_, -__c, SEEK_CUR))
-      return -1;
-#    endif
     if (__update_st)
       __st_ = __state;
     __extbufnext_ = __extbufend_ = __extbuf_;
diff --git a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/offset_range.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/offset_range.pass.cpp
index 6ffe750564c2c9..c6e07d045e1458 100644
--- a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/offset_range.pass.cpp
+++ b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/offset_range.pass.cpp
@@ -11,12 +11,6 @@
 // Test that we can seek using offsets larger than 32 bit, and that we can
 // retrieve file offsets larger than 32 bit.
 
-// On MSVC targets, we only use the 32 bit fseek/ftell functions. For MinGW
-// targets, we use fseeko/ftello, but the user needs to define
-// _FILE_OFFSET_BITS=64 to make them 64 bit.
-//
-// XFAIL: target={{.*}}-windows{{.*}}
-
 // On 32 bit Android platforms, off_t is 32 bit by default. By defining
 // _FILE_OFFSET_BITS=64, one gets a 64 bit off_t, but the corresponding
 // 64 bit ftello/fseeko functions are only available since Android API 24 (7.0).

This allows using the full 64 bit range for file offsets.

This should fix the issue reported downstream at
mstorsjo/llvm-mingw#462.

(cherry picked from commit 86e20b0)
@tstellar tstellar merged commit d777df5 into llvm:release/20.x Feb 1, 2025
12 of 15 checks passed
Copy link

github-actions bot commented Feb 1, 2025

@mstorsjo (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants