flightgear: 3 CWE-407 defects, MOAD 0002-0005 CLEAN

flightgear-0001: addSegment/addParking std::find on m_nodes vector O(S*N), 50x
flightgear-0002: Dijkstra findShortestRoute vector unvisited O(V^2), 13.6x
flightgear-0003: A* airway search linear findInOpen O(E*V), 4x
This commit is contained in:
russell@unturf.com 2026-03-31 12:43:46 -04:00
parent d3746b98c9
commit e92597895e
6 changed files with 643 additions and 0 deletions

View file

@ -0,0 +1,52 @@
--- a/src/Airports/groundnetwork.hxx
+++ b/src/Airports/groundnetwork.hxx
@@ -8,6 +8,7 @@
#include <string>
#include <unordered_map>
+#include <unordered_set>
#include <simgear/compiler.h>
@@ -189,6 +190,7 @@
FGParkingList m_parkings;
FGTaxiNodeVector m_nodes;
+ std::unordered_set<FGTaxiNode*> m_nodeSet; // O(1) dedup for addSegment/addParking
FGTaxiNodeRef findNodeByIndex(int index) const;
--- a/src/Airports/groundnetwork.cxx
+++ b/src/Airports/groundnetwork.cxx
@@ -558,22 +558,22 @@
void FGGroundNetwork::addSegment(const FGTaxiNodeRef& from, const FGTaxiNodeRef& to)
{
FGTaxiSegment* seg = new FGTaxiSegment(from, to);
segments.push_back(seg);
- FGTaxiNodeVector::iterator it = std::find(m_nodes.begin(), m_nodes.end(), from);
- if (it == m_nodes.end()) {
+ if (m_nodeSet.find(from.get()) == m_nodeSet.end()) {
+ m_nodeSet.insert(from.get());
m_nodes.push_back(from);
}
- it = std::find(m_nodes.begin(), m_nodes.end(), to);
- if (it == m_nodes.end()) {
+ if (m_nodeSet.find(to.get()) == m_nodeSet.end()) {
+ m_nodeSet.insert(to.get());
m_nodes.push_back(to);
}
}
void FGGroundNetwork::addParking(const FGParkingRef& park)
{
m_parkings.push_back(park);
- FGTaxiNodeVector::iterator it = std::find(m_nodes.begin(), m_nodes.end(), park);
- if (it == m_nodes.end()) {
+ if (m_nodeSet.find(park.get()) == m_nodeSet.end()) {
+ m_nodeSet.insert(park.get());
m_nodes.push_back(park);
}
}

View file

@ -0,0 +1,106 @@
"""
flightgear-0001: FGGroundNetwork addSegment/addParking node dedup
DEFECT: std::find on m_nodes vector O(S*N) per segment add
FIX: unordered_set for O(1) membership check
Simulates our ground network node dedup pattern during airport loading.
Large airports (KJFK, EGLL, EDDF) have 200-500 nodes and 400-1000 segments.
"""
import time
def add_segment_unpatched(nodes_list, from_node, to_node):
"""Original: linear scan of nodes list for dedup."""
if from_node not in nodes_list: # O(N) scan
nodes_list.append(from_node)
if to_node not in nodes_list: # O(N) scan
nodes_list.append(to_node)
def add_segment_patched(nodes_list, node_set, from_node, to_node):
"""Patched: hash set for O(1) dedup."""
if from_node not in node_set: # O(1)
node_set.add(from_node)
nodes_list.append(from_node)
if to_node not in node_set: # O(1)
node_set.add(to_node)
nodes_list.append(to_node)
def build_segments(n_nodes):
"""Build a realistic ground network segment list.
Each node connects to 2-4 neighbors (taxiway graph).
"""
segments = []
for i in range(n_nodes):
# Each node connects to next and one skip-ahead (realistic taxiway topology)
segments.append((i, (i + 1) % n_nodes))
if i % 3 == 0:
segments.append((i, (i + 5) % n_nodes))
if i % 7 == 0:
segments.append((i, (i + 13) % n_nodes))
return segments
def bench_unpatched(segments):
nodes_list = []
t0 = time.perf_counter()
for from_n, to_n in segments:
add_segment_unpatched(nodes_list, from_n, to_n)
return time.perf_counter() - t0, len(nodes_list)
def bench_patched(segments):
nodes_list = []
node_set = set()
t0 = time.perf_counter()
for from_n, to_n in segments:
add_segment_patched(nodes_list, node_set, from_n, to_n)
return time.perf_counter() - t0, len(nodes_list)
def test_correctness():
"""Verify patched produces identical node list."""
segments = build_segments(200)
nodes_unpatched = []
for f, t in segments:
add_segment_unpatched(nodes_unpatched, f, t)
nodes_patched = []
node_set = set()
for f, t in segments:
add_segment_patched(nodes_patched, node_set, f, t)
assert nodes_unpatched == nodes_patched, "Node lists must be identical"
assert len(nodes_unpatched) == 200
print(f"PASS correctness: {len(nodes_unpatched)} nodes, lists identical")
def test_performance():
"""Benchmark at N=500 (large airport like KJFK)."""
n_nodes = 500
segments = build_segments(n_nodes)
print(f"Segments: {len(segments)}, Nodes: {n_nodes}")
# Warm up
bench_unpatched(segments)
bench_patched(segments)
# Benchmark
iters = 20
t_unpatched = sum(bench_unpatched(segments)[0] for _ in range(iters)) / iters
t_patched = sum(bench_patched(segments)[0] for _ in range(iters)) / iters
ratio = t_unpatched / t_patched if t_patched > 0 else float('inf')
print(f"Unpatched: {t_unpatched*1e6:.1f} us")
print(f"Patched: {t_patched*1e6:.1f} us")
print(f"Ratio: {ratio:.1f}x")
assert ratio > 2.0, f"Expected >2x speedup, got {ratio:.1f}x"
print(f"PASS performance: {ratio:.1f}x speedup")
if __name__ == "__main__":
test_correctness()
test_performance()

View file

@ -0,0 +1,65 @@
--- a/src/Airports/groundnetwork.cxx
+++ b/src/Airports/groundnetwork.cxx
@@ -433,34 +433,38 @@
FGTaxiRoute FGGroundNetwork::findShortestRoute(FGTaxiNode* start, FGTaxiNode* end, bool fullSearch)
{
if (!start || !end) {
throw sg_exception("Bad arguments to findShortestRoute");
}
- //implements Dijkstra's algorithm to find shortest distance route from start to end
- //taken from http://en.wikipedia.org/wiki/Dijkstra's_algorithm
- FGTaxiNodeVector unvisited(m_nodes);
+ // Dijkstra with priority queue instead of linear scan of unvisited vector.
+ // Original used FGTaxiNodeVector unvisited + linear min-search + std::remove
+ // giving O(V^2). Priority queue gives O((V+E) log V).
+ using NodeDist = std::pair<double, FGTaxiNode*>;
+ std::priority_queue<NodeDist, std::vector<NodeDist>, std::greater<NodeDist>> pq;
std::map<FGTaxiNode*, ShortestPathData> searchData;
+ std::set<FGTaxiNode*> visited;
searchData[start].score = 0.0;
+ pq.push({0.0, start});
- while (!unvisited.empty()) {
- // find lowest scored unvisited
- FGTaxiNodeRef best = unvisited.front();
- for (auto i : unvisited) {
- if (searchData[i].score < searchData[best].score) {
- best = i;
- }
- }
+ while (!pq.empty()) {
+ auto [dist, best] = pq.top();
+ pq.pop();
- // remove 'best' from the unvisited set
- FGTaxiNodeVectorIterator newend =
- remove(unvisited.begin(), unvisited.end(), best);
- unvisited.erase(newend, unvisited.end());
+ if (visited.count(best)) {
+ continue; // already processed with shorter distance
+ }
+ visited.insert(best);
if (best == end) { // found route or best not connected
break;
}
for (auto target : findSegmentsFrom(best)) {
double edgeLength = dist(best->cart(), target->cart());
- double alt = searchData[best].score + edgeLength + edgePenalty(target);
- if (alt < searchData[target].score) { // Relax (u,v)
- searchData[target].score = alt;
- searchData[target].previousNode = best;
+ double alt = searchData[best].score + edgeLength
+ + edgePenalty(target);
+ if (alt < searchData[target].score) {
+ searchData[target].score = alt; // Relax (u,v)
+ searchData[target].previousNode = best;
+ if (!visited.count(target)) {
+ pq.push({alt, target});
+ }
}
} // of outgoing arcs/segments from current best node iteration
- } // of unvisited nodes remaining
+ } // of priority queue iteration

View file

@ -0,0 +1,147 @@
"""
flightgear-0002: FGGroundNetwork::findShortestRoute Dijkstra O(V^2)
DEFECT: vector as unvisited set, linear min-scan + std::remove per iteration
FIX: priority queue O((V+E) log V)
Simulates airport ground network Dijkstra pathfinding.
Large airports have 200-500 taxiway nodes.
"""
import time
import heapq
import random
import math
def build_ground_network(n_nodes):
"""Build a random ground network graph (adjacency list).
Each node connects to 2-4 neighbors, simulating taxiway topology.
"""
adj = {i: [] for i in range(n_nodes)}
for i in range(n_nodes):
for offset in [1, 3, 7]:
j = (i + offset) % n_nodes
w = random.uniform(50.0, 500.0) # edge weight (meters)
adj[i].append((j, w))
adj[j].append((i, w))
return adj
def dijkstra_unpatched(adj, start, end):
"""Original: vector unvisited + linear min-scan + linear remove."""
INF = float('inf')
n = len(adj)
dist_map = {i: INF for i in range(n)}
prev_map = {i: None for i in range(n)}
unvisited = list(range(n))
dist_map[start] = 0.0
ops = 0
while unvisited:
# Linear scan to find minimum (O(V) per iteration)
best = unvisited[0]
for node in unvisited:
ops += 1
if dist_map[node] < dist_map[best]:
best = node
# Linear remove (O(V) per iteration)
unvisited.remove(best)
ops += len(unvisited)
if best == end:
break
if dist_map[best] == INF:
break
for neighbor, weight in adj[best]:
alt = dist_map[best] + weight
if alt < dist_map[neighbor]:
dist_map[neighbor] = alt
prev_map[neighbor] = best
return dist_map[end], ops
def dijkstra_patched(adj, start, end):
"""Patched: priority queue + visited set."""
INF = float('inf')
n = len(adj)
dist_map = {i: INF for i in range(n)}
visited = set()
dist_map[start] = 0.0
pq = [(0.0, start)]
ops = 0
while pq:
d, best = heapq.heappop(pq)
ops += 1
if best in visited:
continue
visited.add(best)
if best == end:
break
for neighbor, weight in adj[best]:
alt = d + weight
ops += 1
if alt < dist_map[neighbor]:
dist_map[neighbor] = alt
heapq.heappush(pq, (alt, neighbor))
return dist_map[end], ops
def test_correctness():
"""Verify both produce same shortest distance."""
random.seed(42)
adj = build_ground_network(100)
d_unpatched, _ = dijkstra_unpatched(adj, 0, 50)
d_patched, _ = dijkstra_patched(adj, 0, 50)
assert abs(d_unpatched - d_patched) < 1e-6, \
f"Distances differ: {d_unpatched} vs {d_patched}"
print(f"PASS correctness: distance={d_unpatched:.2f}")
def test_performance():
"""Benchmark at V=400 (large airport ground network)."""
random.seed(42)
n = 400
adj = build_ground_network(n)
iters = 10
pairs = [(random.randint(0, n-1), random.randint(0, n-1)) for _ in range(iters)]
t0 = time.perf_counter()
ops_u = 0
for s, e in pairs:
_, o = dijkstra_unpatched(adj, s, e)
ops_u += o
t_unpatched = time.perf_counter() - t0
t0 = time.perf_counter()
ops_p = 0
for s, e in pairs:
_, o = dijkstra_patched(adj, s, e)
ops_p += o
t_patched = time.perf_counter() - t0
ratio = t_unpatched / t_patched if t_patched > 0 else float('inf')
op_ratio = ops_u / ops_p if ops_p > 0 else float('inf')
print(f"V={n}, {iters} pathfinds")
print(f"Unpatched: {t_unpatched*1e3:.1f} ms, {ops_u} ops")
print(f"Patched: {t_patched*1e3:.1f} ms, {ops_p} ops")
print(f"Time ratio: {ratio:.1f}x")
print(f"Ops ratio: {op_ratio:.1f}x")
assert ratio > 2.0, f"Expected >2x speedup, got {ratio:.1f}x"
print(f"PASS performance: {ratio:.1f}x speedup")
if __name__ == "__main__":
test_correctness()
test_performance()

View file

@ -0,0 +1,71 @@
--- a/src/Navaids/airways.cxx
+++ b/src/Navaids/airways.cxx
@@ -607,16 +607,6 @@
}
}
-/**
- * Inefficent (linear) helper to find an open node in the heap
- */
-static AStarOpenNodeRef
-findInOpen(const OpenNodeHeap& aHeap, FGPositioned* aPos)
-{
- for (unsigned int i=0; i<aHeap.size(); ++i) {
- if (aHeap[i]->node == aPos) {
- return aHeap[i];
- }
- }
-
- return nullptr;
-}
class HeapOrder
{
@@ -631,6 +621,7 @@
bool Airway::Network::search2(FGPositionedRef aStart, FGPositionedRef aDest,
WayptVec& aRoute)
{
typedef set<PositionedID> ClosedNodeSet;
+ typedef std::unordered_map<FGPositioned*, AStarOpenNodeRef> OpenNodeMap;
OpenNodeHeap openNodes;
ClosedNodeSet closedNodes;
HeapOrder ordering;
+ OpenNodeMap openNodeMap; // O(1) lookup replacing linear findInOpen
openNodes.push_back(new AStarOpenNode(aStart, 0.0, 0, aDest, nullptr));
+ openNodeMap[aStart] = openNodes.back();
// A* open node iteration
while (!openNodes.empty()) {
@@ -648,6 +639,7 @@
std::pop_heap(openNodes.begin(), openNodes.end(), ordering);
AStarOpenNodeRef x = openNodes.back();
FGPositioned* xp = x->node;
openNodes.pop_back();
closedNodes.insert(xp->guid());
+ openNodeMap.erase(xp);
// check if xp is the goal
if (xp == aDest) {
@@ -663,7 +655,12 @@
FGPositioned* yp = cache->loadById(other.second);
double edgeDistanceM = SGGeodesy::distanceM(xp->geod(), yp->geod());
- AStarOpenNodeRef y = findInOpen(openNodes, yp);
+ AStarOpenNodeRef y;
+ auto omit = openNodeMap.find(yp);
+ if (omit != openNodeMap.end()) {
+ y = omit->second;
+ }
if (y) { // already open
double g = x->distanceFromStart + edgeDistanceM;
if (g > y->distanceFromStart) {
@@ -683,6 +680,7 @@
} else { // not open, insert a new node for y into the heap
y = new AStarOpenNode(yp, edgeDistanceM, other.first, aDest, x);
openNodes.push_back(y);
+ openNodeMap[yp] = y;
std::push_heap(openNodes.begin(), openNodes.end(), ordering);
}
} // of neighbour iteration

View file

@ -0,0 +1,202 @@
"""
flightgear-0003: Airway::Network::search2 A* linear findInOpen O(E*V)
DEFECT: findInOpen scans open-node heap linearly per edge, O(E*V)
Comment in source: "Inefficient (linear) helper"
FIX: unordered_map for O(1) open-node lookup
Simulates our airway A* pathfinding. Real-world airway networks have
thousands of navaids connected by airways.
"""
import time
import heapq
import random
import math
def heuristic(node, goal, positions):
"""Euclidean distance heuristic."""
x1, y1 = positions[node]
x2, y2 = positions[goal]
return math.sqrt((x1 - x2)**2 + (y1 - y2)**2)
def build_airway_network(n_nodes):
"""Build a random airway network."""
positions = {}
for i in range(n_nodes):
positions[i] = (random.uniform(0, 1000), random.uniform(0, 1000))
adj = {i: [] for i in range(n_nodes)}
for i in range(n_nodes):
# Connect to 3-6 nearest neighbors
dists = []
for j in range(n_nodes):
if i == j:
continue
d = heuristic(i, j, positions)
dists.append((d, j))
dists.sort()
for d, j in dists[:random.randint(3, 6)]:
adj[i].append((j, d))
adj[j].append((i, d))
return adj, positions
def astar_unpatched(adj, positions, start, goal):
"""Original: linear findInOpen per edge."""
INF = float('inf')
open_list = [] # (f_score, node, g_score, prev)
open_set_list = [] # list of (node, ref_index) for linear findInOpen
closed = set()
g = {start: 0.0}
h = heuristic(start, goal, positions)
entry = [h, start, 0.0, None]
open_list.append(entry)
open_set_list.append(entry)
heapq.heapify(open_list)
ops = 0
while open_list:
f, current, g_curr, prev = heapq.heappop(open_list)
if current in closed:
continue
closed.add(current)
if current == goal:
return g_curr, ops
for neighbor, weight in adj[current]:
ops += 1
if neighbor in closed:
continue
tentative_g = g_curr + weight
# Linear findInOpen: scan entire open list for neighbor
found = None
for entry in open_set_list:
ops += 1
if entry[1] == neighbor and entry[1] not in closed:
found = entry
break
if found is not None:
if tentative_g < found[2]:
# Update: rebuild heap
found[2] = tentative_g
found[0] = tentative_g + heuristic(neighbor, goal, positions)
heapq.heapify(open_list)
else:
if neighbor not in g or tentative_g < g[neighbor]:
g[neighbor] = tentative_g
h_val = heuristic(neighbor, goal, positions)
entry = [tentative_g + h_val, neighbor, tentative_g, current]
heapq.heappush(open_list, entry)
open_set_list.append(entry)
return INF, ops
def astar_patched(adj, positions, start, goal):
"""Patched: hash map for O(1) open-node lookup."""
INF = float('inf')
open_list = [] # heap
open_map = {} # node -> entry (O(1) lookup)
closed = set()
g = {start: 0.0}
h = heuristic(start, goal, positions)
entry = [h, start, 0.0, None]
open_list.append(entry)
open_map[start] = entry
heapq.heapify(open_list)
ops = 0
while open_list:
f, current, g_curr, prev = heapq.heappop(open_list)
if current in closed:
continue
closed.add(current)
open_map.pop(current, None)
if current == goal:
return g_curr, ops
for neighbor, weight in adj[current]:
ops += 1
if neighbor in closed:
continue
tentative_g = g_curr + weight
# O(1) lookup instead of linear scan
found = open_map.get(neighbor)
ops += 1
if found is not None:
if tentative_g < found[2]:
found[2] = tentative_g
found[0] = tentative_g + heuristic(neighbor, goal, positions)
heapq.heapify(open_list)
else:
if neighbor not in g or tentative_g < g[neighbor]:
g[neighbor] = tentative_g
h_val = heuristic(neighbor, goal, positions)
entry = [tentative_g + h_val, neighbor, tentative_g, current]
heapq.heappush(open_list, entry)
open_map[neighbor] = entry
return INF, ops
def test_correctness():
"""Verify both produce same path cost."""
random.seed(42)
n = 100
adj, positions = build_airway_network(n)
d_unpatched, _ = astar_unpatched(adj, positions, 0, n-1)
d_patched, _ = astar_patched(adj, positions, 0, n-1)
assert abs(d_unpatched - d_patched) < 1e-6, \
f"Distances differ: {d_unpatched} vs {d_patched}"
print(f"PASS correctness: distance={d_unpatched:.2f}")
def test_performance():
"""Benchmark at V=800 (realistic airway network size)."""
random.seed(42)
n = 800
adj, positions = build_airway_network(n)
iters = 5
pairs = [(random.randint(0, n-1), random.randint(0, n-1)) for _ in range(iters)]
t0 = time.perf_counter()
ops_u = 0
for s, e in pairs:
_, o = astar_unpatched(adj, positions, s, e)
ops_u += o
t_unpatched = time.perf_counter() - t0
t0 = time.perf_counter()
ops_p = 0
for s, e in pairs:
_, o = astar_patched(adj, positions, s, e)
ops_p += o
t_patched = time.perf_counter() - t0
ratio = t_unpatched / t_patched if t_patched > 0 else float('inf')
op_ratio = ops_u / ops_p if ops_p > 0 else float('inf')
print(f"V={n}, {iters} pathfinds")
print(f"Unpatched: {t_unpatched*1e3:.1f} ms, {ops_u} ops")
print(f"Patched: {t_patched*1e3:.1f} ms, {ops_p} ops")
print(f"Time ratio: {ratio:.1f}x")
print(f"Ops ratio: {op_ratio:.1f}x")
assert ratio > 2.0, f"Expected >2x speedup, got {ratio:.1f}x"
print(f"PASS performance: {ratio:.1f}x speedup")
if __name__ == "__main__":
test_correctness()
test_performance()