Skip to content

Commit cd43e38

Browse files
committed
fix asan issues
1 parent f436f65 commit cd43e38

8 files changed

Lines changed: 110 additions & 6 deletions

File tree

CMakeLists.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,22 @@ target_include_directories(
8080
# link time. No-op on non-Windows.
8181
target_compile_definitions(_duckdb_dependencies INTERFACE DUCKDB_STATIC_BUILD)
8282

83+
# Optional AddressSanitizer instrumentation of the Python binding objects ONLY.
84+
# Every binding object library consumes _duckdb_dependencies (for headers) and
85+
# _duckdb links it, so adding the flag here instruments the bindings and links
86+
# the ASAN runtime, while the engine target (duckdb_target, which does NOT
87+
# consume this) stays uninstrumented and keeps hitting the sccache cache. ASAN's
88+
# allocator is process-global, so heap errors involving the instrumented binding
89+
# code are still caught. OFF by default; enable with -DDUCKDB_PY_ASAN=ON.
90+
option(DUCKDB_PY_ASAN
91+
"Instrument the Python binding objects with AddressSanitizer" OFF)
92+
if(DUCKDB_PY_ASAN)
93+
target_compile_options(
94+
_duckdb_dependencies INTERFACE -fsanitize=address -fno-omit-frame-pointer
95+
-g)
96+
target_link_options(_duckdb_dependencies INTERFACE -fsanitize=address)
97+
endif()
98+
8399
# ────────────────────────────────────────────
84100
# Descend into the real DuckDB‑Python sources
85101
# ────────────────────────────────────────────

src/duckdb_py/include/duckdb_python/pybind11/pybind_wrapper.hpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,25 @@ bool try_cast(const handle &object, T &result) {
108108
return true;
109109
}
110110

111+
// pybind11's std::string caster accepted str (as-is) and bytes (decoded UTF-8) and stringified scalars; nanobind's
112+
// nb::cast<std::string> is stricter and surfaces a raw std::bad_cast for non-str input. This helper restores the
113+
// lenient behavior for the identifier / parameter-key / separator sites that relied on it:
114+
// str -> the string as-is
115+
// bytes -> UTF-8 decoded (so read_csv(sep=b"|") and byte-string identifiers keep working)
116+
// else -> str(obj) (so e.g. an int parameter-dict key stringifies to "1", matching pybind11)
117+
// It never throws std::bad_cast.
118+
inline std::string cast_to_string(handle obj) {
119+
// Use check_ directly: an unqualified isinstance<> here is ambiguous between this namespace's override and
120+
// nanobind's (pulled in by the using-directive above).
121+
if (bytes::check_(obj)) {
122+
return cast<std::string>(obj.attr("decode")("utf-8"));
123+
}
124+
if (str::check_(obj)) {
125+
return cast<std::string>(obj);
126+
}
127+
return cast<std::string>(str(obj));
128+
}
129+
111130
// pybind11 compatibility shim: pybind11's py::register_exception<T>(scope, name[, base]) maps to nanobind's
112131
// nb::exception<T>(scope, name[, base]) (which both creates the Python exception type and registers a C++->Python
113132
// translator). Returns the exception object so callers can set .attr()/.doc().

src/duckdb_py/pyconnection.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ py::list TransformNamedParameters(const case_insensitive_map_t<idx_t> &named_par
573573
}
574574

575575
for (auto item : params) {
576-
const std::string &item_name = py::cast<std::string>(item.first);
576+
const std::string &item_name = py::cast_to_string(item.first);
577577
auto entry = named_param_map.find(item_name);
578578
if (entry == named_param_map.end()) {
579579
throw InvalidInputException(
@@ -1297,9 +1297,9 @@ std::unique_ptr<DuckDBPyRelation> DuckDBPyConnection::ReadCSV(const py::object &
12971297
throw InvalidInputException("read_csv takes either 'delimiter' or 'sep', not both");
12981298
}
12991299
if (has_sep) {
1300-
bind_parameters["delim"] = Value(py::cast<std::string>(sep));
1300+
bind_parameters["delim"] = Value(py::cast_to_string(sep));
13011301
} else if (has_delimiter) {
1302-
bind_parameters["delim"] = Value(py::cast<std::string>(delimiter));
1302+
bind_parameters["delim"] = Value(py::cast_to_string(delimiter));
13031303
}
13041304

13051305
if (!py::none().is(files_to_sniff)) {
@@ -2317,7 +2317,7 @@ identifier_map_t<BoundParameterData> DuckDBPyConnection::TransformPythonParamDic
23172317
for (auto pair : params) {
23182318
auto &key = pair.first;
23192319
auto &value = pair.second;
2320-
args[Identifier(py::cast<std::string>(key))] =
2320+
args[Identifier(py::cast_to_string(key))] =
23212321
BoundParameterData(TransformPythonValue(context, value, LogicalType::UNKNOWN, false));
23222322
}
23232323
return args;

src/duckdb_py/pyconnection/type_creation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static child_list_t<LogicalType> GetChildList(const py::object &container) {
3737
for (auto item : fields) {
3838
auto name_p = item.first;
3939
auto type_p = item.second;
40-
auto name = Identifier(py::cast<std::string>(name_p));
40+
auto name = Identifier(py::cast_to_string(name_p));
4141
std::unique_ptr<DuckDBPyType> pytype;
4242
if (!DuckDBPyType::TryConvert(py::borrow<py::object>(type_p), pytype)) {
4343
string actual_type = py::cast<std::string>(py::str((type_p).type()));

src/duckdb_py/pyexpression.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,10 @@ bool DuckDBPyExpression::TryToExpression(py::handle obj, std::unique_ptr<DuckDBP
352352
} else if (py::isinstance<py::str>(obj)) {
353353
// A str becomes a column reference, mirrors the registered str constructor.
354354
result = ColumnExpression(py::cast<py::args>(py::make_tuple(obj)));
355+
} else if (py::isinstance<py::bytes>(obj)) {
356+
// pybind11 decoded bytes as UTF-8 and (like str) treated them as a column reference; preserve that
357+
// so e.g. rel.project(b"col") references column "col" instead of silently building a BLOB constant.
358+
result = ColumnExpression(py::cast<py::args>(py::make_tuple(obj.attr("decode")("utf-8"))));
355359
} else {
356360
// Anything else, including None, becomes a constant -- mirrors the registered object constructor
357361
// (None -> NULL constant; TransformPythonValue throws on genuinely unsupported types).

src/duckdb_py/typing/pytype.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ static LogicalType FromDictionary(const py::object &obj) {
292292
for (auto item : dict) {
293293
auto &name_p = item.first;
294294
auto type_p = py::borrow<py::object>(item.second);
295-
auto name = Identifier(py::cast<std::string>(name_p));
295+
auto name = Identifier(py::cast_to_string(name_p));
296296
auto type = FromObject(type_p);
297297
children.push_back(std::make_pair(name, std::move(type)));
298298
}

tests/fast/test_expression_implicit_conversion.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,18 @@ def test_binary_operator_str_rhs(rel):
177177
assert result == [(True,)]
178178

179179

180+
def test_binary_operator_bytes_rhs(rel):
181+
"""Bytes on the RHS is decoded as UTF-8 and (like str) becomes a ColumnExpression (column reference)."""
182+
expr = ColumnExpression("i") == b"i"
183+
assert isinstance(expr, duckdb.Expression)
184+
assert rel.select(expr).fetchall() == [(True,)]
185+
186+
187+
def test_project_with_bytes_column_name(rel):
188+
"""rel.select(b'col') references the column (bytes decoded), not a silent BLOB constant (regression guard)."""
189+
assert rel.select(b"i").fetchall() == [(42,)]
190+
191+
180192
# ---------------------------------------------------------------------------
181193
# 3. Reflected operators: <value> + col
182194
# ---------------------------------------------------------------------------

tests/fast/test_string_coercion.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
"""String coercion at identifier / parameter-key / separator sites.
2+
3+
nanobind's nb::cast<std::string> is stricter than pybind11's: it rejects bytes and non-str scalars and surfaces a
4+
raw ``RuntimeError: std::bad_cast`` instead of pybind11's lenient conversion. The ``cast_to_string`` helper restores
5+
the lenient behavior (str as-is, bytes UTF-8 decoded, anything else stringified via str()). These guard the
6+
std::bad_cast regression and confirm the realistic cases still match pybind11.
7+
"""
8+
9+
import platform
10+
11+
import pytest
12+
13+
import duckdb
14+
15+
pytestmark = pytest.mark.skipif(
16+
platform.system() == "Emscripten",
17+
reason="Extensions are not supported on Emscripten",
18+
)
19+
20+
21+
def test_execute_int_param_key():
22+
"""An int parameter-dict key stringifies (so {1: v} fills positional $1), matching pybind11."""
23+
con = duckdb.connect()
24+
assert con.execute("SELECT $1 AS a", {1: 5}).fetchall() == [(5,)]
25+
26+
27+
def test_execute_str_param_key():
28+
con = duckdb.connect()
29+
assert con.execute("SELECT $name AS a", {"name": 7}).fetchall() == [(7,)]
30+
31+
32+
def test_struct_type_int_field_key():
33+
"""An int struct field-name key stringifies to "1" (matching pybind11), not a raw std::bad_cast."""
34+
assert str(duckdb.struct_type({1: "INTEGER"})) == 'STRUCT("1" INTEGER)'
35+
36+
37+
def test_struct_type_str_field_key():
38+
assert str(duckdb.struct_type({"a": "INTEGER"})) == "STRUCT(a INTEGER)"
39+
40+
41+
def test_bytes_param_key_decodes():
42+
"""A bytes param-dict key is UTF-8 decoded (b'1' -> '1'); bytes consistently decode at coercion sites."""
43+
con = duckdb.connect()
44+
assert con.execute("SELECT $1 AS a", {b"1": 5}).fetchall() == [(5,)]
45+
46+
47+
def test_bytes_struct_field_key_decodes():
48+
"""A bytes struct field-name key is UTF-8 decoded (b'a' -> 'a'); bytes consistently decode at coercion sites.
49+
50+
These coercion sites previously surfaced a raw 'std::bad_cast' for non-str input; each test here asserting a
51+
concrete result also guards that regression (a std::bad_cast would raise and fail the assertion).
52+
"""
53+
assert str(duckdb.struct_type({b"a": "INTEGER"})) == "STRUCT(a INTEGER)"

0 commit comments

Comments
 (0)