memory error if vari ctor throws

[Bob Carpenter]

I tracked down the problem with log1m() when the argument
is greater than 1.

This unit test:

TEST(AgradRev,log1mErr) {
AVAR a = 10;
AVAR f = log1m(a);
}

causes this error:

Running main() from gtest_main.cc
[==========] Running 2 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 2 tests from AgradRev
[ RUN ] AgradRev.log1m
[ OK ] AgradRev.log1m (0 ms)
[ RUN ] AgradRev.log1mErr
log1m(84891) malloc: *** error for object 0x7fb3d1003218: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
make: *** [test/agrad/rev/log1m] Abort trap: 6

The root cause is that the constructor for log1m_vari(vari*)
throws an exception if the argument has value >= 1. So I'm
pretty sure we've had this problem all along but haven't
stumbled across it.

I'm going to clean up the other vari classes that do this,
too so that the calling function throws rather than the
vari construtor.

I'd like to know what's causing the problem --- it's not in
the destructors, so I'm thinking it must be in the way we
allocate/deallocate the memory stacks.

- Bob

[Ben Goodrich]

On Saturday, May 11, 2013 7:53:41 PM UTC-4, Bob Carpenter wrote:

I'd like to know what's causing the problem --- it's not in
the destructors, so I'm thinking it must be in the way we
allocate/deallocate the memory stacks.

Yes, with -fsanitize=address added to CFLAGS

make clean-all && make CC=clang++ O=0 -j8 test/agrad/rev/log1m
...
[ RUN      ] AgradRev.log1m
=================================================================
==20571==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x631000000850
    #0 0x4573d2 (/opt/stan/test/agrad/rev/log1m+0x4573d2)
    #1 0x472a97 (/opt/stan/test/agrad/rev/log1m+0x472a97)
    #2 0x46ed91 (/opt/stan/test/agrad/rev/log1m+0x46ed91)
    #3 0x59ed72 (/opt/stan/test/agrad/rev/log1m+0x59ed72)
    #4 0x5311ac (/opt/stan/test/agrad/rev/log1m+0x5311ac)
    #5 0x4d1009 (/opt/stan/test/agrad/rev/log1m+0x4d1009)
    #6 0x4d58aa (/opt/stan/test/agrad/rev/log1m+0x4d58aa)
    #7 0x4d8415 (/opt/stan/test/agrad/rev/log1m+0x4d8415)
    #8 0x4f2eb7 (/opt/stan/test/agrad/rev/log1m+0x4f2eb7)
    #9 0x581182 (/opt/stan/test/agrad/rev/log1m+0x581182)
    #10 0x53f1d1 (/opt/stan/test/agrad/rev/log1m+0x53f1d1)
    #11 0x4f1ab4 (/opt/stan/test/agrad/rev/log1m+0x4f1ab4)
    #12 0x46487a (/opt/stan/test/agrad/rev/log1m+0x46487a)
    #13 0x2aaaab92ca54 (/lib/x86_64-linux-gnu/libc-2.17.so+0x21a54)
0x631000000850 is located 80 bytes inside of 65536-byte region [0x631000000800,0x631000010800)
allocated by thread T0 here:
    #0 0x456dc2 (/opt/stan/test/agrad/rev/log1m+0x456dc2)
    #1 0x5d3f49 (/opt/stan/test/agrad/rev/log1m+0x5d3f49)
    #2 0x5d8317 (/opt/stan/test/agrad/rev/log1m+0x5d8317)
    #3 0x5d4cc9 (/opt/stan/test/agrad/rev/log1m+0x5d4cc9)
    #4 0x5d47ce (/opt/stan/test/agrad/rev/log1m+0x5d47ce)
    #5 0x5d482c (/opt/stan/test/agrad/rev/log1m+0x5d482c)
    #6 0x5dd07c (/opt/stan/test/agrad/rev/log1m+0x5dd07c)
==20571==ABORTING
make: *** [test/agrad/rev/log1m] Error 1

Ben

[Bob Carpenter]

Thanks.

Sorry for being so dense here, but I don't know how to
interpret that dump.

For one thing, log1m_vari instances should be smaller
than 80 bytes. Is this multiple ones? Or an attempt to
deallocate something from our var stack that's not there?

Here's the vari:

class log1m_vari : public op_v_vari {
public:
log1m_vari(vari* avi) :
op_v_vari(stan::math::log1p(-avi->val_),avi) {
}
void chain() {
avi_->adj_ += adj_ / (avi_->val_ - 1);
}
};
}

and here's the function getting called to create it:

inline var log1m(const stan::agrad::var& a) {
return var(new log1m_vari(a.vi_));
}

What happens is that the constructor for log1m_vari() throws
an exception.

What I want to understand is what's trying to deallocate.

- Bob

[Ben Goodrich]

On Saturday, May 11, 2013 8:39:57 PM UTC-4, Bob Carpenter wrote:

For one thing, log1m_vari instances should be smaller
than 80 bytes.  Is this multiple ones?  Or an attempt to
deallocate something from our var stack that's not there?

I don't know what is up with the 80 bytes thing. It does say that it is trying to free something that hasn't been malloc()-ed. My guess is that the exception precludes the constructor from finishing and then something comes along and tries to free something that would exist if x < 1.

Ben

[Jiqiang Guo]

I guess when the "new" in the following code fails (throw an exception), according to here (http://www.parashift.com/c++-faq-lite/memory-pools.html), the compiler generates some code to delete the pointer that is supposed to point to the object we are trying to create.   And this delete is problematic here. 

    inline var log1m(const stan::agrad::var& a) {
      return var(new log1m_vari(a.vi_));
    }

So if I overload the delete operator with a no-op for this class, at least the test can finish. 

jq2@jq-tpw510:~/Desktop/stan.git$ git diff src

diff --git a/src/stan/agrad/rev/log1m.hpp b/src/stan/agrad/rev/log1m.hpp

index ccec2b1..635d7b0 100644

--- a/src/stan/agrad/rev/log1m.hpp

+++ b/src/stan/agrad/rev/log1m.hpp

@@ -19,6 +19,7 @@ namespace stan {

         void chain() {

           avi_->adj_ += adj_ / (avi_->val_ - 1);

         }

+        void operator delete(void* p) {} 

       };

     }

 

diff --git a/src/test/agrad/rev/log1m_test.cpp b/src/test/agrad/rev/log1m_test.cpp

index acd32b1..1940d7a 100644

--- a/src/test/agrad/rev/log1m_test.cpp

+++ b/src/test/agrad/rev/log1m_test.cpp

@@ -3,7 +3,7 @@

 #include <gtest/gtest.h>

 

 TEST(AgradRev,log1m) {

-  AVAR a = 0.1;

+  AVAR a = 1.1;

   AVAR f = log1m(a);

   EXPECT_FLOAT_EQ(log(1 - 0.1), f.val());

   

jq2@jq-tpw510:~/Desktop/stan.git$ make test/agrad/rev/log1m

g++ -I src -I lib/eigen_3.1.3 -I lib/boost_1.53.0 -Wall -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -g   -lpthread  -O3 lib/gtest_1.6.0/src/gtest_main.cc test/agrad/rev/log1m.o -DGTEST_HAS_PTHREAD=0 -I lib/gtest_1.6.0/include -I lib/gtest_1.6.0 -o test/agrad/rev/log1m test/libgtest.a -Lbin -lstan -Lbin -lstanc

test/agrad/rev/log1m --gtest_output="xml:test/agrad/rev/log1m.xml"

Running main() from gtest_main.cc

[==========] Running 1 test from 1 test case.

[----------] Global test environment set-up.

[----------] 1 test from AgradRev

[ RUN      ] AgradRev.log1m

unknown file: Failure

C++ exception with description "x can not be less than -1" thrown in the test body.

[  FAILED  ] AgradRev.log1m (0 ms)

[----------] 1 test from AgradRev (0 ms total)

[----------] Global test environment tear-down

[==========] 1 test from 1 test case ran. (0 ms total)

[  PASSED  ] 0 tests.

[  FAILED  ] 1 test, listed below:

[  FAILED  ] AgradRev.log1m

 1 FAILED TEST

--

Jiqiang 

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Bob Carpenter]

Thanks so much --- this is just what I wanted.

Perhaps not so surprising the answer comes in
a FAQ novel about exactly what we're implementing!
Now let me go read it.

- Bob

> stan-dev+u...@googlegroups.com <mailto:stan-dev%2Bunsu...@googlegroups.com>.