Is it expected such test doesn't work with sanitizer?

[MBkkt]

Describe the issue

reserve to smaller than size capacity makes unsigned overflow in reset_reserved_growth

  void reset_reserved_growth(size_t reservation, size_t size) {
    reserved_growth_ = reservation - size;
  }

TEST(Table, ReservedGrowthUpdatesWhenTableDoesntGrow2) {
  IntTable t;
  for (int i = 0; i < 8; ++i) t.insert(i);
  // Want to insert twice without invalidating iterators so reserve.
  const size_t cap = t.capacity();
  t.reserve(3);
  // We want to be testing the case in which the reserve doesn't grow the table.
  ASSERT_EQ(cap, t.capacity());
  auto it = t.find(0);
  t.insert(100);
  t.insert(200);
  t.insert(300);
  // `it` should have been invalidated.
  EXPECT_EQ(*it, 0); // should be some expect_death
}

Is it expected?

Steps to reproduce the problem

Run test

What version of Abseil are you using?

master

What operating system and version are you using?

Linux 6.2.6-arch1-1

What compiler and version are you using?

clang version 15.0.7 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-pc-linux-gnu/12.2.1 Found candidate GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1 Selected GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Selected multilib: .;@m64

What build system are you using?

cmake version 3.26.1

CMake suite maintained and supported by Kitware (kitware.com/cmake).

Additional context

I think fix should be like make reset_reserved_growth call inside raw_hash_set::resize https://github.com/abseil/abseil-cpp/pull/1423

[derekmauro]

We run this test under sanitizers and it passes. Can you share more information about the problem that you are seeing? Like the full build command and error messages?

[MBkkt]

@derekmauro

I probably wasn't clear enough. Something like this explain better.

TEST(Table, ReservedTemp2) {
  if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled.";
  IntTable t;
  for (int i = 0; i < 8; ++i) t.insert(i);
  const size_t cap = t.capacity();
  t.reserve(t.size() + 1);
  t.reserve(2); // without this line everything works as expected
  ASSERT_EQ(cap, t.capacity());
  auto it = t.find(0);
  t.insert(100);
  t.insert(200);
  // `it` should have been invalidated?
  // because no one call reserve(10) only reserve(9) and reserve(2)
  EXPECT_DEATH_IF_SUPPORTED(*it, kInvalidIteratorDeathMessage);
}

TEST(Table, ReservedTemp3) {
  if (!SwisstableGenerationsEnabled()) GTEST_SKIP() << "Generations disabled.";
  IntTable t;
  for (int i = 0; i < 8; ++i) t.insert(i);
  const size_t cap = t.capacity();
  t.reserve(t.size() + 1);
  t.reserve(t.size() - 1); // without this line everything works as expected
  ASSERT_EQ(cap, t.capacity());
  auto it = t.find(0);
  t.insert(100);
  // `it` shouldn't have been invalidated?
  // because was called reserve(9)
  EXPECT_TRUE(*it == 0);
}

[MBkkt]

@derekmauro Hello, is it reproduce issue for you? Or I need to provide something else?

It's different test. It's not in your CI now. It's about two cases:

1. Unsigned overflow in the checker code.
2. Reserve to the large size, then reserve to the smaller size

[derekmauro]

I'm going to quote the bug report form:

It is important that we are able to reproduce the problem that you are experiencing. Please provide all code and relevant steps to reproduce the problem, including your BUILD/CMakeLists.txt file and build commands. Links to a GitHub branch or godbolt.org that demonstrate the problem are also helpful.

Please provide the requested information. "Run test" is not helpful.

[MBkkt]

I filled form with all fields, like compiler and other stuff, and for this issue, it's absolutely doesn't matter. It can be reproduced with any clang. Because it's logical issue in your sanitizer related code.

Is it too difficult to copy 2 examples of code and run them with sanitizers? In such case I think this form just useless