fix: copy_file skip files larger than 1GB (Fixes #4039) by IamHuskar · Pull Request #4051 · rr-debugger/rr

IamHuskar · 2026-02-07T08:10:00Z

The original PR at #4048 . should use rebase instead of merge.

IamHuskar · 2026-02-07T08:14:30Z

Why is the MAP_SHARED check placed after copy_file?

Line 1001 in 1f6d9a0

!(km.flags() & MAP_SHARED)) {

Can we change the order of these if conditions?
If the km region is MAP_SHARED, src.setTrace will be executed, but at that point, the file copying has already been completed.

rocallahan · 2026-02-10T08:22:07Z

src/TraceStream.cc

+      // the size of km is not equal to the file size obtained through the fstat
+      struct stat src_stat;
+      ScopedFd src_fd(file_name.c_str(), O_RDONLY);
+      if (src_fd.is_open() && (fstat(src_fd.get(), &src_stat) == 0)) {


You can use the stat that's a parameter to TraceWriter::write_mapped_region().

You can use the stat that's a parameter to TraceWriter::write_mapped_region().

I searched through all the code and found that some function calls to write_mapped_region pass xxx.fake_stat() as an argument, meaning the stat here might be the result of fake_stat. Can we still directly use stat here?

rocallahan · 2026-02-10T08:25:02Z

src/test/memfd_create_dotnet_huge_mapping.c

+typedef void (*simple_func_t)(void);
+
+#ifdef __x86_64__
+static const unsigned char x86_ret_instruction = 0xC3; // ret


I don't think you actually need to map and execute code. All you need to do is construct a really large totally empty mapping, map it PROT_EXEC, and hopefully that will cause the test to time out if this bug is not fixed properly.

IamHuskar · 2026-02-10T10:31:36Z

It seems that after running scripts/reformat.sh, the code formatting changes dramatically. I wonder if it's due to the version of clang-format. How should I format my code?
@rocallahan

rocallahan · 2026-02-13T22:41:54Z

src/TraceStream.cc

    if ((km.prot() & PROT_EXEC) &&
-        copy_file(km.fsname(), file_name, &backing_file_name) &&
-        !(km.flags() & MAP_SHARED)) {
+        (!(km.flags() & MAP_SHARED) &&


The first ( (and the matching )) is unnecessary.

rocallahan · 2026-02-13T22:42:11Z

src/TraceStream.cc

-        copy_file(km.fsname(), file_name, &backing_file_name) &&
-        !(km.flags() & MAP_SHARED)) {
+        (!(km.flags() & MAP_SHARED) &&
+        (stat.st_size <= 1024 * 1024 * 1024/*1GB*/) &&


This ( and the matching ) are also unnecessary.

rocallahan · 2026-02-13T22:42:45Z

src/test/memfd_create_dotnet_huge_mapping.c

@@ -0,0 +1,61 @@
+#include "util.h"
+#include <time.h>


Pretty sure this #include is not necessary.

rocallahan · 2026-02-13T22:43:46Z

src/test/memfd_create_dotnet_huge_mapping.c

+unsigned long long MaxDoubleMappedSize = 20 * _1GB;
+#define PAGE_SIZE_4K 4096
+int test_ftruncate_huge_mapping(void)
+{


Please manually format this the way we normally do, with { on the same line as the function declaration.

rocallahan · 2026-02-13T22:43:58Z

src/test/memfd_create_dotnet_huge_mapping.c

+  int ret=-1;
+  int fd = memfd_create("double_mapper_test", MFD_CLOEXEC);
+  do
+  {


Similar here and below.

rocallahan · 2026-02-13T22:47:28Z

src/test/memfd_create_dotnet_huge_mapping.c

+  int fd = memfd_create("double_mapper_test", MFD_CLOEXEC);
+  do
+  {
+    if (fd == -1)


Use test_assert(fd >= 0) instead.

rocallahan · 2026-02-13T22:47:36Z

src/test/memfd_create_dotnet_huge_mapping.c

+      atomic_puts("memfd_create failed");
+      break;
+    }
+    if (ftruncate(fd, MaxDoubleMappedSize) == -1)


rocallahan · 2026-02-13T22:47:43Z

src/test/memfd_create_dotnet_huge_mapping.c

+      break;
+    }
+    void* executable_addr = mmap(NULL, PAGE_SIZE_4K, PROT_READ | PROT_EXEC, MAP_PRIVATE, fd, 0);
+    if (executable_addr == MAP_FAILED)


rocallahan · 2026-02-13T22:48:07Z

src/test/memfd_create_dotnet_huge_mapping.c

+      atomic_puts("mmap for executable mapping failed");
+      break;
+    }
+    munmap(executable_addr, PAGE_SIZE_4K);


You don't really need to munmap, it's fine to leak it in a test.

rocallahan · 2026-02-13T22:48:19Z

src/test/memfd_create_dotnet_huge_mapping.c

+#define _1GB (1024 * 1024 * 1024ULL)
+unsigned long long MaxDoubleMappedSize = 20 * _1GB;
+#define PAGE_SIZE_4K 4096
+int test_ftruncate_huge_mapping(void)


This function can return void.

fix: copy_file skip files larger than 1GB (Fixes rr-debugger#4039)

c445f53

rocallahan requested changes Feb 10, 2026

View reviewed changes

Iamhuskar added 3 commits February 10, 2026 17:45

fix:Get fd size from args;Simplify test case; #4093

c84125b

refactor(copy_file_or_trace): simplify condition logic for readability

6ddef36

style: fix indentation

2517dff

rocallahan requested changes Feb 13, 2026

View reviewed changes

IamHuskar force-pushed the fix_dotnet_doublemapping branch from 6d491ec to 2517dff Compare February 14, 2026 07:31

IamHuskar closed this Feb 14, 2026

IamHuskar deleted the fix_dotnet_doublemapping branch February 14, 2026 08:41

Comments

Conversation

IamHuskar commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IamHuskar commented Feb 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IamHuskar commented Feb 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IamHuskar commented Feb 7, 2026 •

edited

Loading