From c0ca355e55c10f6f8911430d595d638c9162d64e Mon Sep 17 00:00:00 2001 From: Silvan Sievers Date: Mon, 8 Jan 2024 12:09:16 +0100 Subject: [PATCH 1/2] [issue1115] Fix a bug in a state registry. When copying a state, the previous code would use a dangling pointer to access state values of the copied state. This issue fixes this bug by looking up the the correct state values. --- src/search/state_registry.cc | 24 ++++++++++++++++++++++-- src/search/state_registry.h | 7 +++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index e7d882cb5a..22a874856b 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -41,6 +41,12 @@ State StateRegistry::lookup_state(StateID id) const { return task_proxy.create_state(*this, id, buffer); } +State StateRegistry::lookup_state( + StateID id, vector &&state_values) const { + const PackedStateBin *buffer = state_data_pool[id.value]; + return task_proxy.create_state(*this, id, buffer, move(state_values)); +} + const State &StateRegistry::get_initial_state() { if (!cached_initial_state) { int num_bins = get_bins_per_state(); @@ -64,6 +70,12 @@ const State &StateRegistry::get_initial_state() { // operating on state buffers (PackedStateBin *). State StateRegistry::get_successor_state(const State &predecessor, const OperatorProxy &op) { assert(!op.is_axiom()); + /* + TODO: ideally, we would not modify state_data_pool here and in + insert_id_or_pop_state, but only at one place, to avoid errors like + buffer becoming a dangling pointer. This used to be a bug before being + fixed in https://issues.fast-downward.org/issue1115. + */ state_data_pool.push_back(predecessor.get_buffer()); PackedStateBin *buffer = state_data_pool[state_data_pool.size() - 1]; /* Experiments for issue348 showed that for tasks with axioms it's faster @@ -81,8 +93,12 @@ State StateRegistry::get_successor_state(const State &predecessor, const Operato for (size_t i = 0; i < new_values.size(); ++i) { state_packer.set(buffer, i, new_values[i]); } + /* + NOTE: insert_id_or_pop_state possibly invalidates buffer, hence + we use lookup_state to retrieve the state using the correct buffer. + */ StateID id = insert_id_or_pop_state(); - return task_proxy.create_state(*this, id, buffer, move(new_values)); + return lookup_state(id, move(new_values)); } else { for (EffectProxy effect : op.get_effects()) { if (does_fire(effect, predecessor)) { @@ -90,8 +106,12 @@ State StateRegistry::get_successor_state(const State &predecessor, const Operato state_packer.set(buffer, effect_pair.var, effect_pair.value); } } + /* + NOTE: insert_id_or_pop_state possibly invalidates buffer, hence + we use lookup_state to retrieve the state using the correct buffer. + */ StateID id = insert_id_or_pop_state(); - return task_proxy.create_state(*this, id, buffer); + return lookup_state(id); } } diff --git a/src/search/state_registry.h b/src/search/state_registry.h index bb35af642b..0604b0fff5 100644 --- a/src/search/state_registry.h +++ b/src/search/state_registry.h @@ -190,6 +190,13 @@ class StateRegistry : public subscriber::SubscriberService { */ State lookup_state(StateID id) const; + /* + Like lookup_state above, but creates a state with unpacked data, + moved in via state_values. It is the caller's responsibility that + the unpacked data matches the state's data. + */ + State lookup_state(StateID id, std::vector &&state_values) const; + /* Returns a reference to the initial state and registers it if this was not done before. The result is cached internally so subsequent calls are cheap. From bd4deab3c06d6d5683733a66dd1b5967beafe3f2 Mon Sep 17 00:00:00 2001 From: ClemensBuechner Date: Tue, 9 Jan 2024 10:19:57 +0100 Subject: [PATCH 2/2] [trivial] Work around failing Github actions. There is a compatibility-issue of packages installed in the runner-image of Ubuntu 22.04 used in our Github actions. This causes many of our automatic tests to fail. We work around this issue by downgrading the responsible package versions. The change should be reverted once the issue is resolved. --- .github/workflows/style.yml | 7 +++++++ .github/workflows/ubuntu.yml | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/.github/workflows/style.yml b/.github/workflows/style.yml index c5efb21750..5fc5f6df83 100644 --- a/.github/workflows/style.yml +++ b/.github/workflows/style.yml @@ -25,6 +25,13 @@ jobs: pip3 install tox sudo apt-get -y install clang-tidy-12 + # TODO: Remove once issue with Ubuntu 22.04 and clang++-14 is resolved. + - name: Work around https://github.com/actions/runner-images/issues/8659 + run: | + sudo rm -f /etc/apt/sources.list.d/ubuntu-toolchain-r-ubuntu-test-jammy.list + sudo apt-get update + sudo apt-get install -y --allow-downgrades libc6=2.35-* libc6-dev=2.35-* libstdc++6=12.3.0-* libgcc-s1=12.3.0-* + - name: Install uncrustify run: | # Set up uncrustify. diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index cbf48048f5..29c09220d2 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -57,6 +57,14 @@ jobs: run: | sudo apt-get -y install ${{ matrix.version.cxx }} + # TODO: Remove once issue with Ubuntu 22.04 and clang++-14 is resolved. + - name: Work around https://github.com/actions/runner-images/issues/8659 + if: matrix.version.cxx == 'clang++-14' + run: | + sudo rm -f /etc/apt/sources.list.d/ubuntu-toolchain-r-ubuntu-test-jammy.list + sudo apt-get update + sudo apt-get install -y --allow-downgrades libc6=2.35-* libc6-dev=2.35-* libstdc++6=12.3.0-* libgcc-s1=12.3.0-* + # Only install CPLEX if its URL/secret is set. - name: Install CPLEX if: ${{ env.CPLEX_URL != 0 }}