Skip to content
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

Replace pipe with internal_pipe in compiler-rt sanitizer common #96786

Closed
Lancern opened this issue Jun 26, 2024 · 1 comment
Closed

Replace pipe with internal_pipe in compiler-rt sanitizer common #96786

Lancern opened this issue Jun 26, 2024 · 1 comment
Labels
compiler-rt duplicate Resolved as duplicate

Comments

@Lancern
Copy link
Member

Lancern commented Jun 26, 2024

Here in compiler-rt sanitizer common code:

bool IsAccessibleMemoryRange(uptr beg, uptr size) {
uptr page_size = GetPageSizeCached();
// Checking too large memory ranges is slow.
CHECK_LT(size, page_size * 10);
int sock_pair[2];
if (pipe(sock_pair))
return false;
uptr bytes_written =
internal_write(sock_pair[1], reinterpret_cast<void *>(beg), size);
int write_errno;
bool result;
if (internal_iserror(bytes_written, &write_errno)) {
CHECK_EQ(EFAULT, write_errno);
result = false;
} else {
result = (bytes_written == size);
}
internal_close(sock_pair[0]);
internal_close(sock_pair[1]);
return result;

On line 296, the code calls pipe, which could be intercepted by sanitizers such as TSAN and make them produce false positives. This can be reproduced by the following example, which mixes UBSan and TSan:

// test.cpp
// g++ -std=c++17 -fsanitize=undefined -fsanitize=thread -o test test.cpp

#include <thread>

class Foo {
public:
  void produce(int) {}
  void consume() {}

  void run() {
    w1_ = std::thread{&Foo::produce, this, 0};
    w2_ = std::thread{&Foo::consume, this};
    w1_.join();
    w2_.join();
  }

private:
  std::thread w1_;
  std::thread w2_;
};

int main() {
  Foo f;
  f.run();
  return 0;
}

Should we replace this pipe call with something like internal_pipe? I can draft a patch for this if necessary. Or this is not a considered scenario?

@Lancern
Copy link
Member Author

Lancern commented Jun 27, 2024

This is a duplicate of google/sanitizers#1106.

@EugeneZelenko EugeneZelenko added the duplicate Resolved as duplicate label Jun 27, 2024
@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt duplicate Resolved as duplicate
Projects
None yet
Development

No branches or pull requests

2 participants