bug in ex::ex(const std::string&, const ex&)
Hi, I think there is a bug in the functions that convert strings to ex. I attach a minimal program that segfaults on my machine. I am using gcc 13.2.0 on Ubuntu 23.10, x86_64, CPU Intel i9-12900H. The problem only arises if I compile GiNaC using the CMake script. The same program works if I compile with configure && make. The problem appears to be in parser/default_reader.cpp, where conversions between pointers to functions and integers are used. The code assumes that the integer underlylng a pointer to function is even, relying on the fact that functions are aligned; this latter condition appears to fail on my system when compiling with CMake. Indeed, inserting a pragma command inside parser/default_reader.cpp (see attached patch) fixes the problem. I do not know if this can be made portable across compilers; maybe it would be easier to modify the code by replacing reader_func with a class that handles the conversion in a type-safe way? Diego Conti
Hi Diego, Thank you for reporting a bug. What is your platform? Please share the output of ./config/config.guess. On 4/3/24 10:23 AM, Diego Conti wrote:
Indeed, inserting a pragma command inside parser/default_reader.cpp (see attached patch) fixes the problem. I do not know if this can be made portable across compilers; maybe it would be easier to modify the code by replacing reader_func with a class that handles the conversion in a type-safe way?
Could you, please, explain to us how you arrived at this patch? All my best, -richy. -- Richard B. Kreckel <https://in.terlu.de/~kreckel/>
Hi Richard, thanks for your reply. On 03/04/24 22:47, Richard B. Kreckel wrote:
Hi Diego,
Thank you for reporting a bug.
What is your platform? Please share the output of ./config/config.guess.
x86_64-pc-linux-gnu
On 4/3/24 10:23 AM, Diego Conti wrote:
Indeed, inserting a pragma command inside parser/default_reader.cpp (see attached patch) fixes the problem. I do not know if this can be made portable across compilers; maybe it would be easier to modify the code by replacing reader_func with a class that handles the conversion in a type-safe way?
Could you, please, explain to us how you arrived at this patch?
Well, with the unpatched GiNAC 1.8.7 code the stack trace is as follows: 0 0x00007ffff7c7e192 in GiNaC::function::eval() const () from build/ginac/libginac.so.11 #1 0x00007ffff7c324cf in GiNaC::ex::construct_from_basic(GiNaC::basic const&) () from build/ginac/libginac.so.11 #2 0x000055555555fdf0 in GiNaC::ex::ex(GiNaC::basic const&) () #3 0x00007ffff7dbd383 in GiNaC::dispatch_reader_fcn(GiNaC::ex (*)(std::vector<GiNaC::ex, std::allocator<GiNaC::ex> > const&), std::vector<GiNaC::ex, std::allocator<GiNaC::ex> > const&) () from build/ginac/libginac.so.11 #4 0x00007ffff7dbda9e in GiNaC::parser::parse_identifier_expr() () from build/ginac/libginac.so.11 #5 0x00007ffff7dbe461 in GiNaC::parser::parse_primary() () from build/ginac/libginac.so.11 #6 0x00007ffff7dbe87f in GiNaC::parser::parse_expression() () from build/ginac/libginac.so.11 #7 0x00007ffff7dbed8b in GiNaC::parser::operator()(std::basic_istream<char, std::char_traits<char> >&) () from build/ginac/libginac.so.11 #8 0x00007ffff7dbf0fa in GiNaC::parser::operator()(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from build/ginac/libginac.so.11 #9 0x00007ffff7dbc2c0 in GiNaC::ex::construct_from_string_and_lst(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, GiNaC::ex const&) () from build/ginac/libginac.so.11 #10 0x000055555555fe46 in GiNaC::ex::ex(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, GiNaC::ex const&) () #11 0x000055555555ed92 in main () The function dispatch_reader_fcn is precisely where the pointer-to-integer conversion is made. I can confirm that function alignment is the problem by logging the address of sqrt_reader inside the function get_default_reader (attached patch to default_reader.cpp). This prints out 140737351734977 before crashing (without the pragma). However, the code assumes that the address is even. With the pragma, the code prints out 140737351734992 sqrt(3) and then exits normally. Best, Diego
All my best, -richy.
Hi Diego, On 4/4/24 9:29 AM, Diego Conti wrote:
The function dispatch_reader_fcn is precisely where the pointer-to-integer conversion is made.
I can confirm that function alignment is the problem by logging the address of sqrt_reader inside
the function get_default_reader (attached patch to default_reader.cpp).
This prints out 140737351734977 before crashing (without the pragma). However, the code assumes that the address is even.
With the pragma, the code prints out
140737351734992
sqrt(3)
and then exits normally.
The comments labelled KLUDGE in parser/parser.cpp hint at something wrong. I suppose that rather than adding a #pragma, this mechanism should be fixed. Right away, I don't see what's wrong with this as it seems to match encode_serial_as_reader_func() in default_reader.cpp:75. I suppose the reason why it works with Autoconf but not with CMake is just different optimization: Autoconf turns on -O2 by default but CMake doesn't, so no optimization. It should of course work with any optimization. All my best, -richy. PS: It's great you care to send patches! Please consider creating them with context, i.e. using diff -u (as git diff does by default). It makes them much easier to read and apply. -- Richard B. Kreckel <https://in.terlu.de/~kreckel/>
note that -falign-functions=16 is the default for -O2 or above in gcc on x86_64-linux: $ gcc -O2 --help=optimizers -Q|grep align-functions= -falign-functions= 16 the autotools build system defaults to -O2. cmake defaults to likely nothing unless you set CMAKE_BUILD_TYPE=Release (-O3) or CMAKE_BUILD_TYPE=RelWithDebInfo (-O2). if ginac really expects aligned functions, it should ensure that's the case (adding -falign-functions=16, setting an equivalent pragma or attribute), or explicitly disallow unoptimised builds.
Hi, On 4/4/24 11:10 AM, ovf wrote:
if ginac really expects aligned functions, it should ensure that's the case (adding -falign-functions=16, setting an equivalent pragma or attribute), or explicitly disallow unoptimised builds.
It shouldn't expect aligned functions. A patch for this just went in. All my best, -richy. -- Richard B. Kreckel <https://in.terlu.de/~kreckel/>
participants (3)
-
Diego Conti
-
ovf
-
Richard B. Kreckel