Skip to content

fix: undefined behavior when transforming non-existing edges in a subgraph#525

Open
Becheler wants to merge 5 commits into
boostorg:developfrom
Becheler:fix/subgraph-124-ci
Open

fix: undefined behavior when transforming non-existing edges in a subgraph#525
Becheler wants to merge 5 commits into
boostorg:developfrom
Becheler:fix/subgraph-124-ci

Conversation

@Becheler

@Becheler Becheler commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes bug #124

Before submitting

  • This PR targets the develop branch.
  • I searched for an existing PR or issue covering the same change.
  • My contribution is licensed under the Boost Software License 1.0.

Type of change

  • Bug fix
  • New feature or API addition
  • Refactor (no behavior change)
  • Documentation
  • Build, CI, or tooling
  • Other (specify below)

Does this PR introduce a breaking change?

  • Yes (describe migration impact below)
  • No

What this PR does

  • Commit 1: document safe utilities (find_vertex and find_edge) and lock in invariants without touching header.
  • Commit 2: Add the sad-path test exercising global_to_local on an edge not in the subgraph. Red on CI (memory corruption / garbage instead of a benign default).
  • Commit 3: Fix subgraph.hpp (global_to_local delegates to find_edge + assert) turning CI error into a loud meaningfull assertion failure that mirrors find_vertex
  • Commit 4 adapts the test to check find_edge first, CI green

Motivation

In #124 it was shown that subgraph::global_to_local(edge_descriptor) is only defined when the input edge is actually in the subgraph. For edges not in the subgraph, the behavior is undefined (it dereferences end()). But its signature, name, and documentation all present it as being well-defined irrespective of the input.

Testing

Added a new subgraph test file. There are currently 5 subgraph_XXX.cpp files that should be tightened in a single test file, but it's outside the scope of this first PR.

Note: Although not a breaking change, someone will notice the change if in a debug build their code that previously invoked the UB and happened to get away with it (garbage returned but never actually used): this will now abort at the assertion instead of running away silently.

Checklist

  • Existing tests pass (b2 in the test/ directory).
  • New behavior is covered by a test, or this is a docs / build / refactor change.
  • Documentation was updated if user-facing behavior changed.
  • No new compiler warnings on the platforms I built against.

@Becheler Becheler requested a review from jeremy-murphy as a code owner July 14, 2026 12:20
@Becheler Becheler self-assigned this Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

Boost dependency footprint vs develop (auto-generated).
PR run 29342271636 vs develop run 29329400139 (7db1f2e420).

Header-inclusion weights (graph files pulling each direct dependency in):

No header-inclusion-weight changes.

Transitive Boost modules: 70 → 70 (0)

@cppalliance-bot

cppalliance-bot commented Jul 14, 2026

Copy link
Copy Markdown

An automated preview of the documentation is available at https://525.graph.prtest3.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-07-14 14:50:22 UTC

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

Compiler-warning counts vs develop (auto-generated).
PR run 29342271671 vs develop run 29329400156 (7db1f2e420).

Job Baseline After Delta
macos (clang, 14) 4 4 0
macos (clang, 17) 4 4 0
macos (clang, 20) 4 4 0
ubuntu (clang-19, 14) 4 4 0
ubuntu (clang-19, 17) 4 4 0
ubuntu (clang-19, 20) 4 4 0
ubuntu (clang-19, 23) 4 4 0
ubuntu (gcc-14, 14) 10 10 0
ubuntu (gcc-14, 17) 10 10 0
ubuntu (gcc-14, 20) 10 10 0
ubuntu (gcc-14, 23) 10 10 0
windows_msvc_14_3 (msvc-14.3) 971 971 0

@Becheler

Copy link
Copy Markdown
Collaborator Author

On Clang-19, C++14, UB is left to user:

subgraph_global_local.cpp(123): test 'sg.global_to_local(e01) == edge_t()' failed in function 'void test_absent_edge_returns_default() [Directedness = boost::directedS]'
1 error detected.

@Becheler

Copy link
Copy Markdown
Collaborator Author

On Clang-19, C++14, UB fails loudly and helpfully:

subgraph_global_local: ../../../boost/graph/subgraph.hpp:237: edge_descriptor boost::subgraph<boost::adjacency_list<boost::vecS, boost::vecS, boost::directedS, boost::property<boost::vertex_color_t, int>, boost::property<boost::edge_index_t, unsigned long, boost::property<boost::edge_weight_t, int>>>>::global_to_local(edge_descriptor) const [Graph = boost::adjacency_list<boost::vecS, boost::vecS, boost::directedS, boost::property<boost::vertex_color_t, int>, boost::property<boost::edge_index_t, unsigned long, boost::property<boost::edge_weight_t, int>>>]: 
Assertion `(in_subgraph)&&("global_to_local: edge is not in this subgraph. " 
"Use find_edge() to check membership first.")' failed.
Aborted (core dumped)

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