Skip to content

[scudo] Clean the TODO in list.h #119323

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
Dec 10, 2024
Merged

Conversation

ChiaHungDuan
Copy link
Contributor

  • Finished the type and size verification
  • Remove the TODO for checking if array size can be fit into LinkTy because if there's a truncation happens, other DCHECK like offset checking will catch the failure. In addition, it's supposed to be a rare case.

* Finished the type and size verification
* Remove the TODO for checking if array size can be fit into LinkTy
  because if there's a truncation happens, other DCHECK like offset
  checking will catch the failure. In addition, it's supposed to be a
  rare case.
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (ChiaHungDuan)

Changes
  • Finished the type and size verification
  • Remove the TODO for checking if array size can be fit into LinkTy because if there's a truncation happens, other DCHECK like offset checking will catch the failure. In addition, it's supposed to be a rare case.

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

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/list.h (+7-6)
diff --git a/compiler-rt/lib/scudo/standalone/list.h b/compiler-rt/lib/scudo/standalone/list.h
index 5c34cbb049e602..e7c69e5bb88d67 100644
--- a/compiler-rt/lib/scudo/standalone/list.h
+++ b/compiler-rt/lib/scudo/standalone/list.h
@@ -48,10 +48,11 @@ class LinkOp {
 
 template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
 public:
-  using LinkTy = decltype(T::Next);
+  using LinkTy = typename assertSameType<
+      typename removeConst<decltype(T::Next)>::type,
+      typename removeConst<decltype(T::EndOfListVal)>::type>::type;
 
   LinkOp() = default;
-  // TODO: Check if the `BaseSize` can fit in `Size`.
   LinkOp(T *BaseT, uptr BaseSize)
       : Base(BaseT), Size(static_cast<LinkTy>(BaseSize)) {}
   void init(T *LinkBase, uptr BaseSize) {
@@ -70,11 +71,12 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
   }
   // Set `X->Next` to `Next`.
   void setNext(T *X, T *Next) const {
-    // TODO: Check if the offset fits in the size of `LinkTy`.
-    if (Next == nullptr)
+    if (Next == nullptr) {
       X->Next = getEndOfListVal();
-    else
+    } else {
+      DCHECK_LE(static_cast<LinkTy>(Next - Base), Size);
       X->Next = static_cast<LinkTy>(Next - Base);
+    }
   }
 
   T *getPrev(T *X) const {
@@ -94,7 +96,6 @@ template <class T> class LinkOp<T, /*LinkWithPtr=*/false> {
       X->Prev = static_cast<LinkTy>(Prev - Base);
   }
 
-  // TODO: `LinkTy` should be the same as decltype(T::EndOfListVal).
   LinkTy getEndOfListVal() const { return T::EndOfListVal; }
 
 protected:

@ChiaHungDuan
Copy link
Contributor Author

This has been approved in #118888

@ChiaHungDuan ChiaHungDuan merged commit aac000a into llvm:main Dec 10, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants