[scudo] Fix the logic of MaxAllowedFragmentedPages #107927
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MTE doesn't support MaxReleasedCachePages which may break the assumption that only the first 4 pages will have memory tagged.
llvmbot
added
compiler-rt
compiler-rt:scudo
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (ChiaHungDuan) ChangesMTE doesn't support MaxReleasedCachePages which may break the assumption that only the first 4 pages will have memory tagged. Full diff: https://github.com/llvm/llvm-project/pull/107927.diff 1 Files Affected:
diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 1a232b9b9fb2d7..7d85832d7fe714 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -725,7 +725,7 @@ MapAllocator<Config>::tryAllocateFromCache(const Options &Options, uptr Size,
uptr EntryHeaderPos;
uptr MaxAllowedFragmentedPages = MaxUnreleasedCachePages;
- if (UNLIKELY(useMemoryTagging<Config>(Options)))
+ if (LIKELY(!useMemoryTagging<Config>(Options)))
MaxAllowedFragmentedPages += CachedBlock::MaxReleasedCachePages;
Entry = Cache.retrieve(MaxAllowedFragmentedPages, Size, Alignment,
|
|
I'm running some tests to confirm the issue has been fixed. |
ChiaHungDuan
force-pushed
the
quick-fix
branch
from
5f6a5b3
to
6e3a54a
Compare
I updated the CL with some comments about how it causes problem for MTE |
cferris1000
reviewed
Sep 10, 2024
| // is dependent on the platform. | ||
| // We allow a certain amount of fragmentation and part of the fragmented bytes | ||
| // will be released by `releaseAndZeroPagesToOS()`. This increases the chance | ||
| // of cache hit rate and reduce the overhead to the RSS at the same time. See |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/3565 Here is the relevant piece of the build log for the reference |
ChiaHungDuan
added a commit
that referenced
this pull request
Sep 11, 2024
This reverts commit 6e854a6.
ChiaHungDuan
added a commit
that referenced
this pull request
Sep 11, 2024
Reverts #107927 We are supposed to check the MaxAllowedFragmentedPages instead.
jrguzman-ms
pushed a commit
to msft-mirror-aosp/platform.external.scudo
that referenced
this pull request
Sep 12, 2024
Reverts llvm/llvm-project#107927 We are supposed to check the MaxAllowedFragmentedPages instead. GitOrigin-RevId: 76151c449080b7239c8b442291514a4300d51cba Change-Id: I16cbbbb22576eb47bf2b7aa30156ac17425308da
VitaNuo
pushed a commit
to VitaNuo/llvm-project
that referenced
this pull request
Sep 12, 2024
MTE doesn't support MaxReleasedCachePages which may break the assumption that only the first 4 pages will have memory tagged.
VitaNuo
pushed a commit
to VitaNuo/llvm-project
that referenced
this pull request
Sep 12, 2024
) Reverts llvm#107927 We are supposed to check the MaxAllowedFragmentedPages instead.
bherrera
pushed a commit
to misttech/mist-os
that referenced
this pull request
Sep 18, 2024
…FragmentedPages" (#108130) Reverts llvm/llvm-project#107927 We are supposed to check the MaxAllowedFragmentedPages instead. GitOrigin-RevId: 8c10827543c8e11689cef95c6e3d3378482887c2 Original-Revision: c682eaae1d2ca9d8e00604ee2712da6d01ff821d Roller-URL: https://ci.chromium.org/b/8737117427431755313 CQ-Do-Not-Cancel-Tryjobs: true Change-Id: I4bcb52dffad477f6cdc1415fa658da6646b673be Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1116349
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Sep 18, 2024
…edPages" (#108130) Reverts llvm/llvm-project#107927 We are supposed to check the MaxAllowedFragmentedPages instead. GitOrigin-RevId: 8c10827543c8e11689cef95c6e3d3378482887c2 Original-Revision: c682eaae1d2ca9d8e00604ee2712da6d01ff821d Change-Id: I3edf550c4622ad9aabd159ab9e032d12f3052bf4
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Sep 18, 2024
… logic of MaxAllowedFragmentedPages" (#108130) Reverts llvm/llvm-project#107927 We are supposed to check the MaxAllowedFragmentedPages instead. GitOrigin-RevId: 4f5f037eb0225da54cb4de75e1382a653e9d1c54 Original-Revision: c682eaae1d2ca9d8e00604ee2712da6d01ff821d Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1116349 Original-Revision: b1e8b615193f7d7a91e62963dc7a31128a10b103 Change-Id: Iffd2ddb6324b23914467ab1e2564b3e8ffd1effe
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MTE doesn't support MaxReleasedCachePages which may break the assumption that only the first 4 pages will have memory tagged.