[SCM] GiNaC -- a C++ library for symbolic computations branch, ginac_1-5, updated. release_1-4-0-198-g8bf0597
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GiNaC -- a C++ library for symbolic computations". The branch, ginac_1-5 has been updated via 8bf0597dde55e4c94a2ff39f1d8130902e3d7a9b (commit) via eaf81b3115697a8e883848ace0ceb919cf443b2c (commit) from eb192338b3abd1252523a013a4f924ee38b10039 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 8bf0597dde55e4c94a2ff39f1d8130902e3d7a9b Author: Jens Vollinga <jensv@balin.nikhef.nl> Date: Fri Jul 31 12:48:58 2009 +0200 Fixed the parser such that it can read in user defined classes again. Fixed default reader to parse also pow, sqrt and power. commit eaf81b3115697a8e883848ace0ceb919cf443b2c Author: Jens Vollinga <jensv@balin.nikhef.nl> Date: Fri Jul 31 11:14:01 2009 +0200 Fixed cast that caused compile error on 64bit machines. ----------------------------------------------------------------------- Summary of changes: ginac/function.pl | 3 ++ ginac/parser/builtin_fcns.def | 76 --------------------------------------- ginac/parser/default_reader.tpl | 5 +++ ginac/parser/parser.cpp | 12 +++++- 4 files changed, 18 insertions(+), 78 deletions(-) hooks/post-receive -- GiNaC -- a C++ library for symbolic computations
Dear Jens, On Fri, Jul 31, 2009 at 12:50:26PM +0200, Jens Vollinga wrote:
commit eaf81b3115697a8e883848ace0ceb919cf443b2c Author: Jens Vollinga <jensv@balin.nikhef.nl> Date: Fri Jul 31 11:14:01 2009 +0200
Fixed cast that caused compile error on 64bit machines.
diff --git a/ginac/parser/parser.cpp b/ginac/parser/parser.cpp index a46017d..d91b7e8 100644 --- a/ginac/parser/parser.cpp +++ b/ginac/parser/parser.cpp @@ -66,8 +66,16 @@ ex parser::parse_identifier_expr() Parse_error_("no function \"" << name << "\" with " << args.size() << " arguments"); } - ex ret = GiNaC::function(reinterpret_cast<unsigned>(reader->second), args); - return ret; + // dirty hack to distinguish between serial numbers of functions and real + // pointers. + ex ret; + try { + ret = GiNaC::function(reinterpret_cast<unsigned>(reader->second), args); + } + catch ( std::runtime_error ) { + ret = reader->second(args); + } + return ret; } I'm afraid this code is a bit dangerous. Suppose reader->second corresponds to the serial of some (GiNaC::)function (and not a pointer to a C++ function). ret = GiNaC::function(...) calls eval(), and it might throw an exception. We catch the exception and dereference ->second => oops... Best regards, Alexei
Hi Alexei, Alexei Sheplyakov schrieb:
I'm afraid this code is a bit dangerous.
Suppose reader->second corresponds to the serial of some (GiNaC::)function (and not a pointer to a C++ function). ret = GiNaC::function(...) calls eval(), and it might throw an exception. We catch the exception and dereference ->second => oops...
you are right. I fixed it now. Thanks! Regards, Jens
On Fri, Jul 31, 2009 at 02:43:38PM +0200, Jens Vollinga wrote:
Alexei Sheplyakov schrieb:
I'm afraid this code is a bit dangerous.
Suppose reader->second corresponds to the serial of some (GiNaC::)function (and not a pointer to a C++ function). ret = GiNaC::function(...) calls eval(), and it might throw an exception. We catch the exception and dereference ->second => oops...
you are right. I fixed it now. Thanks!
I don't think the new variant is any different. 69 // dirty hack to distinguish between serial numbers of functions and real 70 // pointers. 71 try { 72 GiNaC::function f(reinterpret_cast<unsigned>(reader->second), args); 73 return f; eval() is still called here. The compiler has to call ex::ex(const basic&) ctor in order to convert the value being returned to the correct type (ex). That ctor calls ex::construct_from_basic(const basic&), and it calls eval() (which might throw an exception, and so on). Best regards, Alexei
On Fri, Jul 31, 2009 at 03:30:45PM +0200, Jens Vollinga wrote:
oops, yes, thanks. What was I thinking?!?
71 GiNaC::function* f; 72 try { 73 f = new GiNaC::function(reinterpret_cast<unsigned>(reader->second), args); 74 } 75 catch ( std::runtime_error ) { I think if (f) delete f; should be inserted here, otherwise we might leak memory if ->second is a pointer to a (C++) function. Best regards, Alexei
Dear Jens, On Fri, Jul 31, 2009 at 12:50:26PM +0200, Jens Vollinga wrote:
commit eaf81b3115697a8e883848ace0ceb919cf443b2c Author: Jens Vollinga <jensv@balin.nikhef.nl> Date: Fri Jul 31 11:14:01 2009 +0200
Fixed cast that caused compile error on 64bit machines.
Unfortunately the error is still here: libtool: compile: ccache g++-4.1 -DHAVE_CONFIG_H -I. -I../../../../work/sw/GiNaC/ginac -I../config -I/home/pc7135/varg/target/x86_64-linux-gnu/include -Wall -O2 -g -pipe -funroll-loops -ffast-math -finline-limit=1200 -m64 -march=k8 -mfpmath=sse -msse2 -MT parser.lo -MD -MP -MF .deps/parser.Tpo -c ../../../../work/sw/GiNaC/ginac/parser/parser.cpp -fPIC -DPIC -o .libs/parser.o ../../../../work/sw/GiNaC/ginac/parser/parser.cpp: In member function `GiNaC::ex GiNaC::parser::parse_identifier_expr()': ../../../../work/sw/GiNaC/ginac/parser/parser.cpp:73: error: cast from `GiNaC::ex (*)(const GiNaC::exvector&)' to `unsigned int' loses precision A simple test case: $ cat > test.cc << EOF unsigned test(const void* ptr) { return reinterpret_cast<unsigned>(ptr); } EOF $ g++ test.cc test.cc: In function `unsigned int test(const void*)': test.cc:3: error: cast from `const void*' to `unsigned int' loses precision I think the compiler is right since [expr.reinterpret.cast] says: "A pointer can be explicitly converted to any integral type large enough to hold it." The patch below fixes the problem. From: Alexei Sheplyakov <varg@metalica.kh.ua> Subject: [PATCH] Fix compilation error on 64-bit architectures for real. According to [expr.reinterpret.cast] it's not ok to convert a pointer into an integer of a different size. --- ginac/parser/parser.cpp | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/ginac/parser/parser.cpp b/ginac/parser/parser.cpp index 6ed364f..cfa313b 100644 --- a/ginac/parser/parser.cpp +++ b/ginac/parser/parser.cpp @@ -70,7 +70,8 @@ ex parser::parse_identifier_expr() // pointers. GiNaC::function* f = NULL; try { - f = new GiNaC::function(reinterpret_cast<unsigned>(reader->second), args); + unsigned serial = (unsigned)(unsigned long)(void *)(reader->second); + f = new GiNaC::function(serial, args); } catch ( std::runtime_error ) { if ( f ) delete f; -- 1.6.3.3 Best regards, Alexei
--- On Fri, 8/7/09, Alexei Sheplyakov <varg@metalica.kh.ua> wrote:
From: Alexei Sheplyakov <varg@metalica.kh.ua> Subject: [GiNaC-devel] Fix the compliation error *for real* To: "GiNaC development list" <ginac-devel@ginac.de> Date: Friday, August 7, 2009, 1:22 PM Dear Jens,
On Fri, Jul 31, 2009 at 12:50:26PM +0200, Jens Vollinga wrote:
commit eaf81b3115697a8e883848ace0ceb919cf443b2c Author: Jens Vollinga <jensv@balin.nikhef.nl> Date: Fri Jul 31 11:14:01 2009 +0200
Fixed cast that caused compile error on 64bit machines.
Unfortunately the error is still here:
libtool: compile: ccache g++-4.1 -DHAVE_CONFIG_H -I. -I../../../../work/sw/GiNaC/ginac -I../config -I/home/pc7135/varg/target/x86_64-linux-gnu/include -Wall -O2 -g -pipe -funroll-loops -ffast-math -finline-limit=1200 -m64 -march=k8 -mfpmath=sse -msse2 -MT parser.lo -MD -MP -MF .deps/parser.Tpo -c ../../../../work/sw/GiNaC/ginac/parser/parser.cpp -fPIC -DPIC -o .libs/parser.o ../../../../work/sw/GiNaC/ginac/parser/parser.cpp: In member function `GiNaC::ex GiNaC::parser::parse_identifier_expr()': ../../../../work/sw/GiNaC/ginac/parser/parser.cpp:73: error: cast from `GiNaC::ex (*)(const GiNaC::exvector&)' to `unsigned int' loses precision
A simple test case: $ cat > test.cc << EOF unsigned test(const void* ptr) { return reinterpret_cast<unsigned>(ptr); } EOF $ g++ test.cc test.cc: In function `unsigned int test(const void*)': test.cc:3: error: cast from `const void*' to `unsigned int' loses precision
I think the compiler is right since [expr.reinterpret.cast] says: "A pointer can be explicitly converted to any integral type large enough to hold it."
The patch below fixes the problem.
From: Alexei Sheplyakov <varg@metalica.kh.ua> Subject: [PATCH] Fix compilation error on 64-bit architectures for real.
According to [expr.reinterpret.cast] it's not ok to convert a pointer into an integer of a different size.
--- ginac/parser/parser.cpp | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/ginac/parser/parser.cpp b/ginac/parser/parser.cpp index 6ed364f..cfa313b 100644 --- a/ginac/parser/parser.cpp +++ b/ginac/parser/parser.cpp @@ -70,7 +70,8 @@ ex parser::parse_identifier_expr() // pointers. GiNaC::function* f = NULL; try { - f = new GiNaC::function(reinterpret_cast<unsigned>(reader->second), args); + unsigned serial = (unsigned)(unsigned long)(void *)(reader->second); + f = new GiNaC::function(serial, args); } catch ( std::runtime_error ) { if ( f ) delete f; -- 1.6.3.3
Best regards, Alexei
_______________________________________________ GiNaC-devel mailing list GiNaC-devel@ginac.de https://www.cebix.net/mailman/listinfo/ginac-devel
Guys, I think a much more conceptually correct solution (even though this one works) would be to use size_t instead of unsigned long . The following links explain why: http://www.embedded.com/columns/programmingpointers/201803576 http://en.wikipedia.org/wiki/Stdlib.h#size_t http://www.cplusplus.com/reference/clibrary/cstddef/size_t/ . To put it shortly - size_t is big enough to represent output of sizeof(foo) for _any_ foo, so it is big enough to represent numeric value of _any_ pointer. Regards, Sergei.
Hello, On Fri, Aug 07, 2009 at 04:04:46PM -0700, Sergei Steshenko wrote:
Guys, I think a much more conceptually correct solution (even though this one works) would be to use
size_t
instead of
unsigned long .
The politically correct type is uintptr_t. And (AFAIK) the sizeof(size_t) is NOT guaranteed to be the same as sizeof(void*). And on any sane platform sizeof(long) == sizeof(void*) anyway. Best regards, Alexei
--- On Sat, 8/8/09, Alexei Sheplyakov <varg@metalica.kh.ua> wrote:
From: Alexei Sheplyakov <varg@metalica.kh.ua> Subject: Re: [GiNaC-devel] Fix the compliation error *for real* To: "GiNaC development list" <ginac-devel@ginac.de> Date: Saturday, August 8, 2009, 1:45 AM Hello,
On Fri, Aug 07, 2009 at 04:04:46PM -0700, Sergei Steshenko wrote:
Guys, I think a much more conceptually correct solution (even though this one works) would be to use
size_t
instead of
unsigned long .
The politically correct type is uintptr_t. And (AFAIK) the sizeof(size_t) is NOT guaranteed to be the same as sizeof(void*). And on any sane platform sizeof(long) == sizeof(void*) anyway.
Best regards, Alexei _______________________________________________
Alexei, this is what http://www.cplusplus.com/reference/clibrary/cstddef/size_t/ says: " size_t corresponds to the integral data type returned by the language operator sizeof and is defined in the <cstddef> header file (among others) as an unsigned integral type. It expresses a size or count in bytes. ". Please note that 'sizeof' argument is _not_ mentioned, and it makes sense. My logic us that since size_t is is good for _any_ object, it is good for any array A, including the array occupying the whole address space. Here is the same interpretation: http://www.viva64.com/terminology/size_t.html : " size_t. A basic unsigned integer C/C++ type. The type’s size is chosen in such a way as to allow you to write the maximum size of a theoretically possible array into it. On a 32-bit system size_t will take 32 bits and on a 64-bit one – 64 bits. In other words, a pointer can be safely put inside size_t type (an exception is class-function-pointers but this is a special case). size_t type is usually used for loop, array indexing, size storage and address arithmetic counters. In some cases using size_t type is more effective and safe than using a more habitual for the programmer unsigned type. " - please not the " In other words, a pointer can be safely put inside size_t type (an exception is class-function-pointers but this is a special case). size_t type is usually used for loop, array indexing, size storage and address arithmetic counters. " part. Regards, Sergei.
Hello,
Subject: [PATCH] Fix compilation error on 64-bit architectures for real.
According to [expr.reinterpret.cast] it's not ok to convert a pointer into an integer of a different size. [skipped the patch]
Although reading user-defined classes works again it's a bit slower than it used to be. The fix is below (should be applied after the patch mentioned above). Please note: this patch is for 1.5 branch only, I'll post a different patch for the master branch. From: Alexei Sheplyakov <varg@metalica.kh.ua> Subject: [PATCH][GiNaC 1.5] Make the parser as fast as it used to be (before 1.5.2). Commit 8bf0597dde55e4c94a2ff39f1d8130902e3d7a9b (titled as 'Fixed the parser such that it can read in user defined classes again.') made the parser a bit slower, especially if the input contains many terms of user-defined type. The reason for that is quite simple: we throw and catch an exception every time we construct an object of user-defined type: // dirty hack to distinguish between serial numbers of functions and real // pointers. GiNaC::function* f = NULL; try { unsigned serial = (unsigned)(unsigned long)(void *)(reader->second); f = new GiNaC::function(serial, args); } catch ( std::runtime_error ) { if ( f ) delete f; ex ret = reader->second(args); return ret; } Fortunately functions are aligned and we can use much more efficient technique to distinguish between serial and pointers to functions. --- ginac/parser/default_reader.tpl | 55 ++++++++++++++++++++++++++++---------- ginac/parser/parser.cpp | 47 ++++++++++++++++++++++++--------- 2 files changed, 74 insertions(+), 28 deletions(-) diff --git a/ginac/parser/default_reader.tpl b/ginac/parser/default_reader.tpl index 5f83cfe..2018e04 100644 --- a/ginac/parser/default_reader.tpl +++ b/ginac/parser/default_reader.tpl @@ -19,6 +19,12 @@ COMMENT a part of GiNaC parser -- construct functions from a byte stream. #include "power.h" #include "operators.h" #include "inifcns.h" +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif +#ifdef HAVE_STDINT_H +#include <stdint.h> // for uintptr_t +#endif namespace GiNaC { @@ -31,6 +37,31 @@ static ex [+ (get "name") +]_reader(const exvector& ev) (format '#f "~{ev[~a]~^, ~}" (sequence 0 (- nargs 1)))) +]); }[+ ENDFOR +] +// function::registered_functions() is protected, but we need to access it +class registered_functions_hack : public function +{ +public: + static const std::vector<function_options>& get_registered_functions() + { + return function::registered_functions(); + } +private: + registered_functions_hack(); + registered_functions_hack(const registered_functions_hack&); + registered_functions_hack& operator=(const registered_functions_hack&); +}; + +// Encode an integer into a pointer to a function. Since functions +// are aligned (the minimal alignment depends on CPU architecture) +// we can distinguish between pointers and integers. +static reader_func encode_serial_as_reader_func(unsigned serial) +{ + uintptr_t u = (uintptr_t)serial; + u = (u << 1) | (uintptr_t)1; + reader_func ptr = reinterpret_cast<reader_func>((void *)u); + return ptr; +} + const prototype_table& get_default_reader() { using std::make_pair; @@ -42,22 +73,16 @@ const prototype_table& get_default_reader() (if (exist? "args") (get "args") "1") +])] = [+ (get "name") +]_reader;[+ ENDFOR +] - try { - for ( unsigned ser=0; ; ++ser ) { - GiNaC::function f(ser); - std::string name = f.get_name(); - for ( std::size_t nargs=0; ; ++nargs ) { - try { - function::find_function(name, nargs); - prototype proto = std::pair<std::string, std::size_t>(name, nargs); - std::pair<prototype_table::iterator, bool> ins = reader.insert(std::pair<prototype,reader_func>(proto, (reader_func)ser)); - if ( ins.second ) break; - } - catch ( std::runtime_error ) { } - } - } + std::vector<function_options>::const_iterator it = + registered_functions_hack::get_registered_functions().begin(); + std::vector<function_options>::const_iterator end = + registered_functions_hack::get_registered_functions().end(); + unsigned serial = 0; + for (; it != end; ++it) { + prototype proto = make_pair(it->get_name(), it->get_nparams()); + reader[proto] = encode_serial_as_reader_func(serial); + ++serial; } - catch ( std::runtime_error ) { } initialized = true; } return reader; diff --git a/ginac/parser/parser.cpp b/ginac/parser/parser.cpp index cfa313b..acf5f3d 100644 --- a/ginac/parser/parser.cpp +++ b/ginac/parser/parser.cpp @@ -32,6 +32,36 @@ namespace GiNaC { +// <KLUDGE> +// Find out if ptr is a pointer to a function or a specially crafted integer. +// It's possible to distinguish between these because functions are aligned. +// Returns true if ptr is a pointer and false otherwise. +static bool decode_serial(unsigned& serial, const reader_func ptr) +{ + uintptr_t u = (uintptr_t)(void *)ptr; + if (u & 1) { + u >>= 1; + serial = (unsigned)u; + return false; + } + return true; +} + +// Figures out if ptr is a pointer to function or a serial of GiNaC function. +// In the former case calls that function, in the latter case constructs +// GiNaC function with corresponding serial and arguments. +static ex dispatch_reader_fcn(const reader_func ptr, const exvector& args) +{ + unsigned serial = 0; // dear gcc, could you please shut up? + bool is_ptr = decode_serial(serial, ptr); + if (is_ptr) + return ptr(args); + else + return function(serial, args); +} +// </KLUDGE> + + /// identifier_expr: identifier | identifier '(' expression* ')' ex parser::parse_identifier_expr() { @@ -66,19 +96,10 @@ ex parser::parse_identifier_expr() Parse_error_("no function \"" << name << "\" with " << args.size() << " arguments"); } - // dirty hack to distinguish between serial numbers of functions and real - // pointers. - GiNaC::function* f = NULL; - try { - unsigned serial = (unsigned)(unsigned long)(void *)(reader->second); - f = new GiNaC::function(serial, args); - } - catch ( std::runtime_error ) { - if ( f ) delete f; - ex ret = reader->second(args); - return ret; - } - return f->setflag(status_flags::dynallocated); + // reader->second might be a pointer to a C++ function or a specially + // crafted serial of a GiNaC::function. + ex ret = dispatch_reader_fcn(reader->second, args); + return ret; } /// paren_expr: '(' expression ')' -- 1.6.3.3 Best regards, Alexei
participants (4)
-
Alexei Sheplyakov
-
git@ginac.de
-
Jens Vollinga
-
Sergei Steshenko