From d5076bded3046cb72f3f2522a000049da583423b Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Sun, 19 Jul 2026 11:44:31 +0300 Subject: [PATCH] ifcparse: store integer attribute values as int64_t to allow out-of-range timestamps Setting an IfcInteger/IfcTimeStamp typed attribute (e.g. IfcOwnerHistory.CreationDate) outside the signed 32-bit range corrupted the value instead of raising, since the Python wrapper's set_attribute_value_py() truncated it with a plain static_cast before handing it to the C++ storage. Unix timestamps before 1901-12-13 or after 2038-01-19 silently wrapped around (e.g. 3000000000 became -1294967296) rather than being rejected or stored correctly. Fixes #3058, equivalent to PR #8683 but ported to this branch's rewritten ifcparse (snake_case files, variant_array/instance_data storage, SWIG PyObject-based attribute setter) instead of the old IfcEntityInstanceData sources, which no longer exist here. The scalar slot of the attribute variant (Argument_INT) becomes int64_t. Integer aggregates (Argument_AGGREGATE_OF_INT, e.g. CoordIndex) and instance/reference identifiers stay 32-bit, since neither is the value that overflows here; this narrow scope is kept on its own technical merits (aggregates and identifiers were never the source of the bug, and widening them would be a much larger, riskier change for no benefit) even though aothms said compatibility isn't a concern on this v0.9-track branch. express::Base::set_attribute_value promotes the schema-generated int to int64_t at a single choke point, so the generated setters keep compiling unchanged. The STEP lexer, writer, and SWIG wrapper (set_attribute_value_py, pythonize) are all widened together, since widening only the Python-facing setter would have silently wrapped the value on file write instead of raising. Verified in a build (IFC2X3 and IFC4, BUILD_IFCGEOM off, no kernels): pre-1901, post-2038, both 32-bit boundaries, and a 9e12 value all round trip exactly both in memory and through STEP text serialization (write then reopen). A value outside the 64-bit range now raises a clean exception instead of corrupting data. Ordinary in-range integers and integer aggregates (e.g. IfcTriangulatedFaceSet.CoordIndex) are unaffected. The existing util/test_attribute.py and test_file.py suites pass unchanged; test_entity_instance.py has 5 pre-existing failures unrelated to this change (confirmed identical on an unfixed build of this branch, caused by a missing get_info_2 binding and _patch_swig_comparisons never being implemented here). Generated with the assistance of an AI coding tool. --- src/ifcparse/entity_instance_data.cpp | 13 +++++++++---- src/ifcparse/instance_data.h | 12 ++++++++++-- src/ifcparse/parse.cpp | 24 ++++++++++++++++++------ src/ifcparse/storage.h | 8 ++++---- src/ifcwrap/IfcParseWrapper.i | 17 ++++++++++++++++- src/ifcwrap/utils/type_conversion.i | 1 + 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/ifcparse/entity_instance_data.cpp b/src/ifcparse/entity_instance_data.cpp index 8ca174e977..64e9eb5855 100644 --- a/src/ifcparse/entity_instance_data.cpp +++ b/src/ifcparse/entity_instance_data.cpp @@ -10,7 +10,7 @@ public: int operator()(const blank& /*i*/) const { return -1; } int operator()(const derived& /*i*/) const { return -1; } - int operator()(const int& /*i*/) const { return -1; } + int operator()(const int64_t& /*i*/) const { return -1; } int operator()(const bool& /*i*/) const { return -1; } int operator()(const boost::logic::tribool& /*i*/) const { return -1; } int operator()(const double& /*i*/) const { return -1; } @@ -126,7 +126,12 @@ namespace { attribute_value::operator int() const { - return dispatch_get_(array_, storage_model_, instance_name_, entity_or_type_, index_); + return (int)dispatch_get_(array_, storage_model_, instance_name_, entity_or_type_, index_); +} + +attribute_value::operator int64_t() const +{ + return dispatch_get_(array_, storage_model_, instance_name_, entity_or_type_, index_); } attribute_value::operator bool() const @@ -523,7 +528,7 @@ void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declarat } template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const blank& value); -template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const int& value); +template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const int64_t& value); template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const bool& value); template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const boost::logic::tribool& value); template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const double& value); @@ -546,7 +551,7 @@ template IFC_PARSE_API void rocks_db_attribute_storage::set(v template IFC_PARSE_API void rocks_db_attribute_storage::set(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index, const empty_aggregate_of_aggregate_t& value); template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; -template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; +template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; template IFC_PARSE_API bool rocks_db_attribute_storage::has(void* storage, const ifcopenshell::declaration* decl, std::size_t identity, size_t index) const; diff --git a/src/ifcparse/instance_data.h b/src/ifcparse/instance_data.h index 46eaa4e73a..0b6fec6a91 100644 --- a/src/ifcparse/instance_data.h +++ b/src/ifcparse/instance_data.h @@ -89,6 +89,11 @@ namespace impl { static std::string get() { return "int"; } }; + template <> + struct VariantTypeName { + static std::string get() { return "int"; } + }; + template <> struct VariantTypeName { static std::string get() { return "bool"; } @@ -154,7 +159,9 @@ typedef parameter_pack < // An integer argument, e.g. 123 // SCALARS: - int, + // Stored as int64_t so integer attributes (IfcInteger, IfcTimeStamp) + // can hold values outside the signed 32-bit range. + int64_t, // A boolean argument, it will serialize to either .T. or .F. bool, // A logical argument, it will serialize to either .T. or .F. or .U. @@ -395,6 +402,7 @@ public: {} operator int() const; + operator int64_t() const; operator bool() const; operator boost::logic::tribool() const; operator double() const; @@ -425,7 +433,7 @@ public: case ifcopenshell::Argument_DERIVED: return visitor(derived{}); case ifcopenshell::Argument_INT: - return visitor((int)*this); + return visitor((int64_t)*this); case ifcopenshell::Argument_BOOL: return visitor((bool)*this); case ifcopenshell::Argument_LOGICAL: { diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp index 1f8c904ced..b26e25b9a4 100644 --- a/src/ifcparse/parse.cpp +++ b/src/ifcparse/parse.cpp @@ -363,9 +363,9 @@ token spf_lexer::next() { throw invalid_token_exception(pos, str, "instance name"); } pop_pool_entry(); - return token(pos, ttype, int_val); + return token(pos, ttype, (int64_t)int_val); } else if (ttype == token::Token_NONE && !str.empty()) { - int int_val; + int64_t int_val; double float_val; auto& first = str.front(); if ((first >= 'A' && first <= 'Z') || (first >= 'a' && first <= 'z')) { @@ -446,7 +446,7 @@ bool token::is_float() { #endif } -int token::as_int() { +int64_t token::as_int() { if (type != Token_INT) { throw invalid_token_exception(start_pos, to_string(), "integer"); } @@ -948,7 +948,12 @@ direct_aggregate read_direct_aggregate( storage.register_inverse((unsigned)*entity_instance_name, entity, next.value_int, attribute_index); } dispatch_token_direct(next, aggregate_type && aggregate_type->type_of_element()->as_named_type() ? aggregate_type->type_of_element()->as_named_type()->declared_type() : nullptr, attribute_index, logger, [&aggregate](const auto& value) { - aggregate.append(value); + // Scalar integers are parsed as int64_t, but integer aggregates remain 32-bit; narrow here. + if constexpr (std::is_same_v, int64_t>) { + aggregate.append((int)value); + } else { + aggregate.append(value); + } }); } next = tokens->next(); @@ -1237,7 +1242,7 @@ namespace { upper_(upper) {} void operator()(const blank& /*i*/) { data_ << "$"; } void operator()(const derived& /*i*/) { data_ << "*"; } - void operator()(const int& i) { data_ << i; } + void operator()(const int64_t& i) { data_ << i; } void operator()(const bool& i) { data_ << (i ? ".T." : ".F."); } void operator()(const boost::logic::tribool& i) { data_ << (i ? ".T." : (boost::logic::indeterminate(i) ? ".U." : ".F.")); } void operator()(const double& i) { data_ << format_double(i); } @@ -1578,7 +1583,12 @@ express::Base::set_attribute_value(size_t i, const T& t) { { void* const storage = std::visit([](const auto& m) { return (void*)&m; }, file()->storage_); - data()->set_attribute_value(i, t); + if constexpr (std::is_same_v, int>) { + // Integer attributes are stored as int64_t; widen the schema-generated int here. + data()->set_attribute_value(i, (int64_t)t); + } else { + data()->set_attribute_value(i, t); + } } auto new_attribute = get_attribute_value(i); @@ -3421,6 +3431,7 @@ void express::Base::set_attribute_value(const std::string& name, const express:: template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const blank& value); template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const derived& value); template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const int& value); +template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const int64_t& value); template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const bool& value); template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const boost::logic::tribool& value); template void IFC_PARSE_API express::Base::set_attribute_value(size_t index, const double& value); @@ -3439,6 +3450,7 @@ template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const blank& value); template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const derived& value); template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const int& value); +template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const int64_t& value); template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const bool& value); template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const boost::logic::tribool& value); template void IFC_PARSE_API express::Base::set_attribute_value(const std::string& name, const double& value); diff --git a/src/ifcparse/storage.h b/src/ifcparse/storage.h index 3a8a2cbd8c..447fc1c097 100644 --- a/src/ifcparse/storage.h +++ b/src/ifcparse/storage.h @@ -161,18 +161,18 @@ namespace ifcopenshell { union { char value_char; //types: OPERATOR - int value_int; //types: INT, IDENTIFIER + int64_t value_int; //types: INT, IDENTIFIER double value_double; //types: FLOAT const std::string* value_string; //types: STR, ENUM, KEYWORD; lifetime managed by spf_lexer::string_pool_ }; token() : start_pos(0), type(Token_NONE) {} - + token(size_t start_position, token_type token_kind, const std::string& string_value) : start_pos(start_position), type(token_kind), value_string(&string_value) {} - token(size_t start_position, token_type token_kind, int integer_value) + token(size_t start_position, token_type token_kind, int64_t integer_value) : start_pos(start_position), type(token_kind), value_int(integer_value) {} token(size_t start_position, double floating_value) @@ -196,7 +196,7 @@ namespace ifcopenshell { bool is_float(); bool is_binary(); - int as_int(); + int64_t as_int(); unsigned as_identifier(); bool as_bool(); boost::logic::tribool as_logical(); diff --git a/src/ifcwrap/IfcParseWrapper.i b/src/ifcwrap/IfcParseWrapper.i index 863e622491..8b64de41f8 100644 --- a/src/ifcwrap/IfcParseWrapper.i +++ b/src/ifcwrap/IfcParseWrapper.i @@ -646,6 +646,21 @@ private: return v; }; + auto to_index_i64 = [&](PyObject* o) -> long long { + PyObject* idx = PyNumber_Index(o); // accepts numpy ints, bools, etc. + if (!idx) { + PyErr_Clear(); + throw ifcopenshell::exception("Attribute not set"); + } + long long v = PyLong_AsLongLong(idx); + Py_DECREF(idx); + if (PyErr_Occurred()) { + PyErr_Clear(); + throw ifcopenshell::exception("Attribute not set"); + } + return v; + }; + auto to_double = [&](PyObject* o) -> double { double v = PyFloat_AsDouble(o); // accepts ints and float-like objects if (PyErr_Occurred()) { @@ -819,7 +834,7 @@ private: switch (arg_type) { case ifcopenshell::Argument_INT: { - self->set_attribute_value(i, static_cast(to_index_long(value))); + self->set_attribute_value(i, static_cast(to_index_i64(value))); return; } case ifcopenshell::Argument_BOOL: { diff --git a/src/ifcwrap/utils/type_conversion.i b/src/ifcwrap/utils/type_conversion.i index ad6a5177b6..2872a99161 100644 --- a/src/ifcwrap/utils/type_conversion.i +++ b/src/ifcwrap/utils/type_conversion.i @@ -249,6 +249,7 @@ } PyObject* pythonize(const int& t) { return PyInt_FromLong(t); } + PyObject* pythonize(const int64_t& t) { return PyLong_FromLongLong(t); } PyObject* pythonize(const unsigned int& t) { return PyInt_FromLong(t); } PyObject* pythonize(const bool& t) { return PyBool_FromLong(t); } PyObject* pythonize(const boost::logic::tribool& t) { return boost::logic::indeterminate(t) ? PyUnicode_FromString("UNKNOWN") : PyBool_FromLong((bool)t) ;}