-
Notifications
You must be signed in to change notification settings - Fork 650
fix: copy_file skip files larger than 1GB (Fixes #4039) #4051
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -995,10 +995,13 @@ TraceWriter::RecordInTrace TraceWriter::write_mapped_region( | |
| // debuggers can't find the file, but the Linux loader doesn't create | ||
| // shared mappings so situations where a shared-mapped executable contains | ||
| // usable debug info should be very rare at best... | ||
| // copy files when the mapping is PROT_EXEC, unless the file is too big | ||
| // sometimes km.size() does not equal the file size from fstat(). | ||
| string backing_file_name; | ||
| if ((km.prot() & PROT_EXEC) && | ||
| copy_file(km.fsname(), file_name, &backing_file_name) && | ||
| !(km.flags() & MAP_SHARED)) { | ||
| (!(km.flags() & MAP_SHARED) && | ||
| (stat.st_size <= 1024 * 1024 * 1024/*1GB*/) && | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
| copy_file(km.fsname(), file_name, &backing_file_name))) { | ||
| src.initFile().setBackingFileName(str_to_data(backing_file_name)); | ||
| } else { | ||
| src.setTrace(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| #include "util.h" | ||
| #include <time.h> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty sure this #include is not necessary. |
||
| #define memfd_create(...) syscall(__NR_memfd_create, __VA_ARGS__) | ||
| /* | ||
| https://github.com/dotnet/runtime/blob/23aeecc9f91a9ae0a211702dbd849c90cdd81d36/src/coreclr/minipal/Unix/doublemapping.cpp#L85 | ||
| #ifdef TARGET_64BIT | ||
| static const off_t MaxDoubleMappedSize = 2048ULL*1024*1024*1024; | ||
| #else | ||
| static const off_t MaxDoubleMappedSize = UINT_MAX; | ||
| #endif | ||
| To prevent bugs that could result in writing a 2TB file, a 20GB limit is used instead. | ||
| */ | ||
| #define _1GB (1024 * 1024 * 1024ULL) | ||
| unsigned long long MaxDoubleMappedSize = 20 * _1GB; | ||
| #define PAGE_SIZE_4K 4096 | ||
| int test_ftruncate_huge_mapping(void) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function can return void. |
||
| { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please manually format this the way we normally do, with |
||
| int ret=-1; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use spaces the way we normally do. |
||
| int fd = memfd_create("double_mapper_test", MFD_CLOEXEC); | ||
| do | ||
| { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar here and below.
IamHuskar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (fd == -1) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
| { | ||
| atomic_puts("memfd_create failed"); | ||
| break; | ||
| } | ||
| if (ftruncate(fd, MaxDoubleMappedSize) == -1) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| { | ||
| atomic_puts("ftruncate failed"); | ||
| break; | ||
| } | ||
| void* executable_addr = mmap(NULL, PAGE_SIZE_4K, PROT_READ | PROT_EXEC, MAP_PRIVATE, fd, 0); | ||
| if (executable_addr == MAP_FAILED) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here. |
||
| { | ||
| atomic_puts("mmap for executable mapping failed"); | ||
| break; | ||
| } | ||
| munmap(executable_addr, PAGE_SIZE_4K); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't really need to munmap, it's fine to leak it in a test. |
||
| ret = 0; | ||
| } while (0); | ||
| if(fd!=-1){ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And use spaces here like the rest of our code. |
||
| close(fd); | ||
| } | ||
| return ret; | ||
| } | ||
|
|
||
| int main(void) | ||
| { | ||
| #if defined(__i386__) | ||
| atomic_puts("Skipping test on 32 bit"); | ||
| #else | ||
| struct timespec start, end; | ||
| test_assert(-1 != clock_gettime(CLOCK_MONOTONIC, &start)); | ||
| test_ftruncate_huge_mapping(); | ||
| test_assert(-1 != clock_gettime(CLOCK_MONOTONIC, &end)); | ||
| //check timeout | ||
| test_assert((end.tv_sec - start.tv_sec) <= 2); | ||
| #endif | ||
| atomic_puts("EXIT-SUCCESS"); | ||
| return 0; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first
((and the matching)) is unnecessary.