Discussion:
Redefining assert can cause ODR violations
Felipe Magno de Almeida
2010-07-29 19:29:16 UTC
Permalink
Hello,

I see that mico redefines assert macro with something specific. I hope
I'm missing something, but I don't think this should be done, because
it can lead to One Definition Rule violations, which causes undefined
behavior.
For example:

-- a.h --

inline void foo(int x)
{
assert(x != 0);
}

-- a.cpp --

#include "a.h"

void bar()
{
foo(1);
}

-- b.cpp --

#include <CORBA.h>
#include "a.h"

void my_bar()
{
foo(1);
}

-- --

And we have multiple implementations of foo which are different after
preprocessing.
Also, I don't see why mico should have the responsability to address
assert's for code outside its own.
If mico wants to optimize its assertions, it should define a
MICO_ASSERT macro that does whatever it wants it to do.


Regards,
--
Felipe Magno de Almeida
Karel Gardas
2010-07-31 21:29:56 UTC
Permalink
Hello,

indeed. mico/assert.h is included inside CORBA.h which is not the best
and pollutes user code with MICO assert. It might be interesting to see
object files sizes change when MICO's assert is used and when the std
assert is used as comment in mico/assert.h suggests some dramatic
differences. Do you care to do such test on your platform?

Thanks,
Karel
Post by Felipe Magno de Almeida
Hello,
I see that mico redefines assert macro with something specific. I hope
I'm missing something, but I don't think this should be done, because
it can lead to One Definition Rule violations, which causes undefined
behavior.
-- a.h --
inline void foo(int x)
{
assert(x != 0);
}
-- a.cpp --
#include "a.h"
void bar()
{
foo(1);
}
-- b.cpp --
#include <CORBA.h>
#include "a.h"
void my_bar()
{
foo(1);
}
-- --
And we have multiple implementations of foo which are different after
preprocessing.
Also, I don't see why mico should have the responsability to address
assert's for code outside its own.
If mico wants to optimize its assertions, it should define a
MICO_ASSERT macro that does whatever it wants it to do.
Regards,
--
Karel Gardas ***@objectsecurity.com
ObjectSecurity Ltd. http://www.objectsecurity.com
Karel Gardas
2010-08-13 20:52:14 UTC
Permalink
Hi Felipe,

this issue is fixed in the latest MICO development sources. Either get
them from darcs repository or using this snapshot:
http://www.mico.org/snapshots/mico-100813.tar.bz2 for testing.

Thanks,
Karel
Hi Filipe,
I'm asking to do this test since if the result is huge difference then
we shall try to clean MICO's headers to not pollute application level
code with our assert. If the result is acceptable then we shall remove
MICO assert completely. Even if the result is huge difference then we
might consider disabling assert for normal build by -DNDEBUG. I'm
looking forward to seeing your results.
For msvc-8.0 debug and gcc debug there's absolutely no difference in mico's
library size, static or shared. Since assert shouldn't be defined in release I
didn't test that scenario.
Thanks for doing this test!
Karel
Regards,
--
Karel Gardas ***@objectsecurity.com
ObjectSecurity Ltd. http://www.objectsecurity.com
Loading...