Starlight-JS / starlight

JS engine in Rust
https://teletype.in/@starlight-js
Mozilla Public License 2.0
511 stars 9 forks source link

fix: gc space bit map index check #61

Closed jameslahm closed 3 years ago

jameslahm commented 3 years ago

It's not relevant to finally return. So, I open a new pr here.

playXE commented 3 years ago

Hmm Android ART from where I borrowed this code does not seem to have such a check, maybe there's something going wrong elsewhere? https://github.com/LineageOS/android_art/blob/lineage-18.1/runtime/gc/accounting/space_bitmap-inl.h

template<size_t kAlignment>
inline bool SpaceBitmap<kAlignment>::Test(const mirror::Object* obj) const {
  uintptr_t addr = reinterpret_cast<uintptr_t>(obj);
  DCHECK(HasAddress(obj)) << obj;
  DCHECK(bitmap_begin_ != nullptr);
  DCHECK_GE(addr, heap_begin_);
  const uintptr_t offset = addr - heap_begin_;
  size_t index = OffsetToIndex(offset);
  return (bitmap_begin_[index].load(std::memory_order_relaxed) & OffsetToMask(offset)) != 0;
}
jameslahm commented 3 years ago

Actually, I even can't run hello-world.js with segmentation fault. My rustc versions is rustc 1.55.0-nightly (e8cb1a4a5 2021-06-27) and operating system is mac osx BigSur 11.4. And I find the cause of segment fault is that *self.bitmap_begin.add(index) will read illegal address. And I think it's reasonable to check the index not exceeding the max index. Because it's possible to read the index out of bounds to cause segment fault.

playXE commented 3 years ago

Hmm okay. I'm going to merge this but will also take a look what can cause such problem